Better practice for version checker repeated checks

Currently, this is the code that starts up the version checker script
InitVersionChecker(timer, moreinfo)
{
	#if defined PRE_RELEASE_VERSION
	#pragma unused timer
	#pragma unused moreinfo
	VersionCheckerStatus = false;
	#else
	if(timer)
	{
		SetTimer("ReportServerVersion", 1 * 60 * 60 * 1000, true);
	}
	if(moreinfo)
	{
	    HTTP(1, HTTP_GET, VERSION_CHECKER_FORCEUSER_URL, "", "ForceUserToUseNewest");
	}
	VC_ConnectionFailures = 0;
	HTTP(0, HTTP_GET, VERSION_CHECKER_VERSION_URL, "", "SaveVersionInStr");
	#endif
}

More closely, this
SetTimer("ReportServerVersion", 1 * 60 * 60 * 1000, true);

This keeps repeating the version checks to see whether it's up to date or not over time. However I'm not much happy with it being in a timer like this. I'm not quite sure what could be better though. I have a few suggestions

Check under OnPlayerConnect (not every time though, maybe once every 15 new player connections or so...)
Check when the last player in the server disconnects (i.e when the server is left empty).
...?

I feel more comfortable with the second option (i.e OnPlayerDisconnect: last player leaves). What do you think?

Well, I think first one is better.
What if a player keeps logging in and out, It would be even worse then the current checking.
so I guess first one seems proper.

First one? Do you mean the timer method? @ApplePieLife

@Whitetigerswt I'd like to hear from you.

@KHKKhalid I mean the OnPlayerConnect method once each x players, but, that's also not a good idea.
I could call 7 tcw's on a raw and cancel them all if I don't feel like playing.
So OnPlayerConnect way is pretty much same as OnPlayerDisconnect however, Players connect to the server for a reason. (and they might disconnect from the server/s for any reason).
Personally, I can't say one of the methods are  better than the other. They are almost the same, Using current method with a bit more modification would seem proper.
If you really think it's a bad suggestion to keep current one, then monitor the time taken to reach x players and the time taken for them all to disconnect, I'm quite sure it will be way less time than the timer you are using above, however, You can't really expect players to play 24/7.
Lets make a bit of fast compare.
-- Current method: once an hour.
-- Using OnPlayerConnect with players count check.
Lets say they are going to play a tcw.
so 5-10 mins for players joining + 72 mins 9 rounds x 8 mins on worst scenario of a tcw.
-- Using OnPlayerDisconnect:
??? player might connect and stay afk for hours or connect and disconnect for no reason.
so It's either OnPlayerConnect with checks of players (or - if I may suggest - when a tcw is end/over and 7 rounds were played atleast)

Well, you totally misunderstood things. @ApplePieLife
OnPlayerConnect
I will not check version every time it is called as I said I will do the checks once every 5 to 10 OnPlayerConnect calls.
OnPlayerDisconnect
I will not check version every time it is called. I will only check version when the LAST player leaves (e.g 1/40) and then server is empty (i.e 0/X).

@KHKKhalid That's what I had in mind.
my calculation of OnPlayerConnect are based that you choose a check of 6+ players online, I don't really think I misunderstood anything.
the OnPlayerDisconnect issue I had in my mind. Server 0/40 > I connect 1/40 > Disconnect 0/40 (server checks version), Lets say I did that once each 10 mins, nonsense ? some people connect and stay online for hours then they disconnect.
So in some cases, OnPlayerDisconnect might take mins to be called over and over and in others it might take hours (or even days)

I've already thought about this but that's not very smart either @ApplePieLife

I would just do it when the server starts, tbh, don't know why you need this on a timer in the first place. you don't need to check for updates except when the server starts. thats how the AC does it, hasn't really ever been an issue there.
but if you have to, i'd do it in onplayerdisconnect. HTTP calls are threaded so it won't slow down anything. even if it did, 0 players are connected so who cares?

Yeah, exactly @Whitetigerswt. As I said, I feel more comfortable to using OnPlayerDisconnect but I was kinda jumbled up and just needed someone to promote it for me :stuck_out_tongue:

don't know why you need this on a timer in the first place.

I want server owners to get and listen to updates QUICKER.


Failure to open attachments

First of all, thanks for making nmail. Not being used to terminal MUAs, I wanted to try and started with Neomutt but was overwhelmed by the number of controls and configuration settings required to make it fully functional, then tried sup but disliked the lack of sidebar and again, numerous different controls (they all have a purpose, I know, but nmail does a better job at summarizing most important keys on the screen). No sidebar in nmail either, but a very straightforward UI including one listing folders, very much like a sidebar. I also love that attachments are open as temporary files by default. Great stuff.
I am trying to open image attachments but everything I try in main.conf fails, be it xdg-open, viewnior or ristretto. I am running Ubuntu xfce4 in chroot from a Sailfish OS device (with XWayland). I installed gtk2-engines-pixbuf due to multiple warnings in log.txt, which worked, but I am still getting this after a session in which I try to open attachments:
2020-01-13 02:31:11.599 | INFO  | starting nmail v1.12  (main.cpp:103)
2020-01-13 02:31:11.599 | INFO  | using Linux/gcc-8.3.0  (main.cpp:107)
2020-01-13 02:31:15.561 | ERROR | mailimap_uid_fetch(m_Imap, set, fetch_type, &fetch_result) = 9  (imap.cpp:349)
2020-01-13 02:32:18.415 | INFO  | exiting nmail  (main.cpp:258

Exemple with image files and ext_viewer_cmd=xdg-open in main.conf:

It's the same if I configure xdg-open to use ristretto instead of viewnior, except ristretto just stays empty without the warning message. I checked document files too, they open Libreoffice but the temp files are not found. I checked the tmp/ folder without quitting nmail, and indeed no file appearing in there after trying to open attachments in nmail. Every attempts seems to try creating a new unique filename, if I believe viewnior and Libreoffice's "/home/user/.nmail/tmp/tempfile.xxxx does not exist" errors.
[Edit] In another session with --verbose on, I could not find anything useful in log.txt.

Hi! Thanks for the extensive bug report, much appreciated! You've already answered almost all the questions I would ask. :)
Could you try saving an attachment and open outside nmail (just to ensure nmail properly processes the attachment)? In the attachment view you can press s and then press enter to save in current working dir, and then check if the file can be opened manually.
Regarding how nmail is supposed to work: When opening an attachment, nmail saves the attachment in a temporary file and launches the command specified by ext_viewer_cmd setting, with the path to the temporary file as only argument. Once the command has completed/exited, the temporary file is immediately deleted, which can make it a little more tricky to debug.

Thanks for the fast answer. I can confirm saving the file works as expected, the file was properly downloaded and could be opened with the applications I tried when just "opening" from nmail.
The temporary file "cuisine" might be a bit trickier to debug, but it really is nice as it avoids cluttering the disk for no reason with files that would be hard to find again without using nmail anyway, I really like that (though a prompt to choose where to save files would be nice, but saving in pwd form where nmail was launched is relevant too).

Thank you! OK, I think I need to add some more debug logging in nmail in the portion that executes the external command. No time today though unfortunately, but I'll get back soon.

For what it's worth, I have installed nmail on my computer running Solus and am experiencing the same issue with all kinds of attachments (html, text, image, pdf). The default applications are properly called, but fail to find the files. I've tried running nmail as root, but still the same issue.
If I monitor the ~/.nmail/tmp folder when attempting to open an attachment, I can confirm that no file is being created, even temporarily.

Ok thanks for testing another distro. Since the correct application is launched I have a feeling there's an issue with how nmail launches the application. I suspect that xdg-open returns immediately when being called, and thus nmail proceeds to delete the temporary file (before the external program has loaded the file). I have only tested nmail a little on Linux (Ubuntu 18.04) but I have not seen this issue (mainly tested image attachments). On Mac the launching program is open -Wn which always waits for the called program to terminate. Perhaps xdg-open does not always do that.
I don't have a Linux system with GUI available at the moment, but perhaps you could test running xdg-open manually on some local file, from the command line and see if it immediately returns (before the viewing application has terminated) or not?
If it does return immediately I have some idea on how I can fix it.
In parallel I'll look at adding some logging.

I've added some logging in 9844808 which could also help determine in the external command xdg-open is exiting immediately or not.

I can confirm xdg-open returns immediately (i.e., the terminal is not staying in a busy state as it does with other commands, and I cannot close the opened application by hitting ctrl+c in the terminal). When monitoring the actual nmail tmp/ folder, I could not see any file being created, even briefly, but maybe there is a reason for that.
I have not tried your commit yet, let me know if my description is clear enough or if you prefer me to git pull and build again.


Request to fix Expandable Item BUG

@davideas First set the Item default to collapse, then search for input values, then cancel the search. Now click on any item and try to expand the collapse. But after the item tries to expand and collapse, the item will automatically close the collapse.

Fix.

Bug 2: When enter "2," in the search box and cancel the search, the above problem will occur. This is because mExpandedFilterFlags incorrectly cached the status when the second input of the first search, so this problem is caused.

@tcqq, I finally had time to look into it. I'm fixing it.


Aligned background color

Ensure the loader also displays the appropriate color in conjunction with the modal window and sideframe.

LGTM


Allow custom MSI URL when bootstrapping a Windows VM

What are you trying to do and what's not working?
I want to clone a Windows VM and then bootstrap it with a Chef server. The node does not have Internet access, and so when using knife windows on a command line, I use the --msi-url option to specify a custom chef-client install URL.
However, I cannot do this from knife vsphere today. It would be great if we could add an option like --bootstrap-msi-url that is passed on to BootstrapWindowsWinrm.
Please paste the command and output (run with -VV). Don't forget to X out any passwords or sensitive information
knife vsphere vm clone <name> --template win-2012-template --folder <folder> --bootstrap --linked-clone --bootstrap-protocol winrm --cips dhcp --resource-pool <rp>

A knife-windows command to bootstrap using a custom MSI URL:
knife bootstrap windows winrm <ip address of node> -x <user> -N <name> --msi-url <a url inside my org that my node can reach>

Other details, such as gem version and if you're working with a Linux or Windows VM
Gem version 1.2.14.

This is also mentioned in issue #272 but that issue has a broader discussion.

So my problem is that I don't do much Windows work so testing these things is kind of troublesome.
JJ and I worked through #284 and it does look like getting options over to the Bootstrap code is as simple as that. See the bootstrap_for_windows_node method.

I am able to try it out against Windows VMs. I currently work on a fork of knife-vsphere where I cannot submit PRs upstream, but I am happy to report back here :)

Can you try it out against my branch above? https://github.com/swalberg/knife-vsphere/tree/msi-url (or just copy paste into your local copy, it's only a few lines)
Thanks.

Thanks, yes it almost works. I just had to add a URL value to the new parameter, like this:
diff --git a/lib/chef/knife/vsphere_vm_clone.rb b/lib/chef/knife/vsphere_vm_clone.rb
index e4de2d2..3cbf0e7 100644
--- a/lib/chef/knife/vsphere_vm_clone.rb
+++ b/lib/chef/knife/vsphere_vm_clone.rb
@@ -147,6 +147,10 @@ class Chef::Knife::VsphereVmClone < Chef::Knife::BaseVsphereCommand
          long: '--fqdn SERVER_FQDN',
          description: 'Fully qualified hostname for bootstrapping'

+  option :bootstrap_msi_url,
+         long: '--bootstrap-msi-url URL',
+         description: 'Location of the Chef Client MSI. The default templates will prefer to download from this location.'
+
   option :bootstrap_protocol,
          long: '--bootstrap-protocol protocol',
          description: 'Protocol to bootstrap windows servers. options: winrm/ssh',
@@ -778,6 +782,7 @@ class Chef::Knife::VsphereVmClone < Chef::Knife::BaseVsphereCommand
       bootstrap.config[:winrm_password] = get_config(:winrm_password)
       bootstrap.config[:winrm_transport] = get_config(:winrm_transport)
       bootstrap.config[:winrm_port] = get_config(:winrm_port)
+      bootstrap.config[:msi_url] = get_config(:bootstrap_msi_url)
     elsif get_config(:bootstrap_protocol) == 'ssh'
       bootstrap = Chef::Knife::BootstrapWindowsSsh.new
       bootstrap.config[:ssh_user] = get_config(:ssh_user)

I had put that in bootstrap_common_params, did it not work there? Or does it not make sense for an SSH bootstrapped node (I guess I could have equally put it after the if statement.

Sorry, I missed that. I will try that.

It works in bootstrap_common_params too. So I only changed this line to have the URL value:
 long: '--bootstrap-msi-url URL',


Need to support the tags parameter in the /usage endpoint

Proposed changes
Add the tags parameter to the /usage reporting endpoint
https://developers.deepgram.com/api-reference/#usage-summary
Context
The API supports filtering summarized usage by tag, but the SDK doesn't support it.

@MichaelJolley
Doesn't this already support the tag?
Looking at the UsageOptions it looks to me like the tag is already in there.
https://github.com/deepgram/deepgram-node-sdk/blob/fbc58689bc2891ca4ae667ab7dc4baec72b3d6bf/src/types/usageOptions.ts#L5

Nope. Apparently, I didn't close this.


[BEAM-10125] adding cross-language KafkaIO integration test

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[ ] Choose reviewer(s) and mention them in a comment (R: @username).
[ ] Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

R: @chamikaramj

This change is also used for XLang KafkaIO internal test.

LGTM. Thanks.

Retest this please

Retest this please

Retest this please

Retest this please

Retest this please

Retest this please


Process/Instance hierarchy in MDWHub

Provide these features in MDWHub similar to what we have in Designer.

Hi Jack,
Sorry about the confusion. I will try to explain the scenario.
In TOM we have broken the modules into multiple MicroServices, which call each other. Each MicroService consists of/triggers multiple processes.
When I right click on any process instance and view hierarchy in mdw designer, it will list the calling process and called process instances. But it wont list the calling and called microservices and their process instances.
Because there are more process instances (than what mdw listed under process hierarchy) belonging to same transaction via MicroService calls, it is convinient to see all of them in the hierarhy results.
So my question was, is there any feature that lists the MicroServices hierarchy. (By Microservices hierarchy, I mean process instances hierarchy under each MicroService)
Let me know if you like to IM or have a short call. Thanks for your response.
Thanks,
Imran

With the change in service Designer calls to now call Processes service, it (Processes) now can handle the same functionality to show the process hierarchy, so now it's just a matter of displaying it in Hub.   Search for "callHierarchyFor" in Processes.java.


String union types narrowed to falsy should narrow string to ""

Bug Report
🔎 Search Terms
string truthy falsy narrow literal empty
🕗 Version & Regression Information

~This is a crash~
~This changed between versions ______ and _______~
This is the behavior in every version I tried, and I reviewed the FAQ for entries about narrowing
~I was unable to test this on prior versions because _______~

⏯ Playground Link
Playground link with relevant code
💻 Code

let value = Math.random() > 0.5 && "Some Name";

if (!value) {
  value; // Type: false | string, but should be false | ""
}

🙁 Actual behavior
Inside the narrowed block, value should be false | "", because no other string value is falsy.
🙂 Expected behavior
It's the more general false | string instead.
This was a hard issue to search for; #41503, #33878, and #31156 seemed related but not duplicate to me.
Shoutout to RyanCavanaugh for sending me a code snippet that looked like this -- no good deed goes unpunished in the issue tracker 😄

This seems like a good idea. I'm wondering what it'd break; would love a draft PR so we could run it over real-world code to see.

Shoot sorry for missing this @ahejlsberg, it dropped off my radar. I've seen the occasional snippet of code that uses "" instead of null or undefined in unions of response types.
type Result = "ACK" | "ERR" | "";

declare const result: Result;

if (result) {
  // handle that a result was received
} else {
  // couldn't get a result
}

I don't have any examples that are open source, though.


Update the Documentation

Main changes
Context
#1
Changelog

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
517

Relevant Lines:
517

💛  - Coveralls


Leverage OriendDB inheritance within Ferma OGM

Dear Team members,
first of all I am very sorry for my poor english.
Expected behavior
I am in my very first try in the "NoSQL" world. As I come from Hibernate / JPA experience, I would like to use the Ferma OGM framework on top of OrientDB Graph. So I have wrote some interfaces annotated with com.syncleus.ferma.annotations.* annotations and I have registered them within a types collection, calling new com.syncleus.ferma.DelegatingFramedTransactionalGraph<OrientGraph>(orientGraph, true, types) and finally I added some vertices from every type in types, calling addFramedVertex(type) method.
I expected to find in the Schema a new OrientDB Class for each Class contained in types collection.
Actual behaviour
But when I explored the Schema with Studio, I didn't see any new classes, but instead all vertices I added saved as V class, having the type information stored as 'ferma_type' property.
As I read here, this seems the default expected behaviour of Ferma, with the only opportunity to change the property name.
Important Questions
So can I leverage the wonderful multiple inheritance mechanism of OrientDB with Ferma OGM classes? As a second chance, are there any way to use POJOs (as I always did in JPA) on top of the GRAPH API so one can automatically reflect the Java classes structure in actual OrientDB classes?
Thank you
Giulio Quaresima
Runninng Mode

[ ] Embedded, using PLOCAL access mode
[ ] Embedded, using MEMORY access mode
[x] Remote

Misc

[ ] I have a distributed setup with multiple servers. How many?
[ ] I'm using the Enterprise Edition

OrientDB Version

[ ] v2.0.x - Please specify last number:
[ ] v2.1.x - Please specify last number:
[x] v2.2.x - Please specify last number:

Operating System

[x] Linux
[ ] MacOSX
[ ] Windows
[ ] Other Unix
[ ] Other, name?

Java Version

[ ] 6
[ ] 7
[x] 8

hi @giulio-quaresima,
We have no problem on this kind of management on orientdb side, and we have some specific api that would allow this case, did you try to ask the same question on the Ferma issue tracker? We have no control on Ferma code, but we are more than happy to contribute to it.
Regards

Thank you. It seems that Ferma issue tracker doesn't work these days, and the project doesn't seem very active as well. Maybe I will do the job using directly the Graph API and writing some mapping between it and my domain model.

I resolved the issue using the wonderful Frames API and its annotation
@TypeField ("@class")
in my domain supertype.
Thanks

Cool, I didn't know you could do that. @giulio-quaresima would you like to write a blog post about FERMA and OrientDB, making the life of future developers with FERMA easier?

Yes, I'd like very much to contribute! But to do the job I used Frames, not Ferma. With Ferma I didn' succeed: i tried with
new PolymorphicTypeResolver(reflectionCache, "@class")
but I got
java.lang.IllegalStateException: The class V cannot be found.
Browsing the source code, it is clear that Ferma, with such configuration, when you call addFramedVertex(MyDomainEntity.class), first create the Vertex in the graph, and then call vertex.getProperty("@class") to check if the property already exists (expecting not with newly created Vertices), but the @class property already contains "V", as for any newly created vertex in OrientDB, and Class.forName("V") doesn't work, obviously.
Incredibily, in the Ferma: Comparing the Alternatives page it is stated that Frames doesn't let "customize the way type information is stored in the graph". On the contrary, Frames support such a customization, through @TypeField and @TypeValue annotations, and as I can see it addresses also the particular behaviour of OrientDB's @class property: with Ferma, as I showed above, it doesn't work, and, also, PolymorphicTypeResolver let to customize the property name only, leaving the property value bounded to the full qualified name of the class.
With Ferma I encountered other limitations, too. For example, I wonder how I could simply manage embedded properties (another plus of your multi-model architecture), while with Frames it's easy to do with the great flexibility provided by the @JavaHandler annotation. Probably it's possible with Ferma too, with concrete classes extending VertexFrame, but it seems more cumbersome.
In general, Frames appears better designed, with a more declarative approach and much more flexibility, and it's also better documented: too sad that Frames has been discarded from Tinkerpop 3!

What about a blog post with Frames? :-)

OK, I will do it gladly, after I have tested some other Frames functionalities. Would you give me the instructions to post to the blog?

You could write it in a Google Docs. The best place to write it is DZone or even on our web site.

There is a way to workaround this with Ferma, you can use
fg.addFramedVertex("class:YourLabel", new DefaultClassInitializer<>(Something.class));.
This will create the vertex with @class YourLabel and another property ferma_type as well.


ENH: Performance improvement for BinaryEncoder

Fixes #80
For a small test case, I ran the following:
import category_encoders as ce
import numpy as np
import pandas as pd
df = pd.DataFrame({'a': np.random.randint(0,high=10, size=10000)})
be = ce.BinaryEncoder(cols=['a'])

And timed be.fit_transform(df). On master, it ran in 101 ms, with this PR, it ran in 18ms.

Awesome! Thanks, @thomasjpfan


[FormRecognizer] Detect content-type of input form

Automatically detect content type so user doesn't have to pass this as a parameter

merged


A Prisma error that covers all possible errors as mentioned in Errors reference

Problem

In any try/catch that runs a prisma query you'd have to check if the error is an instance of a specific prisma error in order to handle different kinds of errors and get the type-safety, for example if you want the the code or message property of an error.
If you want to catch all possible prisma errors you'd have to write a seperate if statement for all of them, for example if you want to log the error.
if (error instanceof Prisma.PrismaClientKnownRequestError) {
  log(error.code, error.message)
}

and so on...

If you dont do this, a possible error will not be logged because you are not checking if it's an instance of. If you dont do any if statements you dont have the type safety
catch (error) {
  log(error.code) // type unknown, accepts a string;
  log(error.message) // type unknown, accepts a string
}

Suggested solution

A prisma error that covers all exceptions
catch (error) {
  if (error instanceof Prisma.Error) {
    log(error.code, error.message) // this will log any error that prisma throws + typesafety. both code and message are a string
  }
}

Alternatives

Additional context

I believe https://github.com/prisma/prisma/issues/5040 would solve this issue.

I dont believe it would, you'd still have to make a bunch of if statements to cover all possible scenarios, and in each scenario i'd have to call the log function to have typesafety.

any news?

Would love a better way to handle errors. Even a property on the error object that returns just the error itself would be fantastic. I really don't want to return:
Invalid `prisma.user.update()` invocation in
/home/leonardo/ironeko-fondamenta/.next/server/pages/api.js:127:32

  124   prisma
  125 }) => {
  126   try {
→ 127     return await prisma.user.update(
  An operation failed because it depends on one or more records that were required but not found. Record to update not found.

To my frontend.

Crazy that you can get this highly detailed error message from the engine
  20 async create(data: CreateUserDto) {
→ 21   const user = await this.prismaService.user.create({
         data: {
           email<EMAIL_ADDRESS>           password: '$2a$10$CBE/dMtwekhH9IlocK1cPOXr9g.NfI3pCmaSmLFVOYv5yvkbBdY26',
       +   name: String,
       +   surname: String,
       ?   walletAddress?: String | null,
       ?   role?: INVESTOR | AGENT
         }
       })

Argument name for data.name is missing.
Argument surname for data.surname is missing.

Note: Lines with + are required, lines with ? are optional.

But not this from the client to just deliver it in the response:
{
  code: 123,
  meta: ['name', 'surname'],
  message: 'missing required fields'
}

Would love a better way to handle errors. Even a property on the error object that returns just the error itself would be fantastic. I really don't want to return:
Invalid `prisma.user.update()` invocation in
/home/leonardo/ironeko-fondamenta/.next/server/pages/api.js:127:32

  124   prisma
  125 }) => {
  126   try {
→ 127     return await prisma.user.update(
  An operation failed because it depends on one or more records that were required but not found. Record to update not found.

To my frontend.

can specify it like this
new PrismaClient({
    errorFormat: 'minimal',
  })

refer this for more info

Hello everyone!
Do we have a solution for this or creating a bunch of "if" to handle all errors is still the solution we have?
Thanks!

If I user prisma with expressjs, and there is an unhandled prisma error, then the server crashes. It would be great to have a way to catch all the unhandled errors from prisma and log it.

An example for a global error handler would be much appreciated, I haven't been able to deliver this exceptions safely to the client without making the code look ugly
currently I'm doing it like this (ugly):
const result = await db.movie.create(...).catch((e) => {
  return await handlePrismaErrors()(req, res);
})

if (result) {
  // keep doing stuff
} else {
  // response early return, return from here as well
 return;
}

and then


Possibly move call method into StencilObject base class

None of the backend classes override generate_pre_run or generate_post_run, so there is no reason other than argument typing for it to be in the base class.

The __call__ of StencilObjects is code generated and contains the specific field names in the signature that are declared in the stencil. They can not be moved to the base class equivalently.
On the other hand,  generate_pre_run and generate_post_run are methods in code generation, not run time. Are you referring to the generaters rather than the StencilObject here?

The call of StencilObjects is code generated and contains the specific field names in the signature that are declared in the stencil. They can not be moved to the base class equivalently.

Sorry I wasn't clear. I thought it could just be __call__(*args, **kwargs) and forwarded to the stencil. However, when thinking about this more I think you've convinced me that this function signature correctness is important enough to have the code generate it.


[bug] Remove jovyan user from Jupyter Dockerfile

Describe the bug
QHub uses actual usernames instead of the conventional jovyan user, but that user still exists in the base Jupyterlab Dockerfile and has its own home directory in addition to the actual username: https://github.com/Quansight/qhub/blob/main/qhub/template/image/Dockerfile.jupyterlab#L11
This results in some unexpected behavior when running basic commands:

creation of ssh keys appear exclusively in /home/jovyan folder, irrespective of user
other commands e.g. ls hit the /home/{username} folder instead

Expected behaviour
Any and all commands should be run inside the user's /home/{username} folder, and /home/jovyan should not exist.

Alternative: have all users just be jovyan with their own UIDs. This would be a reversion to older behavior.

@costrouc @danlester about this, haven't we moved to keycloak for managing user files, and then we used the username as an alias in the terminal? As result, everyone would be jovyan with the same uids and different aliases...
What should we do regarding cases as follow?

/home/jovyan and /home/ both exist at the same time. Creating ssh keys goes to the jovyan folder while ls etc hits the  folder.

We can remove jovyan from the Dockerfile and still keep the home at /home/jovyan if we want. Removing jovyan from the image will simplify the dockerfile build.
My thoughts where I'd eventually like this to go. I want this to "feel" like the user is on a genuine linux system.
We run each user as a unique uid, with proper primary gid, and secondary group ids, along with a home directory in the expected location that matches their username. This is the expected "linux" behavior. We have the tooling the QHub to achieve this now (except for populating /etc/group). This brings a lot of benefits to security and redhat has some articles on this.
My take on why /home/joyvan became a convention is due to jupyterhub not having the tooling to support dynamic users with different uids/gids.

The main issue right now is that there are two home folders /home/jovyan and /home/<username> that exist at the moment. We only need one. By removing /home/jovyan  we will only have one. If we would prefer to keep jovyan then we could just set username to jovyan.

having /home/<username> is nice but only if we can do it without causing problems.

We discussed this at our stand-up this morning and it sounds like it's a fairly straightforward fix to remove jovyan entirely. I believe @tylerpotts @iameskild are tasked with it, and @costrouc will review the resulting PR.

Closes by PR #1089


peg emsdk version

It seems that the following line will always install the latest tagged emsdk release: https://github.com/robertknight/tesseract-wasm/blob/534e5f7df602a432e5e61ee1dcc832c72a80fe4c/Makefile#L67
It looks like this might make builds non-deterministic. Should it be changed to a specific version?

I'm also wondering, now, if it's even necessary to install/activate emsdk this way, or if it can be run right out of the third_party/emsdk directory.

It looks like this might make builds non-deterministic. Should it be changed to a specific version?
I'm also wondering, now, if it's even necessary to install/activate emsdk this way, or if it can be run right out of the third_party/emsdk directory.

I don't know the answers to these two questions. If the build i not deterministic, it would be good to make them so. If it is possible to avoid the install/activate steps, without complicating the rest of the build, that would also be good.

It looks like this might make builds non-deterministic. Should it be changed to a specific version?

I looked into the way the latest version gets resolved. It is specified in a JSON config file that is part of the emsdk package, so if you check out the emsdk repo at a specific commit, as we are doing, latest will always resolve to the same version - currently "3.1.15". So the build is actually deterministic/repeatable, as long as latest is handled in the same way.

Ah, excellent. Thanks for figuring this out.


GH2228: Fix CakeTaskExtensions accessibility

Fixes #2228

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.:x: gitfool sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

The TeamCity build failed with an unrelated error run git clean.

@gitfool  your changes have been merged, thanks for your contribution 👍


FIx monitor slug in Oban + Quantum integrations

Closes #720.

@milmazz this is a first pass to get things working, but I suspect we'll want some level of customization here anyway. I don't want to figure out the API now though, I just want to get this fixed for now so that we have time to give it some more thought.


west: openocd: Add config file dir to search path

Use the folder the config file is in as an additional search directory
for openocd. This way additional files, like custom debug interface files
can be put in the support folder of a board and the openocd.cfg can use
them.
Signed-off-by: Christian Taedcke<EMAIL_ADDRESS>

reopening to kick CI


Fix random replacement of http with https in resources (issue #3599)

An earlier "fix" for urls in emails added a blanket replace on each and every resource being retrieved. This is not just incorrect (as it also affects resources that should not be touched), it is also a performance hit.
This PR rolls back the fix and adds a "scheme" token to the portal settings. The [portal:scheme] token will resolve to either "https" or "http" depending on SSLEnabled for the portal. It is not used in any email in the framework currently, but if someone needs this in a template, it can be used.
Note that using the scheme token looks at the entire portal, so won't be a solution for those with mixed http/https situations. The [portal:fullurl] uses the current page's SSL setting, but obviously depends on context and so was never meant for emails.

once this PR has been included, I'll have a look to update texts of the resource files.

So, there's a small chance for this to be a breaking change.  If someone has a modified version of an email template which uses the old http://[Portal:URL]/default.aspx?ctl=PasswordReset&resetToken=[Membership:PasswordResetToken] URL, it will no longer get auto-upgraded to https.  @donker @mitchelsellers do you think it's worth either doing a find/replace in the email code, or running an update of localized resources during upgrade (i.e. find/replace http://[Portal:URL] with [Portal:FULLURL])?

So, there's a small chance for this to be a breaking change. If someone has a modified version of an email template which uses the old http://[Portal:URL]/default.aspx?ctl=PasswordReset&resetToken=[Membership:PasswordResetToken] URL, it will no longer get auto-upgraded to https. @donker @mitchelsellers do you think it's worth either doing a find/replace in the email code, or running an update of localized resources during upgrade (i.e. find/replace http://[Portal:URL] with [Portal:FULLURL])?

Yes, this is a breaking change, but should not affect too many sites. However, we should definetely mention it in release notes

So, there's a small chance for this to be a breaking change. If someone has a modified version of an email template which uses the old http://[Portal:URL]/default.aspx?ctl=PasswordReset&resetToken=[Membership:PasswordResetToken] URL, it will no longer get auto-upgraded to https. @donker @mitchelsellers do you think it's worth either doing a find/replace in the email code, or running an update of localized resources during upgrade (i.e. find/replace http://[Portal:URL] with [Portal:FULLURL])?

Absolutely. Which is why it goes in 9.7. Like Sebastian says, chances are slim and frankly, it is IMHO worse that those who included external fixed links using http find that the http was changed. So I am favouring those that highlighted that latter issue.

Regarding the breaking change, the CoreMessagingScheduler, at least, does its own replace HTTP with HTTPS, so that should still continue to work:
https://github.com/dnnsoftware/Dnn.Platform/blob/91223d2b4307ea98a1c4e5b8a472e996c60a1c76/DNN Platform/Library/Services/Social/Messaging/Scheduler/CoreMessagingScheduler.cs#L572-L581

But it doesn't look like CoreMessagingScheduler uses the standard token replacement, it has custom tokens (e.g. [SITEURL] instead of [Portal:URL]).  The scenario where we would expect breakage is the call to Mail.SendMail(newUser, MessageType.UserRegistrationPublic, PortalSettings) in UserModuleBase.  @mitchelsellers @valadas are you okay with this small breaking change in 9.7?  It's not a big deal to me.
Globals.AddHTTP does reference HttpContent.Current, but it checks for null first, so it should be safe to use from a scheduled task.

@bdukes Yes, I think we are fine with this one, it errs on the side of security essentially.  But I'm flagging as potentially breaking so we are sure to grab it in the release notes


Fix a crash when requesting money

Details
This fixes a regression introduced by https://github.com/Expensify/App/pull/35287.
The policy can initially be undefined, which wasn't handled correctly.
Fixed Issues

$ https://github.com/Expensify/App/issues/36433
PROPOSAL: N/A
Tests

Same as QA Steps
Offline tests

QA Steps

Go to FAB > Request money
Enter any amount, proceed
Select a non-workspace participant, proceed
[ ] Verify that the money request got successfully created

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari
https://github.com/Expensify/App/assets/2590174/978ce0ab-d48d-4c38-bbfd-686f55e38764

MacOS: Desktop

@rushatgabhane This likely won't need the C+ review

@pecanoro I tested this on one platform, as this is a straight-forward change with no platform-specific aspects

"TypeScript Checks" sound completely unrelated, likely an issue on main

"TypeScript Checks" sound completely unrelated, it's an issue on main: #34811 (comment)

@cubuspl42 please merge main after this PR gets merged

@cubuspl42 please merge main & respond to my comment so we can get this movin 🙏

oh shit, optional chaining is forbidden in .js files? I was hoping we removed that restriction completely, not just for .ts 🙃 my bad

I'll revert it in a moment

Reviewer Checklist

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If any new file was added I verified that:

[ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[ ] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[ ] If the PR modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label so the design team can review the changes.

[ ] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[ ] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 1.4.41-8 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.41-12 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


Inconsistency in "Vector Widening Saturating Scaled Multiply-Add" examples

In chapter 13.4 Vector Widening Saturating Scaled Multiply-Add, there are the code examples as follows:
# Widening signed-unsigned-integer scaled multiply-accumulate
vwsmaccsu.vv vd, vs1, vs2, vm
             # vd[i] = clip(-((signed(vs1[i])*unsigned(vs2[i])+round)>>SEW/2)+vd[i])
vwsmaccsu.vx vd, rs1, vs2, vm
             # vd[i] = clip(-((signed(x[rs1])*unsigned(vs2[i])+round)>>SEW/2)+vd[i])

# Widening unsigned-signed-integer scaled  multiply-accumulate
vwsmaccus.vx vd, rs1, vs2, vm
             # vd[i] = clip(-((unsigned(x[rs1])*signed(vs2[i])+round)>>SEW/2)+vd[i])

The code comment says "multiply-accumulate" and the instruction mnemonic is "macc", but the operation equation has "minus" sign in front of the multiplication result. Should the "minus" sign be removed?

This is an editing error that arose when the multiply-subtract instructions were replaced with signed-unsigned multiply-add instructions.  Thank you for finding it.


[SBTrack 2] Prototype Public API for Sender Queue Client

Design public API for SenderQueueClient for  Febuary 2020 Milestone.
Febuary 2020 Milestone Objective:

Sender client (everything below is already implemented in EventHubProducerClient)
A. constructor overloads
B. connection string, queue/topic name, options
C. namespace, queue/topic name, credential, options
D. Options to constructors to support everything from EH like retry policy, web sockets, proxy etc.
Readonly properties
Fully qualified namespace
EntityName
Methods
Create a batch of messages (mimic the EventData from Event Hubs for now)
Send the batch

More on Febuary 2020 Milestone Objective :  https://microsoft.sharepoint.com/:o:/t/AzureDeveloperExperience/Eh6vsEGjjYhNspabYWw-e2sBYvmt-LPd7T2nRWf1C34xJQ?e=HY5kCX

Fixed as part of #4506


wait_for_idle is not enough to validate that upgrade was finished

Cou finished all upgrade steps for placement or octavia, but it failed on checking workload status. This happen when all units were in idle state meaning that it should be fine to continue.
Examples:
Check if the workload of 'placement' has been upgraded (c)ontinue/(a)bort/(s)kip:c
Generating upgrade plan... ✖
2023-11-02 16:49:45 [ERROR] Cannot upgrade units 'placement/1' to wallaby.

Change charm config of 'octavia' 'openstack-origin' to 'cloud:focal-wallaby' (c)ontinue/(a)bort/(s)kip:c
Wait 120 s for app octavia to reach the idle state. (c)ontinue/(a)bort/(s)kip:s
Check if the workload of 'octavia' has been upgraded (c)ontinue/(a)bort/(s)kip:c
Generating upgrade plan... ✖
2023-11-02 19:08:19 [ERROR] Cannot upgrade units 'octavia/0, octavia/1, octavia/2' to wallaby.

it's possible that we need to make check of workload status with timeout, so we can wait until units really settle.

This is happening for all reactive charms, so it’s not a COU issue. We can try to implement some workaround like trying to run the openstack-upgrade action, but this is a dirty fix.
We can open a bug to have a workaround like running openstak-upgrade when this is detecte.
See #2039604 for more context about the bug and possible workaround


Extensions samples should not set SuppressTfmSupportBuildWarnings

https://github.com/dotnet/extensions-samples/blob/c33a8bba6ac4ec61ad19bd4ab9dbd5b5838b6991/Directory.Build.props#L10
The SuppressTfmSupportBuildWarnings switch shouldn't be used in samples as it's a workaround to temporarily allow runtimes to consume a package that doesn't support that runtime anymore by falling back to the netstandard2.0 implementation. For System.* packages this switch might not work and we clearly document that this is an unsupported and untested scenario: https://github.com/dotnet/runtime/blob/28a069017673a03473fdd06e8d693a91e3eb9fbb/eng/packaging.targets#L222

cc @joperezr @RussKie @ericstj

Thanks for logging this, @ViktorHofer. In our case, this is actually intentional given though packages from dotnet-extensions do work on net462, and we want our samples to also cross-compile for that TFM. Do note that while dotnet/extensions can work in net462, it is not an officially supported configuration, as dotnet/extensions are only officially supported in net6.0 and above. We set the property globally to acknowledge that we understand that those TFMs are unsupported by at least some of the packages, but we still want to provide samples for  those.
We could add a comment above the setting of this property to explain this, if you think that would be valuable.

Are those extension samples customer facing?

Yes, they are the ones in this repo. Essentially all of the samples in this repo work in both net6.0+, as well as net462. But for net462, they throw build warnings due to unsupported configurations in some packages, which is why we set the property.

I'm not sure why we need this setting on officially supported versions of .NET Framework? Our packages only emit that warning on unsupported TFMs (runtimes): < net6.0 and < net462.

Our packages

This applies to packages from dotnet/runtime, but in dotnet/extensions we use the same property to control out of support frameworks, and in there, net462 is not officially supported

I wasn't aware of that but that worries me, actually a lot. These samples promote this setting to enable experimental .NET Framework support for extensions libraries. At the same time this disables the important dotnet/runtime package warning that is emitted when using our packages on out-of-support runtimes. This is mixing concerns and from our perspective harmful as this disables a part of our packaging infrastructure that we intentionally put in place.


Touches are retained

The class CCTouch ( build for x-platform ) retains touches. This is not allowed.
Having ongoing touches, while destroying components, will eventually lead to a crash.

I got rid of CCTouch tottally in 3.5. Since we don't have Android anymore, we can use UITouch directly. Now CCTouch is just a define for legacy versions.
#define CCTouch UITouch


⚠ Bofrost.nl has degraded performance

In a6f882b, Bofrost.nl (https://www.bofrost.nl) experienced degraded performance:

HTTP code: 200
Response time: 3766 ms

Resolved: Bofrost.nl performance has improved in 8d2a96a.


RPC Integration issue - GetBlock

Currently GetBlock fails, with the following message:
{"err": "invalid response: 500 Internal Server Error {\"result\":null,\"error\":{\"code\":-1,\"message\":\"JSON value is not a boolean as expected\"},\"id\":1}\n: error fetching block by hash <hash>: unable to get current block"}

This is likely due to a difference between the expected JSON-RPC method parameters and the dogecoin implementation.
This currently also prevents the Indexer process to successfully detect that dogecoind is online.
Could be an easy fix?

I can work on this one but may need some help along the way.

Anything you need :-)

Easy fix would be to remove the blockVerbosity parameter here https://github.com/rosetta-dogecoin/rosetta-dogecoin/blob/main/bitcoin/client.go#L365 ? If I am not mistaken by default in dogecoin this value is a boolean and it is true ?
But I guess we want to modify the files in the bitcoin folder as little as possible ?
I did try it. But I am having some issue with the docker build caching...

This is my first foray into golang and I've been mostly learning about package management past 24 hr.
It seems that in the docker build process we are still pulling a lot of files from coinbase/rosetta-bitcoin and changes made to local files are not being included in the build.  An example for verification purposes changed blockchain: Bitcoin to blockchain: Dogecoin in bitcoin.json, only to test if changes are reflected when config is loaded on boot and it is unfortunately not included.
I know this is per your recommendation for least amount of work in maintaining this repo and keeping it synced w upstream but I think we will have to at some point delineate for development purposes (I am thinking maybe a new branch using either a proxy repo of rosetta-bitcoin as a new intermediary temp upstream or looking into GOPROXY for local dev env).
I know this is 'hacky' but typically the way I learn best is by making tweaks on my local to show more explicit errors in console logs and without the ability to make local changes and view the exact error, it feels like searching for a needle in a haystack.  For instance I have not been able to replicate that RPC error.  If you have time could you show me how to reproduce?  Or where to look for that in logs?  Let me know if you can think of something that might resolve this.
Outside of that, I encountered a new odd but trivial bug within WSL2 dev env wherein the linux subsystem and Windows clocks become unsynced and dockerd complains about systems date & time being set correctly.  This was easily solved by entering sudo hwclock --htcosys which syncs subsystem time with Windows.  Only applicable to folks using WSL2.
Anyway hope all are doing well!  Still plugging away. ❤

Can you provide steps to reproduce or where to look for logs?

see https://gist.github.com/patricklodder/f47a1e5fd03ddaf1e29a83c81c181edf
After downloading, you can apply the patch with git apply add_net_status_probe.patch on current main branch.

Assigning a boolean value to blockVerbosity instead of an integer should fix the issue.

Can you provide steps to reproduce or where to look for logs?

see https://gist.github.com/patricklodder/f47a1e5fd03ddaf1e29a83c81c181edf
After downloading, you can apply the patch with git apply add_net_status_probe.patch on current main branch.

Great thank you!  Will fix this shortly.

I apologize for the delay but I am still having troubles with what seems like it should be a quick fix.  Here are my logs:
`panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x94665b]
goroutine 11 [running]:
github.com/coinbase/rosetta-bitcoin/bitcoin.(*Client).post(0x0, 0xe70af8, 0xc00003ad00, 0xc74762, 0x11, 0x127a680, 0x0, 0x0, 0xe64060, 0xc000180b80, ...)
/home/xanimo/go/pkg/mod/github.com/coinbase/rosetta-bitcoin@v0.0.9/bitcoin/client.go:825 +0x1fb
github.com/coinbase/rosetta-bitcoin/bitcoin.(*Client).getBlockchainInfo(0x0, 0xe70af8, 0xc00003ad00, 0x0, 0x0, 0xc0005555c0)
/home/xanimo/go/pkg/mod/github.com/coinbase/rosetta-bitcoin@v0.0.9/bitcoin/client.go:381 +0xa5
github.com/coinbase/rosetta-bitcoin/bitcoin.(*Client).getBlockHash(0x0, 0xe70af8, 0xc00003ad00, 0x0, 0xc00055575c, 0x2, 0x2, 0x10100000000)
/home/xanimo/go/pkg/mod/github.com/coinbase/rosetta-bitcoin@v0.0.9/bitcoin/client.go:398 +0x107
github.com/coinbase/rosetta-bitcoin/bitcoin.(*Client).getBlock(0x0, 0xe70af8, 0xc00003ad00, 0x0, 0xc00055575c, 0xc00008b038, 0xc00055575c)
/home/xanimo/go/pkg/mod/github.com/coinbase/rosetta-bitcoin@v0.0.9/bitcoin/client.go:356 +0x67
github.com/coinbase/rosetta-bitcoin/bitcoin.(*Client).NetworkStatus(0x0, 0xe70af8, 0xc00003ad00, 0x0, 0x2, 0x1)
/home/xanimo/go/pkg/mod/github.com/coinbase/rosetta-bitcoin@v0.0.9/bitcoin/client.go:166 +0x55
main.main.func5(0xc0001ea410, 0x0, 0xe70af8, 0xc00003ad00, 0xc0000121a0, 0xc00001e5a0)
/home/xanimo/source/repos/rosetta-dogecoin/main.go:199 +0x151
created by main.main
/home/xanimo/source/repos/rosetta-dogecoin/main.go:195 +0xa37`
Does this mean that changes made to 'our' bitcoin/client.go are irrelevant or do they merge directories on build or someting?  If so how can we point to use our client.go?

@xanimo can you do a draft PR of where you are at so that we can see the code, do a checkout and reproduce locally? That way it's easier to support.

@xanimo can you do a draft PR of where you are at so that we can see the code, do a checkout and reproduce locally? That way it's easier to support.

It's the most bare bones PR yet haha.  But yeah I'm still having trouble going to call it a night!

An intermediate Block structure (defined in bitcoin/types.go) will need to be created, getblock "blockhash" true/1 returns an array of transaction id strings, whereas getblock "blockhash" 2 returns an array of transaction objects. GetBlock will need to send a getrawtransaction request for each transaction id in the array to get the transaction data required to populate the Txs slice in the Block structure.

getrawtransaction is unable to retrieve the genesis block coinbase transaction, we would have to hard code a response, which isn't ideal. Alternatively we could decode the raw block data (getblock "blockhash" false) and use the decoded data to populate block and transaction structures. Until a new dogecoin core with an updated rpc interface is released, using the raw block data seems to be the most efficient solution.

getrawtransaction is unable to retrieve the genesis block coinbase transaction

Hi.  Could you post the error stack trace you are getting please?  Thanks.

getrawtransaction is unable to retrieve the genesis block coinbase transaction

Hi.  Could you post the error stack trace you are getting please?  Thanks.

Think we got it actually. 🙂

getrawtransaction is unable to retrieve the genesis block coinbase transaction

Hi.  Could you post the error stack trace you are getting please?  Thanks.

Think we got it actually. 🙂

I have no doubt of that.  I was just curious to see what that error looks like so I could compare it with errors I get and hopefully learn more about the project :)

getrawtransaction is unable to retrieve the genesis block coinbase transaction

Hi.  Could you post the error stack trace you are getting please?  Thanks.

Think we got it actually. 🙂

I have no doubt of that. I was just curious to see what that error looks like so I could compare it with errors I get and hopefully learn more about the project :)

This was the getrawtransaction error:

Hi! Just wondering given the current status an estimate of finishing coding?

@victorsk2019 the genesis block is hard coded, so by design getrawtransaction is unable to get the coinbase transaction from the genesis block. What we did was use getblock "hash" false to get the raw blocks which includes the transaction data/bytes, and then used decoderawtransaction to decode each transaction. It's definitely not the most efficient solution, but I figure the next dogecoin core release will have an updated rpc interface, and the solution will no longer be necessary.

@victorsk2019 the genesis block is hard coded, so by design getrawtransaction is unable to get the coinbase transaction from the genesis block. What we did was use getblock "hash" false to get the raw blocks which includes the transaction data/bytes, and then used decoderawtransaction to decode each transaction. It's definitely not the most efficient solution, but I figure the next dogecoin core release will have an updated rpc interface, and the solution will no longer be necessary.

Credit to @RichDevX for getBlock func decoding the genesis block transaction lines 383-453 in dogecoin/client.go.  Different variables in my branch but same function.
And on intermediary block struct to hold raw transaction in string array before decoding and splicing into *Transaction array in Block struct response.  Lines 224-269 and 388-408 in dogecoin/types.go.

@victorsk2019 the genesis block is hard coded, so by design getrawtransaction is unable to get the coinbase transaction from the genesis block. What we did was use getblock "hash" false to get the raw blocks which includes the transaction data/bytes, and then used decoderawtransaction to decode each transaction. It's definitely not the most efficient solution, but I figure the next dogecoin core release will have an updated rpc interface, and the solution will no longer be necessary.

Credit to @RichDevX for getBlock func decoding the genesis block transaction lines 383-453 in dogecoin/client.go. Different variables in my branch but same function.
And on intermediary block struct to hold raw transaction in string array before decoding and splicing into *Transaction array in Block struct response. Lines 224-269 and 388-408 in dogecoin/types.go.

Thank you for clarifying.  It makes it easier for me to spot changes rather than comparing code line-by-line :)

Fixed for v1 blocks with #19


fix: unbundle 2-label metrics

Motivation

On network thread, it takes 3% of cpu time for gauge.inc()

Description

Use single-label metric to improve performance

Closes #426

Codecov Report
Patch coverage has no change and project coverage change: -0.21 :warning:

Comparison is base (324c693) 82.96% compared to head (a21d5eb) 82.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
- Coverage   82.96%   82.75%   -0.21%
==========================================
  Files          48       48
  Lines       11896    11925      +29
  Branches     1282     1282
==========================================
  Hits         9869     9869
- Misses       2027     2056      +29

Impacted Files
Coverage Δ

src/metrics.ts
19.54% <0.00%> (-0.74%)
:arrow_down:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


[pytest-bdd] def browser_type -- getattr(): attribute name must be string

Hi there. Thanks a lot for the plugin!
Would like to try it with pytest-bdd, however got the following error:
    @pytest.fixture(scope="session")
    def browser_type(playwright: Playwright, browser_name: str) -> BrowserType:
>       return getattr(playwright, browser_name)
E       TypeError: getattr(): attribute name must be string

related step definition:
@given("test_playwright")
def test_playwright(page):
    pass

env:
Python: 3.9.0,
Packages: {'pytest': '6.2.2', 'py': '1.10.0', 'pluggy': '0.13.1'},
Plugins:  Faker-6.5.0, progress-1.2.2, base-url-1.4.2, bdd-4.0.2, xdist-2.2.1, metadata-1.11.0, playwright-0.1.1, repeat-0.9.1, forked-1.3.0, allure-pytest-bdd-2.8.36, rerunfailures-10.0
^```

please advise
thanks!

Are you using class-based tests like the unittest.Testcase class? If so, then its a duplicate of https://github.com/microsoft/playwright-pytest/issues/55

Are you using class-based tests like the unittest.Testcase class? If so, then its a duplicate of #55

No, we don't.
Moreover, it works OK with adding to conftest.py browser_type fixture copy with specific browser name:
_conftest.py_

@pytest.fixture(scope="session")
def browser_type(playwright, browser_name: str):
    return getattr(playwright, 'chromium')

looks like it isn't compatible with pytest-bdd where a test consists of a set of steps (special fixtures).
it stops working when 'page' is the argument of a bdd step.

Would it be possible that you create a small reproducible for me in form of a Git repository? I never have used pytest-bdd before and having it directly runable to see the error would be awesome. Thanks!

Would it be possible that you create a small reproducible for me in form of a Git repository? I never have used pytest-bdd before and having it directly runable to see the error would be awesome. Thanks!

sure, please try https://github.com/sqandrew/pytestbdd-pw-test

Thanks! I verified that the fix in #56 works for this case too. Probably pytest_bdd is using class based tests under the hood.

great news! waiting for release

I released version 0.1.2 which should fix it.

it helped, thanks!


CAMEL-18661: Make span current and clear scope properly for async processing

Fixesr https://issues.apache.org/jira/browse/CAMEL-18661
!Looking for input on the overall approach!
This change enables  current span propagation to underlying libraries and end-user-code (for OpenTelemetry):
for all spans created by Camel, it calls into span.makeCurrent()
However, OpenTelemetry (and other tracing tools) rely on ThreadLocal to propagate context. They do it carefully for Executors, Reactor, etc, so it works in async scenarios too.
Instrumentations have to close scope returned by span.makeCurrent() on the same thread where it was created to avoid leaking context (by leaving current span on current thread).
This is easy to guarantee with sync operations in Camel - ExchangeStarted/Sending and ExchangeCompleted/Sent events (where spans start and end) are called on the same thread.
However, for async operations, they are called on different threads.
This change also adds a new event - ExchangeAsyncStarted (naming needs more work), that notifies that Processor.process call has ended, i.e. async operation has started. Tracers would end the scope (but not span) when this event is sent.
Alternative approach:
add a dependency on camel-tracing to camel-base-engine and call into ActiveSpanManager directly:
try(AutoCloseable scope = ActiveSpanManager.makeSpanCurrent(exchange)) {
    sync = processor.process(exchange, async);
}

This would be easier to maintain and would more or less guarantee that scope is disposed on the same thread, however, dependency on tracing might not be desirable.
Traces with this change
(tick/testme spans come from Camel and were previously not correlated with ServiceBus spans coming from Azure SDK)

General question: is there a good place/way to wrap callback executions (user or 3rd party library) with something like
try(AutoCloseable scope = ActiveSpanManager.makeSpanCurrent(exchange)) {
    sync = processor.process(exchange, async);
}

Sorry for not giving this PR attention - however we have been buys (as usual).
Observability with open-telemtry and micrometer is important and thanks for helping with this.

I'm sorry, too. It was on my to-do list to review this and I forgot about it

@davsclaus @oscerd thank you! Once you folks have time to take a look, please let me know what approach you'd like me to pursue nd I'm happy to work on it further.

@davsclaus @oscerd friendly ping to take a look. I'd be happy to continue working on it based on your initial feedback.

add a dependency on camel-tracing to camel-base-engine and call into ActiveSpanManager directly:

This is not possible, camel-base should be 100% independent on camel itself only.

Its actually the opposite

var res = producer.process(exchange, ac);
if the returned value == true then it was a sync call and its done

if the returned value == false, then its async call, and currently processing by another thread (done when the callback is triggered)

@davsclaus @oscerd  thank you for the feedback. I've implemented the current approach with the extra event and added tests. Can you please take another look?

Hi @lmolkova
Thanks for your contribution and support to look into this issue. Looking into build.log, this is the reason why it has failed:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:testCompile (default-testCompile) on project camel-tracing: Compilation failure: Compilation failure:
[ERROR] /home/runner/work/camel/camel/components/camel-tracing/src/test/java/org/apache/camel/tracing/ActiveSpanManagerTest.java:[45,23] error: cannot find symbol
[ERROR]   symbol:   method isCurrent()
[ERROR]   location: variable span of type SpanAdapter
[ERROR] /home/runner/work/camel/camel/components/camel-tracing/src/test/java/org/apache/camel/tracing/ActiveSpanManagerTest.java:[49,24] error: cannot find symbol
[ERROR]   symbol:   method isCurrent()
[ERROR]   location: variable span of type SpanAdapter
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:testCompile (default-testCompile) on project camel-tracing: Compilation failure
	at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:375)
	at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute(MojoExecutor.java:351)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:215)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:171)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:163)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
	at org.mvndaemon.mvnd.builder.SmartBuilderImpl.buildProject(SmartBuilderImpl.java:178)
	at org.mvndaemon.mvnd.builder.SmartBuilderImpl$ProjectBuildTask.run(SmartBuilderImpl.java:198)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: org.apache.maven.plugin.compiler.CompilationFailureException: Compilation failure
	at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1310)
	at org.apache.maven.plugin.compiler.TestCompilerMojo.execute(TestCompilerMojo.java:183)
	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
	at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:370)
	... 12 common frames omitted

Please note that you don't have access to the pipeline's generated build.log but I think you should be able to reproduce it locally.

Using the event notifier to emit a "async processing" event is tricky as there are potentially more places this can happens in the core routing engine. However they all bubble up to the base-engine in the CamelInternalProcessor, especially for spans that would call external systems via "SendProcessor".

@bvahdat  thank you! it should be fixed now.
@davsclaus I removed notifications from everywhere except CamelInternalProcessor and yes, everything seems to be working.
Do you have in mind any scenarios I can run to validate to make sure I didn't miss something? I tried to cover what I can think of in CurrentSpanTests.

@lmolkova have you tested your latest code with the AWS servicebus to see if the diagram/span looks correct

@davsclaus, yes, I did (with Azure ServiceBus ) and yes, it works fine and looks the same as in the description.

I can't repro build error locally and would appreciate if someone can post log from the pipeline run. Thanks!

I can't repro build error locally and would appreciate if someone can post log from the pipeline run. Thanks!

Locally I get the following test failures:
[ERROR] Failures:
[ERROR] org.apache.camel.management.AddEventNotifierTest.testAddAndRemove
[ERROR]   Run 1: AddEventNotifierTest.testAddAndRemove:70 expected: <8> but was: <11>
[ERROR]   Run 2: AddEventNotifierTest.testAddAndRemove:70 expected: <8> but was: <11>
[ERROR]   Run 3: AddEventNotifierTest.testAddAndRemove:70 expected: <8> but was: <11>
[INFO]
[ERROR] org.apache.camel.management.JmxNotificationEventNotifierTest.testExchangeDone
[ERROR]   Run 1: JmxNotificationEventNotifierTest.testExchangeDone:80 Get a wrong number of events ==> expected: <8> but was: <11>
[ERROR]   Run 2: JmxNotificationEventNotifierTest.testExchangeDone:80 Get a wrong number of events ==> expected: <8> but was: <11>
[ERROR]   Run 3: JmxNotificationEventNotifierTest.testExchangeDone:80 Get a wrong number of events ==> expected: <8> but was: <11>
[INFO]
[ERROR] org.apache.camel.management.JmxNotificationEventNotifierTest.testExchangeFailed
[ERROR]   Run 1: JmxNotificationEventNotifierTest.testExchangeFailed:107 Get a wrong number of events ==> expected: <4> but was: <6>
[ERROR]   Run 2: JmxNotificationEventNotifierTest.testExchangeFailed:107 Get a wrong number of events ==> expected: <4> but was: <6>
[ERROR]   Run 3: JmxNotificationEventNotifierTest.testExchangeFailed:107 Get a wrong number of events ==> expected: <4> but was: <6>
[INFO]
[ERROR] org.apache.camel.management.PublishEventNotifierTest.testExchangeDone
[ERROR]   Run 1: PublishEventNotifierTest.testExchangeDone:57->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <9> but was: <12>
[ERROR]   Run 2: PublishEventNotifierTest.testExchangeDone:57->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <9> but was: <12>
[ERROR]   Run 3: PublishEventNotifierTest.testExchangeDone:57->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <9> but was: <12>
[INFO]
[ERROR] org.apache.camel.management.PublishEventNotifierTest.testExchangeFailed
[ERROR]   Run 1: PublishEventNotifierTest.testExchangeFailed:72->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <5> but was: <7>
[ERROR]   Run 2: PublishEventNotifierTest.testExchangeFailed:72->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <5> but was: <7>
[ERROR]   Run 3: PublishEventNotifierTest.testExchangeFailed:72->ContextTestSupport.assertMockEndpointsSatisfied:376 mock://event Received message count. Expected: <5> but was: <7>
[INFO]
[ERROR] org.apache.camel.management.RemoveEventNotifierTest.testRemove
[ERROR]   Run 1: RemoveEventNotifierTest.testRemove:69 expected: <17> but was: <20>
[ERROR]   Run 2: RemoveEventNotifierTest.testRemove:69 expected: <17> but was: <20>
[ERROR]   Run 3: RemoveEventNotifierTest.testRemove:69 expected: <17> but was: <20>
[INFO]
[INFO]
[ERROR] Tests run: 310, Failures: 6, Errors: 0, Skipped: 1

Can you try running all the tests starting from the core folder?

also run component/camel-spring-xml as it has a bunch of tests and some about event notifier

@bvahdat thanks! @davsclaus working on it and should will be able to fix all failures ASAP.

[ERROR] Failures:
[ERROR] org.apache.camel.management.RemoveEventNotifierTest.testRemove
[ERROR]   Run 1: RemoveEventNotifierTest.testRemove:79 expected: <17> but was: <20>
[ERROR]   Run 2: RemoveEventNotifierTest.testRemove:79 expected: <17> but was: <20>
[ERROR]   Run 3: RemoveEventNotifierTest.testRemove:79 expected: <17> but was: <20>

The test class needs to be renamed,
CurrentSpanTests -> CurrentSpanTest
Then when you run tests it will fail (also by CI as it was not run before)
[ERROR] Errors:
[ERROR] org.apache.camel.opentelemetry.CurrentSpanTest.testContextDoesNotLeak
[ERROR]   Run 1: CurrentSpanTest.testContextDoesNotLeak » CamelExecution Exception occurred dur...
[ERROR]   Run 2: CurrentSpanTest.testContextDoesNotLeak » CamelExecution Exception occurred dur...
[ERROR]   Run 3: CurrentSpanTest.testContextDoesNotLeak » CamelExecution Exception occurred dur...

@lmolkova okay so there is a simple test error in core/camel-management
And then there is a test error in your new test class (need to rename the class)
                        // context is cleaned up
                        assertFalse(Span.current().getSpanContext().isValid());

This ^^^ fails

It happens when its the JDK fork-join pool that are in use
pool-17-thread-4 -> Valid context: false
pool-17-thread-8 -> Valid context: false
pool-17-thread-2 -> Valid context: false
pool-17-thread-9 -> Valid context: false
pool-17-thread-1 -> Valid context: false
Camel (camel-8) thread #11 - Delay -> Valid context: false
ForkJoinPool.commonPool-worker-5 -> Valid context: true

thanks a lot, @davsclaus ! I was able to reproduce testContextDoesNotLeak locally with Java 11 (does not repro with 17 I used before)  and working on fixing it.   I'll try my best to fix it today.

Thanks for looking into this. Yeah I only tried with JDK11, as I use that primary due to Camel v3 is still Java 11 based.
Could be that the JDK ForJoin pool behaves differently or something?

@lmolkova by any chance, could you please rebase on top of the main branch when you are done?

Yeah in the mean time we have upgraded: upgrade opentelemetry to the version 1.21.0

@bvahdat sure, I rebased!
I believe (again) that I fixed tests. There were two issues:

We still need to clean the context in SendProcessor:  since we have multiple nested spans, we need to clean scope after them.  I think I found all the new event-related tests.
Test was not correct: sometimes aggregation can be scheduled on the same thread on which outer span is still active - it's ok and if we had OTel executor instrumentation enabled, it'd populate this context on other threads too. Added check and description to the test. This behavior was different on Java 11 and Java 17 and affected Java 11 much more. It's stable now on both versions.

thanks @lmolkova and camel team!!


Digest updates need uniqueness

Existing digest updates are blocking future ones because title and branch name are the same.
We should not recreateClosedPRs because there might be bad updates that users want to ignore.
Suggest to add short hash to title

Thanks for creating this @rarkins, just referencing my tweet here in case it's useful.

@renovateapp Just got "Renovate Ignore Notification" for a merged PR, is that a bug? https://github.com/LearningLocker/xapi-service/pull/13#issuecomment-339817150

https://twitter.com/tweetRyanSmith/status/923918141939421184

@ryansmith94 now fixed and new Dockerfile commit already made to your repo: https://github.com/LearningLocker/xapi-service/commit/b25b4a4394e58d13492718e771699563a03b2e50
Thanks for reporting the bug

That's awesome! Thanks for quick fix @rarkins :smile:
No worries.

Hey @rarkins I've just seen this issue again https://github.com/LearningLocker/xapi-agents/pull/17#issuecomment-341690502

@ryansmith94 thanks for bringing it to my attention. This time it looks like probably a bug:

I see a little more what's going on:

The PR title got updated twice to match new digests, but the actual commit itself failed to update.

@ryansmith94 this will be fixed shortly, however you will need to rename that existing PR yourself in order to get the digest updated. You can do that already though.

Hey @rarkins, you're welcome for the report. Thanks for the quick fix and reply. Sorry for my slow reply, spent a lot of time pairing yesterday and must have missed this.
What do I need to change the PR title to? I'm guessing I just need to change the hash to be "48d5a6"?

Change it in any way to fool the matching eg add a -

Oh right nice will do, thanks @rarkins. Have a nice weekend 👍


Fix for React Router v6 quirk in @shopify/performance

Description
Recently encountered an issue in the Admin when updating from React Router v5 to v6. We were seeing our metrics reporting substantially lower numbers of navigation_complete metrics where full_page_navigation was set to true, and substantially more of them set to false.
Our calculation for the full_page_navigation tag comes from the Navigation#isFullPageNavigation method of the @shopify/performance library, which in turn is just checking to see if this was the first Navigation object for the top-level Performance object.
The issue here was that in React Router v6, they internally use a replaceState call on initialization in order to set a piece of state that they can use to track the number of navigations: https://github.com/remix-run/react-router/blob/a5e3ca647d733211599310626366e75b1ee228a7/packages/router/history.ts#L606
Within @shopify/performance, we consider any calls to popstate, pushState, or replaceState, to mark the end of the current Navigation object and to start the creation of a new one. The idea here is that if you've got a Navigation that's in progress, but the user changes the state by means of something like clicking a redirect link, then we want to start that new Navigation immediately.
This use of replaceState inside React Router is unfortunate, but required by their team in order to keep track of things for their internal usage. @shopify/performance seeks to be agnostic of any type of router that's in use, but in this case we need to make an exception to account for this behaviour to avoid triggering additional Navigations and breaking our metrics.
In the future, I'll be writing up an issue that details some additional shortcomings around @shopify/performance's inability to support certain patterns that are in use with ReactRouter/Remix such as fetch-then-render, and how we might evolve this library to account for them.

/snapit

Confirmed this behaviour in a spin instance. This should be safe for us to start adopting on react-router v5 and will work forwards in v6


Added git-secrets check to Github Actions

Added git-secrets check to Github Actions
Description
The added Github action will check and report PR check failure if a credential is committed into the repository.
Checklist:

[x ] I have tested my changes. No regression in existing tests.
[x ] My code is Linted.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Since people still seem to be creating branches on this repo, we should probably set the trigger to all branches ('**')

/bot run checks

/bot run checks


Dev branch failing julia 1.7 tests

Although Julia 1.8 and 1.9 tests are passing (see #9) Julia 1.7 tests appear to fail on current dev: #10

@EssamWisam can you please investigate?


Fix some issues with elevation caching and model level dimensions.

Fix #1066

Disagree. Neither of these will fix something meaningful in stable. The first one ("unknown" - "dimensionless") is a behavioural change that "fixes" a check I introduced in develop. Should got to develop.
The second one is in the code since years and hasn't affected us so far. It defers the check on elevation consistency a bit. The gallery code does something different than "normal" usage

ok changed "bug" to "enhancement"


Values with the highest colors are displayed white

I have changed this:
for (int i = 1; i < numberOfColorClasses; i++) {
if (value <= min + (i * classSize)) {
return colorPalette[i];
} else if (value > (classSize * i) && i == numberOfColorClasses - 1) {
return colorPalette.last;
}
}
to this:
for (int i = 1; i < numberOfColorClasses; i++) {
if (value <= min + (i * classSize)) {
return colorPalette[i];
} else if (value > min + (classSize * i) &&
i == numberOfColorClasses - 1) {
return colorPalette.last;
}
}
I think this makes more sense. I think what you are trying to do is that if in the last iteration still nothing is returned, then you just return the last value. Probably you also just write ¨else if (i == numberOfColorClasses - 1) ¨ because the first condition must be true anyway, in case the previous IF does not catch

I am refering to heatmap_painter.dart line 200


Monitoring does not show data from other nodes

Hi,
I am trying to install this stack on my clustered environment, where I have multiple AWS VM's connected on the swarm. I have a swarm manager VM to handle all this.
I am trying to install monitoring on a separate VM and keep all the other things on the existing VM's. For this I have updated the services config in the compose file to be:
deploy:
placement:
constraints:
- node.hostname == monitor
If I do this the monitor services come up fine, but they show the data for the monitor services only. If I remove the constraint, then the monitor data shows up fine.
Is there a way i can separate out the monitor dockers to a separate VM and get the monitoring data for all the services/nodes/dockers?

swarmprom's stack by default creates an overlay network:
docker-compose.yml:
networks:
net:
driver: overlay
attachable: true
To deploy other services or stacks that will be routable/addressable by DNS tasks.name to each other (so that Prometheus can connect to them and scrape them), the other (monitor) node will also need to be a member of the same Docker swarm, so that the overlay network is available to it, and you'll need to define the service or stack to use the existing swarmprom_net:
your-docker-compose.yml on another host in the swarm:
networks:
default:
external:
name: swarmprom_net

If you don't want the constraint of your monitoring host being a member of the swarm, you could change swarmprom to use Docker "host networking", and then containers will be addressable via the Docker host's IP.  Or, you could configure the other swarmprom containers to expose their ports and connect to them via the swarm. Both of these options carry with them the penalty of only one similar service per port per host. When using the default overlay network, each container gets it's own internal swarm IP address on the overlay network and so multiple containers listening on the same port can run on the same host.

This problem took my three days. On all nodes, you must allow the necessary ports that the applications use or "sudo ufw disable".


chore: set dynamicImportInCjs to false for cjs build

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 79.683%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
8103

Relevant Lines:
9204

💛  - Coveralls


Fix refresh on server start

Describe your changes and why you are making these changes
This PR fixes an issue that we couldn't trigger a missed workflow on server start. The issue is introduced as the handler init is out-of-date.
Related issue number (if any)
We should follow up for testing in ENG-1689
Checklist before requesting a review
Workflow trigger failed with nil pointer error on this fix, succeed with this fix.

[x] I have created a descriptive PR title. The PR title should complete the sentence "This PR...".
[x] I have performed a self-review of my code.
[ ] I have included a small demo of the changes. For the UI, this would be a screenshot or a Loom video.
[ ] If this is a new feature, I have added unit tests and integration tests.
[ ] I have run the integration tests locally and they are passing.
[ ] I have run the linter script locally (See python3 scripts/run_linters.py -h for usage).
[ ] All features on the UI continue to work correctly.
[x] Added one of the following CI labels:

run_integration_test: Runs integration tests
skip_integration_test: Skips integration tests (Should be used when changes are ONLY documentation/UI)

@likawind Can we get this merged? Just ran into this bug as well


Add make target for running e2e tests on gke-autopilot

Description
With this the make targets for e2e tests are added. This works only for the following tests:

applicationMonitoring/default
appMon/withoutcsi
appMon/labelversion
appMon/readonlycsi (NOTE: this test will not work with the current version of the operator, as there were problems with the whitelisting of the image and csi Driver initContainer, this will be fixed)

How can this be tested?
Create a gke-autopilot cluster and run the added make targets. But please be aware that the tests will only work after https://github.com/Dynatrace/dynatrace-operator/pull/2098 is merged.
Checklist

[x] PR is labeled accordingly with a single label
[x] I have read and understood the contribution guidelines

Codecov Report

Merging #2099 (4555a17) into main (a6d6a12) will not change coverage.
The diff coverage is n/a.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@           Coverage Diff           @@
##             main    #2099   +/-   ##
=======================================
  Coverage   63.24%   63.24%
=======================================
  Files         242      242
  Lines       15374    15374
=======================================
  Hits         9723     9723
  Misses       4909     4909
  Partials      742      742

:loudspeaker: Have feedback on the report? Share it here.

IMO, we can merge the change because make targets are correct.
Cc @luhi-DT, issues with tests themselves we can improve in follow-up PRs.

I would agree and I can btw confirm that the tests run as before when using an older agent version.