fixed viewport constraint behavior when viewer is rotated

Related to https://github.com/openseadragon/openseadragon/pull/2041, fixes https://github.com/openseadragon/openseadragon/issues/2040
As far as I can tell, this works as expected with different values for visibilityRatio, viewport rotations, etc. I don't see a reason it should break anything, but... it would be good to have others test it out, of course.
One thing I haven't changed, but I'm wondering about, is why the _raiseConstraintsEvent() is called every time applyConstraints() is called, even if no constraints were actually applied... is this desirable? It basically means every navigation event raises the constrain event, even if no boundary was hit.

@iangilman I updated the relevant unit tests here in this PR. As far as I can tell, they are now working correctly. There was another test - I think TileSource.animate - that seemed to be failing because of floating-point issues. The test does not have epsilon values included for the asserts; I tweaked the implementation of Viewport._applyBoundaryConstraints to return the unmodified bounds if no constraints were hit, which should avoid small errors introduced by rotating and un-rotating the bounds. Perhaps eventually it'd be worth updating this unit test to allow floating point errors to pass the test though.

Excited that you took this on! The solution is working nicely in my testing, looking into one small quirk with zooming into the corner areas when rotated 90 degrees where the zoom goes in the wrong direction but I'll try again with an isolated test outside of my actual application, it's quite possible that this issue only affects my setup.

@pearcetm:

One thing I haven't tested out yet, because I really don't know what the intended use-case is, is the block of code at line 685 in _fitBounds when the constraints flag in the options argument is true. I don't think this should break anything that was previously working (since I don't think it was ever really working for non-zero rotations), but it would be good to test out and fix any bugs if needed...

Cool... That's called from fitBoundsWithConstraints which is there in case you want to enforce the constraints when you have it zoom/pan into a specific location. Should be pretty easy to set up a test for that... Let me know if you want pointers.
@AaronPlave:

Excited that you took this on! The solution is working nicely in my testing, looking into one small quirk with zooming into the corner areas when rotated 90 degrees where the zoom goes in the wrong direction but I'll try again with an isolated test outside of my actual application, it's quite possible that this issue only affects my setup.

Wonderful! Thank you for testing. It would be great to hear if that issue you found is something we need to address.

@iangilman Thanks for pointing me in the right direction to look into fitBoundsWithConstraints. I found an old demo which appears that it probably hadn't worked in a long time, but it was a good starting point. Once I updated the demo (which can be loaded from http://localhost:8000/test/demo/fitboundswithconstraints.html when running the development environment), it was apparent that there were definitely problems with that functionality.
I think the changes I've now included have everything working right. I'd appreciate if you could take a look (and @AaronPlave too if possible).

@iangilman Good point; the list was a carryover from the original demo page. I've updated the demo, and the comment for getConstrainedBounds

Sorry about the delay, managed to reproduce this relatively minor issue on the demo page of this branch. Not sure if this issue is something introduced by this PR since one couldn't test this exact functionality on the master branch due without the fixes in this PR. I can't seem to upload any file to github PR comments right now so unfortunately need to use a temporary video link https://tempclip.com/en/E8NihPy9oPpu4XW/watch for the screencap of the issue. But to repro locally you can go to the fitBoundsWithConstraints demo, zoom in to the top left corner, rotate the image by 90º, and then zoom in to the top left corner (might have to hide the control buttons). The zoom in the top left corner on the rotated image does not zoom into the cursor and goes off in a different direction.

@AaronPlave Sure enough! Thank you for catching this! I can reproduce it, and I've tried it without the visibilityRatio: 1.0 to validate that it is the constraints that are causing the problem.
@pearcetm Can you take a look?

Thanks for finding this @AaronPlave.
This is weird. I've done some digging, but I haven't sorted out yet what's going on.
It seems that when degrees==0, viewportToViewerElementRectangle(bounds) returns a $.Rect with x==0, y==0, width==800, height==500 during all boundary checks, as I think it should. For non-zero rotations, though, the bounds rectangle when converted to viewer element x, y coordinates are NOT 0, 0 during animations. After a pan animation finishes, it returns to (close to) 0, 0.
I'm not sure what to make of this yet. It doesn't seem right, though, and may indicate a deeper issue with how coordinate frames/transformations are implemented. @iangilman any thoughts? I'll keep digging in the meanwhile.

I think I figured it out - it seems that rotation was not being taken into account within Viewport.getCenter() which was causing all sorts of weirdness. It seems to be working now, but please test! Hopefully fixing this doesn't break anything else that relies on getCenter()...

@iangilman I am wondering if there are other places that rotated viewports aren't handled correctly (or handled at all), especially during spring-based animations. During animating rotation itself, for example... I'll take a look but thought you might have some good ideas of potential problem areas.

@pearcetm Looking good to me! I'm not surprised there are places like that that aren't taking rotation into account. I'm not sure what to look for in terms of negative effects. I did a search for getCenter in the code, and of course it's used in a lot of places (both directly and indirectly). The fact that the change hasn't broken any tests seems like a good sign.
@AaronPlave It looks like the bug is fixed; do you want to take another look?

@AaronPlave Not to be a pest, but it would be great if you could take a look at the latest to verify that it fixes the issue you found! I'm hoping to merge this patch this week so we can do a new release :)

I'll take a look now! I need to sort out my github notifications, this keeps getting lost in the noise.

@iangilman @pearcetm the fix looks good to me, nicely done! Sorry for the hold up.

@AaronPlave Awesome, thank you for helping with this!
@pearcetm Thank you for making this happen!


Add conflict error when create an exist volume with a different driver

Signed-off-by: Lei Jitang<EMAIL_ADDRESS>When create a volume and the volume exist,
but the volume driver is different, we should throw a
conflict error.
For example
$ docker run -ti -v data:/data busybox
this will create a volume data with driver local
and then
docker run -ti --volume-driver convoy -v data:/data busybox
user want to create a volume data with driver convoy
actually, the volume data is exist with driver local,  but
docker run -ti --volume-driver convoy -v data:/data busybox
just run without any error or warning, so the user just think the
volume data is using driver  convoy. This could lead some problem.
With this patch, this will show a error
Error response from daemon: Conflict: volume 'data' exist with volume diver 'local'
ping @cpuguy83

@coolljt0725 is this solving the same issue as #16009?

@runcom  Yes, sorry didn't aware there is a similar fix. But this PR solve volume create issue  both use docker volume create and docker run/create

I'm pretty sure the other PR is doing that as well https://github.com/docker/docker/pull/16009#discussion_r38531165

@runcom  no, #16009 just solve the issues docker volume create but not docker run/create. I build that branch and have a try.
$ docker volume create --name test
test
$ docker volume create --name test -d convoy
Error response from daemon: Conflict. Cannot create volume 'test' using the 'convoy' driver because the volume 'test' already exists and the driver name is 'local'.
$ docker run -ti --volume-driver convoy -v test:/data busybox
/ # exit
$
@cpuguy83's comment is to make docker run/create create a exist volume on different work,
but I don't know why we should make it work, I think we should also throw a conflict error.

@coolljt0725 sorry but I'll close this PR since it's a duplicate of #16009 which was opened before and has tests and documentation update. Thanks for your contribution. I'll ask @azurezk to use your fix.


Add TSDF RangeDataInserter

Adds TSDF RangeDataInserter, tests and integrates the configuration files.

@wally-the-cartographer merge

Merge requested by authorized user kdaun. Merge queue now has a length of 3.

@wally-the-cartographer merge

Merge requested by authorized user kdaun. Merge queue now has a length of 5.

@wally-the-cartographer merge

Merge requested by authorized user cschuet. Merge queue now has a length of 6.

Error: exit status 1
Log:
=> handle work item: PR 1236 on googlecartographer/cartographer.
 => Switching to branch master.
 $ git reset --hard HEAD [data/cartographer]
HEAD is now at 7dfe4042 Add TSDF RangeDataInserter (#1236)
 $ git clean -fd [data/cartographer]
 $ git checkout master [data/cartographer]
Already on 'master'
Your branch is up to date with 'origin/master'.
 $ git pull [data/cartographer]
From https://github.com/googlecartographer/cartographer
   7dfe4042..eff103f6  master     -> origin/master
Updating 7dfe4042..eff103f6
Fast-forward
 cartographer/cloud/internal/handlers/add_trajectory_handler.cc      | 3 +++
 cartographer/cloud/internal/handlers/add_trajectory_handler_test.cc | 2 ++
 2 files changed, 5 insertions(+)
Current branch master is up to date.
 => Checking out kdaun/integrate_range_data_inserter as local branch pr_1236.
 $ git remote add kdaun git@github.com:kdaun/cartographer.git [data/cartographer]
fatal: remote kdaun already exists.
 $ git fetch kdaun [data/cartographer]
 $ git branch --track pr_1236 kdaun/integrate_range_data_inserter [data/cartographer]
Branch 'pr_1236' set up to track remote branch 'integrate_range_data_inserter' from 'kdaun'.
 => Switching to branch pr_1236.
 $ git reset --hard HEAD [data/cartographer]
HEAD is now at eff103f6 Ignore initial poses on gRPC server side. (#1234)
 $ git clean -fd [data/cartographer]
 $ git checkout pr_1236 [data/cartographer]
Switched to branch 'pr_1236'
Your branch is up to date with 'kdaun/integrate_range_data_inserter'.
 $ git pull --force [data/cartographer]
Already up to date.
Current branch pr_1236 is up to date.
 => merge master.
 $ git merge master [data/cartographer]
Merge made by the 'recursive' strategy.
 cartographer/cloud/internal/handlers/add_trajectory_handler.cc      | 3 +++
 cartographer/cloud/internal/handlers/add_trajectory_handler_test.cc | 2 ++
 2 files changed, 5 insertions(+)
 => Running clang-format on all *.cc and *.h files
 $ git commit -am Ran clang-format. [data/cartographer]
On branch pr_1236
Your branch is ahead of 'kdaun/integrate_range_data_inserter' by 3 commits.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
 $ git push --force kdaun HEAD:integrate_range_data_inserter [data/cartographer]
To github.com:kdaun/cartographer.git
 ! [remote rejected]   HEAD -> integrate_range_data_inserter (permission denied)
error: failed to push some refs to 'git@github.com:kdaun/cartographer.git'
 => Abandoning branch pr_1236.
 => Switching to branch master.
 $ git reset --hard HEAD [data/cartographer]
HEAD is now at 997268be Merge branch 'master' into pr_1236
 $ git clean -fd [data/cartographer]
 $ git checkout master [data/cartographer]
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
 $ git branch -D pr_1236 [data/cartographer]
Deleted branch pr_1236 (was 997268be).

Current branch master is up to date.
 => Checking out kdaun/integrate_range_data_inserter as local branch pr_1236.
 $ git remote add kdaun git@github.com:kdaun/cartographer.git [data/cartographer]
fatal: remote kdaun already exists.
 $ git fetch kdaun [data/cartographer]
 $ git branch --track pr_1236 kdaun/integrate_range_data_inserter [data/cartographer]
Branch 'pr_1236' set up to track remote branch 'integrate_range_data_inserter' from 'kdaun'.
 => Switching to branch pr_1236.
 $ git reset --hard HEAD [data/cartographer]
HEAD is now at eff103f6 Ignore initial poses on gRPC server side. (#1234)
 $ git clean -fd [data/cartographer]
 $ git checkout pr_1236 [data/cartographer]
Switched to branch 'pr_1236'
Your branch is up to date with 'kdaun/integrate_range_data_inserter'.
 $ git pull --force [data/cartographer]
Already up to date.
Current branch pr_1236 is up to date.
 => merge master.
 $ git merge master [data/cartographer]
Merge made by the 'recursive' strategy.
 cartographer/cloud/internal/handlers/add_trajectory_handler.cc      | 3 +++
 cartographer/cloud/internal/handlers/add_trajectory_handler_test.cc | 2 ++
 2 files changed, 5 insertions(+)
 => Running clang-format on all *.cc and *.h files
 $ git commit -am Ran clang-format. [data/cartographer]
On branch pr_1236
Your branch is ahead of 'kdaun/integrate_range_data_inserter' by 3 commits.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
 $ git push --force kdaun HEAD:integrate_range_data_inserter [data/cartographer]
To github.com:kdaun/cartographer.git
 ! [remote rejected]   HEAD -> integrate_range_data_inserter (permission denied)
error: failed to push some refs to 'git@github.com:kdaun/cartographer.git'
 => Abandoning branch pr_1236.
 => Switching to branch master.
 $ git reset --hard HEAD [data/cartographer]
HEAD is now at fb2e85d9 Merge branch 'master' into pr_1236
 $ git clean -fd [data/cartographer]
 $ git checkout master [data/cartographer]
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
 $ git branch -D pr_1236 [data/cartographer]
Deleted branch pr_1236 (was fb2e85d9).


Researcher Profile binding with a configurable field instead of email

Is your feature request related to a problem? Please describe.
As described in the documentation:
https://wiki.lyrasis.org/display/DSDOC7x/Researcher+Profiles
the field that enable us to bind an eperson with a person entity is the email address. But, according to  GDRP a person's email is sensitive data, and may not exist. IMHO, other field could be configured to be used to make the ownership matching.
Describe the solution you'd like
Since both typed items and epersons are DSpace Objects (DSO), both have metadata. A new researcher profile configuration, perhaps based on spring that could be used to specify which fields to be used for binding each side (entity type Person and an eperson).
Than, all instances of email calls, like this:
https://github.com/DSpace/DSpace/blob/main/dspace-api/src/main/java/org/dspace/profile/ResearcherProfileServiceImpl.java#L205-L208
https://github.com/DSpace/DSpace/blob/main/dspace-api/src/main/java/org/dspace/profile/ResearcherProfileServiceImpl.java#L352-L357
should be changed in order to use the configured field instead of the direct access.

Seems like a reasonable idea, but would have to wait for a major release.  May also have some overlap with future discussions about "ORCID Login improvement" which 4Science is looking to bring to DSpace Developer Meetings later in July.  If I recall correctly, part of the idea is to allow for a link between ORCID profile and DSpace Researcher Profile which is not reliant on email address... so it might solve this ticket.
In any case, pulling over to the 8.0 board for tracking. Needs discussion & volunteer.


Add initial files

🤔 Well, it created a tag from this PR even though I specified on: push: [ main ] so I guess I have some stuff to figure out lol


I've implemented a smallish bot using the lib that's supposed to respond to messages.
But after running for a while all incoming messages are processes, but once it tries to send a message I hit the following:

Unexpected: write tcp <IP_ADDRESS>:58189-><IP_ADDRESS>:443: i/o timeout
Unexpected: write tcp <IP_ADDRESS>:58189-><IP_ADDRESS>:443: i/o timeout
nlopes/slack2016/09/01 12:55:45 slack.go:86: Sending PING  20
nlopes/slack2016/09/01 12:55:45 slack.go:80: RTM Error sending 'PING 20': write tcp <IP_ADDRESS>:58189-><IP_ADDRESS>:443: i/o timeout
nlopes/slack2016/09/01 12:55:45 slack.go:86: killing connection
Unexpected: &{false}
Unexpected: &{1 3}
Unexpected: read tcp <IP_ADDRESS>:58189-><IP_ADDRESS>:443: use of closed network connection

Message is also not being re-sent on reconnect. Any ideas what I'm doing wrong? I saw that sendOutgoingMessage doesn't handle connection drops, but...

Same issue here, when sending a message through RTM with the the following code:
rtm.SendMessage(rtm.NewOutgoingMessage(mytext, IDchannel))

The RTM event I receive:
event.type: "outgoing_error"
event.data: write tcp <IP_ADDRESS>:57025-><IP_ADDRESS>:443: i/o timeout

Worked fine before, but started a few days ago. This code:
rtm.SendMessage(rtm.NewTypingMessage(event.Channel))

now results in i/o timeout and disconnect:
OutgoingError: write tcp 172.*.*.*:41407-><IP_ADDRESS>:443: i/o timeout
Disconnected: intentional false
IncomingError: read tcp 172.*.*.*:41407-><IP_ADDRESS>:443: use of closed network connection

Looks like RTM hits some new rate limits, and Slack closes connection. I had to turn off typing notifications. api.PostMessage works fine.

So I'm using this wrapper for the bot, which uses rtm.SendMessage: https://github.com/BeepBoopHQ/go-slackbot/blob/master/bot.go#L116, this one times out regularly. @ignat you're saying if I replace that with api.PostMessage it would work?
That's cool, I'm just curious what we can do for this lib to fix RTM as well.

I think this is related with https://github.com/nlopes/slack/commit/b7940f209838f93dc7ab0d823145513c9660f22a.
I'm reverting until I have some time to look into these issues.

Reverted branch works well indeed.

I can confirm that this commit caused the problem: https://github.com/nlopes/slack/commit/89559108ff024e2559376013eacd57e2829b7bd6
Also, it seems that we're attempting to set the write deadline on the PING only where in actuality when using SetWriteDeadline it affects any writes thereafter on the current connection.
So there are two issues:

the one second deadline is too short
SetWriteDeadline should probably be moved to ManageConnection()

I just submitted this PR #102 to resolves these issues.


prevent nullable types with falsify values being used directly in conditional statement

Rule Suggestion
Is your rule for a general problem or is it specific to your development style?
general
What does your suggested rule do?
let foo: number?;
if (a) {
  bar();
} else {
  qux();
}

qux is called both when a is zero or a is undefined.
But in many cases(at least in my experience), two cases should be treated differently, like
if (a === undefined) {
  bar();
} else if (a === 0) { // it's not mandatory!
  baz();
} else {
  qux();
}

I think the new rule that forces programmers to write more explicit conditions for nullable types with its own falsify value will reduce mistakes (which I've been made).
We might need to deal only with number, string, bigint, boolean, while only they have falsify values.
List several examples where your rule could be used
I felt hard to explain the rule without an example, so I gave one above already.
I think other examples would be virtually the same as it.
Additional context

Hi @worudso - the strict-boolean-expressions intends to solve that very issue. Does that work for you?

Closing for inactivity. Do speak up if there's more context that should be posted!


feat: add gitignore

This change adds a '.gitignore' file which excludes the binaries generated for the web app and the demo app.

Yep, good thing to include into repo :)
Thank you very much!


ENH: Allow versioneer to be used as a module in PEP-518 mode

As https://github.com/python-versioneer/versioneer-518 proves, the only thing preventing versioneer to be used in a non-vendored fashion and consistent with PEP-518 is making the variables available in the vendored module available via import versioneer.
This will not prevent vendored installations but may lead to silent interference. I also haven't added a non-vendored versioneer setup command, as I haven't evaluated what all the vendored version does that would be useful in a non-vendored context.
Others' thoughts would be welcome (@bucknerns?).
Closes #193.

The local versioneer.py will take precedence over site packages version so it won't affect any vendored installs. If that is not the case I think having versioneer installed would prevent building vendored versioneer packages since the module doesn't have the get_version call.

I think that is sometimes the case, which leads some projects to add sys.path.insert(0, ".") to their setup.py. I don't fully understand why it seems to happen to some projects and not others.

@jwodder - would you be so kind to provide a review on this PR? My foo in Python build/dist/*-utils and corresponding PEPs is not that strong so can't really assess possible implications.

After letting this rest for a bit, I tried using it on a new repository and found this wasn't as ready as I remembered. Got around to adding versioneer setup to the base command that skips the versioneer install (vendoring) phase. Also added a test that demonstrates that it can work in python -m build's isolated build process. This required a hack to inject the current repository directory as versioneer @ file:///....
Will rebase and update the docs after #188.

I think we might want to consider the CLI a bit before pushing this out. Right now the installation process is:
versioneer install          # Vendors versioneer then runs setup
python versioneer.py setup  # Rerun setup from vendored versioneer

I'm currently proposing that this be possible to replace with versioneer setup or python -m versioneer setup which will skip the installation step and always use the environment-installed versioneer. This double-use of setup, even though it means the same thing, might hide when someone is accidentally invoking the environment versioneer instead of the vendored versioneer, potentially leading to subtle bugs. I think instead we want to revert to versioneer install for the main command and versioneer.py setup for the vendored module, and add a --vendor/--no-vendor flag.
versioneer install              # Vendors versioneer then runs setup
versioneer install --vendor     # Same as default
versioneer install --no-vendor  # Non-vendored setup

I will push this soon, but adding a comment to explain my reasoning.

Took a break to come back to this with fresh eyes. On the whole, I'm happy with it. Would anybody be interested in reviewing and/or test-driving? @bucknerns @yarikoptic?

I'm checking it out

@bucknerns FWIW I currently have to do this to get python -m build to work with isolation:
[build-system]
requires = [
    "setuptools",
    "versioneer @ file:///data/projects/versioneer/",
    "wheel"
]
build-backend = "setuptools.build_meta"

Once there's a release, it shouldn't be necessary. And I think python -m build --no-isolation should work without the @.

I'm checking it out

@bucknerns Any news?

Looks good, I pushed a copy to test.pypi.org and added to a pyproject.toml and did a pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ ./exampleproject  of an example project to test it out.

fyi had to version lock it in the pyproject.toml but should be good on official version "versioneer==0.24.dev0"

Awesome! Thanks for the review. I'll merge now. I can write some release notes and pushing 0.24 to PyPI today or tomorrow, but feel free to go ahead and do it yourself.


Method for neovim-gnuplot highlight using your /syntax/gnuplot.vim file

In fact, your effect for highlight of gnuplot files in neovim is almost done. Here is what I do.

grab your /syntax/gnuplot.vim file to .config/nvim/syntax, intact.

create a file .config/nvim/filetype.vim, and the content should be like:

if exists("did_load_filetypes")
finish
endif
augroup filetypedetect
au! BufNewFile,BufRead .plt,.plot,.gnuplot,.gnu,*.gp setf gnuplot
augroup END
The method is the manual approach C of neovim document: new-filetype in https://neovim.io/doc/user/filetype.html#new-filetype

try any .gp file. I works for me now

You're right, in fact I've migrated to lua and added this to my config:
vim.filetype.add({
  filename = {
    ["gnuplotrc"] = "gnuplot",
  },
  pattern = {
    ["${XDG_CONFIG_HOME}/gnuplot/Palettes/.*%.pal"] = "gnuplot",
  },
})

local gnuplot = { "plt", "plot", "gnuplot", "gnu", "gp" }
for _, ext in ipairs(gnuplot) do
  vim.filetype.add({ extension = { [ext] = "gnuplot" } })
end

On other note I haven't done much to this repo since I'm now using a gnuplot syntax I made for treesitter. Before I knew what treesitter was I tried to to the same effect using regex which is really slow.
Maybe in the future I'll update this repo for gnuplot 6.0 but remove most of the regex nonsense so that it's faster and usable.

Syntax highlight comparison
Default vim/neovim gnuplot syntax file:

gnuplot.vim Is much better at the expense of being painfully slow:

tree-sitter-gnuplot which is simply better in every way:


[NoQA] change webpack config and rename .web files to .website files

Details
Webpack doesn't include .desktop.tsx files in bundle when running desktop app and desktop app wouldn't work if we wanted to write some desktop specific code. This PR fixes it and unblocks this task , that has components that have such .desktop.tsx files. I've also noticed that some of our web specific files have .web suffix and renamed then to have .website suffix for better standarization.
Fixed Issues
$ https://github.com/Expensify/App/issues/24990 related to
PROPOSAL: N/A
Tests

[x] Verify that no errors appear in the JS console
[X] Verify that creating a distance request flow works as expected
[x] Test the app generally, sending messages, logging, requesting money

Offline tests
N/A
QA Steps

[x] Verify that no errors appear in the JS console
[X] Verify that creating a distance request flow works as expected
[x] Test the app generally, sending messages, logging, requesting money

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 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
https://github.com/Expensify/App/assets/88395093/94920187-9dec-4e4e-9ab9-1ad3938835bd

Android: mWeb Chrome
https://github.com/Expensify/App/assets/88395093/2b57fda3-ddd7-4964-8aa9-2ee166ff2157

iOS: Native
https://github.com/Expensify/App/assets/88395093/1948dc7f-6805-49de-8a67-54dd25da1dbc

iOS: mWeb Safari
https://github.com/Expensify/App/assets/88395093/36876384-bfe7-442d-acb0-9103678021be

MacOS: Chrome / Safari
https://github.com/Expensify/App/assets/88395093/cf03d22d-039c-4824-b799-cf70bc56c019

MacOS: Desktop
https://github.com/Expensify/App/assets/88395093/3b61405c-7b09-4d95-a3cb-de6fc4a92135

I have read the CLA Document and I hereby sign the CLA

recheck

@mountiny iOS has failed 🤔

Looks like the iOS build was cancelled without specific reason. Maybe another build will solve it. cc @blazejkustra @mountiny

that probs does not matter here

@SzymczakJ Fill out Tests section please 🙏

Reviewer Checklist

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

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] 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)
[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 checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

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

[x] 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
[x] I verified proper code patterns were followed (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] 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 verified that this PR follows the guidelines as stated in the Review Guidelines
[x] 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)
[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] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] 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)
[x] 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)
[x] All JSX used for rendering exists in the render method
[x] 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

[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 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.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos
Confirmed web and desktop are building correctly, otherwise n/a

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

Will merge this one then

: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.

🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.10-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
cancelled 🔪

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.10-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.10-1 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
cancelled 🔪

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.10-1 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


Protocol Buffer update for release 1.7.x at Fri Oct 30 09:29:00 UTC 2020

Note that there's gonna be some minor changes to this. Will be added to this PR.


Build separate images for web server and celery worker

Creates a separate build stage for web server and celery worker
Moves the CMD block that starts the nginx unit server into the web server build stage, and adds a CMD block for the celery worker
Updates our CD workflow to build docker images for both the web server and celery worker, targeting their respective build stages.

@BryonLewis these are changes needed to accommodate the AWS deployment. Will this interfere with your PRs?

@BryonLewis these are changes needed to accommodate the AWS deployment. Will this interfere with your PRs?

  Do we want to add a rabbitMQ service to the docker-compose to make deployment as similar to local dev as possible?
Also adding the new environment variable to the template.env and docker-compose to pass it into django?

I reverted the changes to the Dockerfile, as I realized we can avoid having separate images for web app + worker. This is advantageous because it will ensure they stay synced up.

@BryonLewis these are changes needed to accommodate the AWS deployment. Will this interfere with your PRs?

 Do we want to add a rabbitMQ service to the docker-compose to make deployment as similar to local dev as possible? Also adding the new environment variable to the template.env and docker-compose to pass it into django?

I added a rabbitmq service to docker-compose, and also updated the template.env.


Upgrading async profiler to 1.8.8

Fixes #2759

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-09-11T07:03:00.885+0000

Duration: 57 min 57 sec

Test stats :test_tube:

Test
Results

Failed
0

Passed
3141

Skipped
36

Total
3177

:green_heart: Flaky test report
Tests succeeded.
:robot: GitHub comments
To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

run benchmark tests : Run the benchmark tests.

run jdk compatibility tests : Run the JDK Compatibility tests.

run integration tests : Run the Agent Integration tests.

run end-to-end tests : Run the APM-ITs.

run windows tests : Build & tests on windows.

run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)


Mark tests as skipped when ch_open encounters E901

On some of the Debian build systems, the IPv6 channel tests fail because ch_open('[::1]:<port>', ...) raises the error "E901: getaddrinfo() in channel_open(): Address family for hostname not supported".
This appears to happen because getaddrinfo() can't perform the reverse lookup for the ::1, which is a config issue on that system.  Therefore, instead of reporting a test failure, mark the test as skipped due to the bad network config.

Thanks, I was first wondering, why CheckIPv6Loopback() didn't catch it, but since this seems to be a system configuration issue I agree it is fine to skip in your case.


Added an item with github source authentication to the dotnet CLI in the instructions for GitHub Packages

Why:
The article itself, although contains information on how to authenticate github source for NuGet integration with GitHub Packages still indicate GitHub Actions in the title, which can confuse a user who is working with GitHub Packages for the first time and is trying to upload their project manually (again using dotnet CLI)
Example of this experience is this discussion: https://github.com/orgs/community/discussions/49186
What's being changed (if available, include any code snippets, screenshots, or gifs):
Update guide in Publishing a package and Publishing a package using a nuget.config file and add step about authenticating GitHub Packages for user:
1. Authentificate GitHub Packages with the `dotnet` command-line interface (CLI). Replace `OWNER` with your username or company name.

    ```shell
    dotnet nuget add source --username OWNER --password ${ secrets.GITHUB_TOKEN } --store-password-in-clear-text --name github "https://nuget.pkg.github.com/OWNER/index.json"
    ```

Check off the following:

[x] I have reviewed my changes in staging, available via the View deployment link in this PR's timeline (this link will be available after opening the PR).

For content changes, you will also see an automatically generated comment with links directly to pages you've modified. The comment won't appear if your PR only edits files in the data directory.

[x] For content changes, I have completed the self-review checklist.

I apologize for the "spammy" commits with fixes and adjustments to your answer.
As was mentioned in the discussion, I removed the “duplicate” authorizations, changed ${{ secrets.GITHUB_TOKEN }} to YOUR_GITHUB_PAT, combined and moved the authorization instructions to the “Publish a package” block, if something else required, I'll be happy to try to fix/finish it.

@Falcion Thank you so much for that last batch of adjustments! 💛 I'll update the branch and get this merged once tests are passing 🍏


Change question about starting a remote node

[x] I have read the Contributing file of the documentation.
[x] I have searched the issues (including closed ones) and believe that this is not a duplicate.
[x] I am willing to lend a hand to help implement this feature.

Feature Request
Feature Description
The question in the script asking if do you want to start a remote node sould be changed by:
Do you want to start a new local node instance? Y/n
It is, in fact, less confusing. The script only lets you to start a local node, and by asking if you want to start a remote node, it is not easy to imagine that you have to start the remote node by yourself.

Also, I think that '.availableCurrencies' file should be renamed without the dot at the beginning, because that means in Unix that a file is hidden (that's the only purpose of a dot), and there is not any reason why that file should be hidden.


[TabNavigator] onPress Override

Use Case:
If a app has a tab that requires authentication and there is no session present it should pop up a modal prompting login or signup.
I haven't dug too much into react-navigation's API. After converting from React Native's Navigation and TabBarIOS I find that this is the only thing I am really missing.
I would like to look into this and possibly open a PR if no one else is working on it currently.
In a quick search I found this issue #546, and the recommended solution is to build your own custom tab bar.

I have been working on this for a couple hours and finally found a solution that doesn't use Redux. (The app I am working on has an explicit, Redux ONLY when Redux is absolutely necessary policy, so far we've not needed Redux in any way :)
I used this as a basis for the solution (https://reactnavigation.org/docs/routers/#Handling-Custom-URIs)
Heres the jist

const MyTabNav = TabNavigator({
  Tab1: {
    screen: Tab1,
    navigationOptions: {
      title: 'Tab1',
    }
  },
  Tab2: {
    screen: Tab2,
    navigationOptions: {
      title: 'Tab2',
    }
  }
},{
  tabBarPosition: 'bottom',
});

export default class MyComponent extends Component {
  constructor(props) {
    const originalGetStateForAction = MyTabNav .router.getStateForAction;
    Object.assign(MyTabNav .router, {
      getStateForAction(action, state) {
        /* do what you want right here
         *
         * custom stuff
         */
        return originalGetStateForAction(action, state);
      }
   });
  }
}

Actually you can replace the above by using
https://reactnavigation.org/docs/navigators/#onNavigationStateChange-prevState-newState
<MyTabNav onNavigationStateChange={(prevState, nextState) => {}} />

@jayesbe, Does that prevent the default handler for the state change from happening? Or in addition to it?
In my case, I want each tab button to clear the sub-stacks so I'm always going to the first item, by overriding jumpToIndex on the tabBarComponent:
export const TabContainer = TabNavigator({
VaultStack: {
screen: VaultStack,
}
}
}, {
tabBarComponent: (props) => {
const { navigation, navigationState } = props;
const _jumpToIndex = props.jumpToIndex; // Just in case
return (<TabView.TabBarBottom {...props} jumpToIndex={(index) => {
tab = navigationState.routes[index];
tabRoute = tab.routeName;
firstRoute = tab.routes[0].routeName; // navState is Tabs object
// Do what you want here
		const tabAction = NavigationActions.navigate({ routeName: tabRoute });
		const firstScreenAction = NavigationActions.reset({ index: 0,
			actions: [ NavigationActions.navigate({ routeName: firstRoute }) ]
		});
		navigation.dispatch(tabAction);
		navigation.dispatch(firstScreenAction);
	}} />)
},

});

@rileybracken , instead of changing it here, why not have a method in each component that requires authentication to render() a LoginScreen if they're not signed in? That way, they won't have the animation. Think in terms of statefulness. This is what i have in my Vault component:
  render() {
    if(!this.props.userProfile) { return (<Login/>) } // Prompt for login if needed
    // .... continue for signed in users....

This way, you won't have to worry about 'redirecting' from the LoginScreen back to the "intended" screen. The login screen just disappears once the userProfile is set in redux.

@tmaly1980  I couldn't prevent tab change. The docs show an example of how to do it but it doesnt work.
What I ended up doing was moving the instantiation of the TabNavigator to inside my constructor.
  constructor(props, context) {
    super(props, context);

    const component = this;
    this.MyTabNavigator = TabNavigator({
	  MyTab: {
	    screen: MyScreen,
	    navigationOptions: {
	      title: 'My Screen',
	    }
	  }
	},{
	  tabBarPosition: 'bottom',
	  tabBarOptions: {
	    indicatorStyle: {
	      backgroundColor: '#fff'
	    },
	    style: {
	      backgroundColor: '#2b87a2'
	    },
	    onTabPress: (tab) => {
	    	// onTabPress stuff here..
	    }
	  }
	});
  }

  render() {
    const MyTabNavigator = this.MyTabNavigator;
    return (
      <MyTabNavigator />
    );
  }

To be honest I think that the best route to fix this (for my use case) would be with a HOC.
It is gone over documented well in this issue.
It is similar to what @tmaly1980 is saying. Your HOC would be something like authenticatedUser, before it mounts it would check if a user is authenticated, if not it would fire the session StackNavigator, if they are authenticated it would fire the profile TabNavigator.

Once I have the HOC implemented I will post it in the issue, above is just my idea of how it would work.

Closing in favour of #314

import hoistNonReactStatic from 'hoist-non-react-statics';

const withGetOnPressOverriden = (routesAndFunctions) => (WrappedComponent) => {
    class overriden extends React.Component {
        render() {
            const getOnPress = (_,data) => {
                const routeName = data.route.routeName;
                if(routesAndFunctions[routeName]) {
                    return () => routesAndFunctions[routeName]();
                }
                else {
                    return this.props.getOnPress(_,data)
                }
            }
            return <WrappedComponent {...this.props} getOnPress={getOnPress}/>;
        }
    }
    return hoistNonReactStatic(overriden, WrappedComponent);
}

const TabBarBottomOverriden = withGetOnPressOverriden(
    {
        CreatePost: () => console.log('createPost')
    }
)(TabBarBottom)


chore: bdd-spring-1587407736 to 0.0.1

chore: Promote bdd-spring-1587407736 to version 0.0.1

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approvers:
If they are not already assigned, you can assign the PR to them by writing /assign  in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


How to use with Alpine.plugin()

Maybe I'm missing something, but I can't get this to run using Alpine.plugin().
import Alpine from "alpinejs";
import * as wizard from "@glhd/alpine-wizard";

Alpine.plugin(wizard);

window.Alpine = Alpine;
Alpine.start();

Doesn't seem to work.

Strike that. Rookie error. This works
import Alpine from "alpinejs";
import wizard from "@glhd/alpine-wizard";

Alpine.plugin(wizard);

window.Alpine = Alpine;
Alpine.start();


Diamond receive routing

Basic test of support for dynamic receive function in diamond proxies.

Merging directly because these are just tests.


on terminal window, output is often truncated, like if a flush was missing

Issue Type: Bug
Usually in python debugging
VS Code version: Code 1.21.1 (79b44aa704ce542d8ca4a3cc44cfca566e7720f1, 2018-03-14T14:47:13.351Z)
OS version: Windows_NT ia32 10.0.16299

System Info

Item
Value

CPUs
Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz (8 x 2592)

Memory (System)
15.90GB (11.99GB free)

Process Argv
C:\Program Files (x86)\Microsoft VS Code\Code.exe

Screen Reader
no

VM
0%

Extensions (6)

Extension
Author (truncated)
Version

MagicPython
mag
1.0.12

python
ms-
2018.2.1

cpptools
ms-
0.15.0

java
red
0.20.0

python
tht
0.2.3

vscode-java-debug
vsc
0.7.0

Reproduces only with extensions

Even with all Java and MagicPython extensions disabled, the issue persists...
This is an example :
Traceback (most recent call last):
File "C:\Anaconda3\lib\site-packages\matplotlib\backend_bases.py", line 1319, in _on_timer
ret = func(*args, **kwargs)
File "c:/PyProjects/CH/MAIN.py", line 1787, in update_figure
self.add_X()
File "c:/PyProjects/CH/MAIN.py", line 1043, in add_X
self.add_x(1)
File "c:/PyProjects/CH/MAIN.py", l
PS C:\PyProjects\CH>
what to do ?
thanks

Step 1 read how to file nice issues https://github.com/Microsoft/vscode/wiki/Submitting-Bugs-and-Suggestions


Clarify what happened to Spanish subdivisions

In the past 4 months there has been a shift in how Spanish subdivisions are handled.
The ISO list is mixed language-wise, provinces are named in their majority language, hence Lleida instead of Llerida, etc. This was the approach taken by the dataset as well. In 2018 the ES list changed:

The province list has been separated into es/ca/ga versions. This makes sense at a glance.
The provinces were rekeyed, the previous keys were province names, now we're using the codes (C/VI/AB/etc). This is confusing, cause the key is supposed to be used when formatting an address, but Spain doesn't use codes that way, they always write the province names.
The ISO codes have been removed (no isoid for any of them). This looks accidental. Other countries have ISO codes defined even if the subdivision is keyed by the short version of the code.

So, I'm asking for clarification, and treating #3 as a bug.
I've had to revert to the 2017 version of the list in commerceguys/addressing, cause even the translation change was confusing users, who were used to the mixed list.

Is this still an issue.
AFAICT the iso-ids are present, and the 'key' values are the full names not abbreviations.

I don't see any changes compared to when I opened the issue.
E.g. https://chromium-i18n.appspot.com/ssl-address/data/ES/L has no isoid.


Unable to compile for Extension

... due to UIApplication.shared.keyWindow is not available in extension

sorry, I missed the "NVActivityIndicatorView/AppExtension" one that work in Extension


Python: JSON in Python

Description
Documentation on JSON in python
Domain
Python_Domain
Type of Contribution
Documentation
Code of Conduct

[X] I follow Contributing Guidelines & Code of conduct of this project.

/assign

@adityabisoi can I work on this?


Existing Flatpaks

Installing this has sent my system into meltdown.
Do I need to uninstall existing flatpaks first?

Can you define "meltdown" ?
You don't need to uninstall flatpkas for this module to work. From the README:
_By default nix-flatpak will only manage (install/uninstall/update) packages declared in the services.flatpak.packages and repositories declared in services.flatpak.remotes. Flatpak packages and repositories installed by the command line or app stores won't be affected.
Set services.flatpak.uninstallUnmanaged = true to alter this behaviour, and have nix-flatpak manage the lifecycle of all flatpaks packages and repositories._
Beware though that  depending on how you setup nix-flatpak (nixos vs home-manager), and how you install packages from cli (system vs user), you might end with duplicate installed packages.

no worries @C4E5G3 ! Glad it is fixed. Feel free to re-open, if it turns out to be an issue related with nix-flatpak.


Added Count Child Records action

Hi,
I have added a new action named "Count Child Entity Record". This accepts the child entity name & lookup field name and returns the count.
Please review and if you like it, add it to the core solution.
Thanks,
Ravi Kashyap

merged to master


MAINT: backports for 2.0.3

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

0 of 4 (100.0%)  changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.3%) to 87.943%

Totals

Change from base Build<PHONE_NUMBER>:
0.3%

Covered Lines:
2582

Relevant Lines:
2936

💛  - Coveralls


feat: make job-worker owned collections readonly in meteor SOFIE-1556

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Code quality

What is the current behavior? (You can also link to an open issue here)

Various collections are owned by the job-worker. For these, when executing a job it loads some documents into memory, operates on this in memory cache making its changes, then any changed documents get written back to the database with replace.
If meteor was to modify any of these documents while a job is in progress, the changes made by meteor can end up lost.

What is the new behavior (if this is a feature change)?

To clarify the ownership, and that they should not be modified, these collections are now readonly in meteor. This ensures that new code wont modify these collections without it being obvious that it shouldnt be doing that.
As we do need to be able to perform modifications in the cleanup cronjob, migrations and unit tests, there is a mutableCollection property on these collections which gives access to the write methods. The hope is that this separation makes it clear that it is not supposed to be done, and will make it easier to keep track of these uses and review them for correctness.

Other information:

When making these readonly, I have verified that they are also readonly on the client.
To ensure that the collections are readonly in any publications, I have reworked how the collection security is setup, so that it is defined when creating the server collection. This allows us to be confident that it is setup as intended and correct.
Status

[ ] Code documentation for the relevant parts in the code have been added/updated by the PR author
[x] The functionality has been tested by the PR author
[ ] The functionality has been tested by NRK

Codecov Report
Patch coverage has no change and project coverage change: +19.99 :tada:

Comparison is base (413007b) 56.37% compared to head (f499652) 76.37%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@              Coverage Diff               @@
##           release50     #929       +/-   ##
==============================================
+ Coverage      56.37%   76.37%   +19.99%
==============================================
  Files            472       31      -441
  Lines          75306     5743    -69563
  Branches        3706      572     -3134
==============================================
- Hits           42455     4386    -38069
+ Misses         32801     1355    -31446
+ Partials          50        2       -48

see 439 files with indirect coverage changes

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


"Page Source" link is broken on site built on nbsite

This is with reference with an issue in pyviz.org.
Thanks to @ceball, its been noticed there is a part of code that's deleting folder - '_sources' during the build.
Also, the code block between lines 43-52 in same file is not doing much.
However, the removal of the code blocks, and corresponding effect on the application is not tested in local/dev build.  This is to be tested.

What's the request here?

I think @dheepakg is just following up on the suggestion in https://github.com/pyviz/pyviz.org/issues/57 to track that issue here:

Sorry for the confusion caused.
'Page Source' of the site generated from nbsite is broken, this needs to be fixed.

The URL in the footer of pyviz.org is broken. Refer - https://github.com/pyviz/pyviz.org/issues/57
The footer url is pointing to https://pyviz.org/ _sources/index.rst.txt
During nbsite build,  we are removing '_sources' directory. Refer - https://travis-ci.org/pyviz/pyviz.org/jobs/657893376#L809
The deletion happens due to the block - https://github.com/pyviz-dev/nbsite/blob/abb56c2a5b1348c97c2b1f2072632dc8a74eba00/scripts/nbsite_cleandisthtml.py#L32
I've made the changes to retain _sources folder - https://github.com/dheepakg/nbsite/blob/master/scripts/nbsite_cleandisthtml.py#L32
However, this code change resulted in failure of one test case - test_build_deletes_by_default.
Please refer - https://travis-ci.com/github/dheepakg/nbsite/builds/152841644#L641
In this test case, we are expecting 10 items inside "builddocs". But we end up having 11 items inside the folder (count incremented due to _sources).
My question is, should we stick to 10 items inside the folder or the test case should be modified and proceeded with the changes?
Thanks for helping!

@jbednar @ceball
Can you please check this

If nbsite's going to stop removing _sources now, it's fine to update the test to reflect that.
I think it's no problem in general to leave _sources, and that should have been the default originally. I don't know why it was not, so I feel slight unease we might cause a problem (e.g. maybe some project has huge files going in there or something?). But hopefully someone will speak up if we do cause a problem! Or even better, before then (e.g. maybe @philippjfr or @jsignell, as other frequent victims of nbsite...).

I think there should probably be a new flag that allows the user to control whether the source is deleted. That way the default could stay the same and downstream libraries can choose whether or not to retain the source.


Utility function to modify/delete users from team

While working on #17 I found that these are already implemented via the Team#add_user and Team#remove_team_member functions. Is there something else we need for modifying memberships?

While working on #17 I found that these are already implemented via the Team#add_user and Team#remove_team_member functions. Is there something else we need for modifying memberships?

That's great. No, I think this issue belongs in the AllSpice Demos repository actually. Feel free to close it.
The request came from our customer who has a script where teams across multiple orgs are updated from a text file containing user names. They have been able to add new users, but the script doesn't allow updating the teams. @danielallspice is that script public? If that's not public, maybe we can just put together a simple demo that will add/remove a user from a team across orgs.

The user has access to requests_patch, so we could handle it in APIDemos, but again, my preference is to add modify_team to py-allspice.

There already is a patch route for editing teams, except it doesn't work with a specific method like modify_team. I've covered how it works in the "Editing Teams" section of this example script. TL;DR: You set the fields of the team to what you want:
my_team.name = "my-new-team-name"

and then you call the commit() method on it:
my_team.commit()

This calls the patch route and updates it. This works for editing the fields of the team itself, but if you have to e.g. edit the members of the team you have to use the add_user and remove_team_member methods.

Awesome. Thank you for the clarification.


Feature/steps box shadow

Codecov Report

Merging #27 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop      #27   +/-   ##
========================================
  Coverage    94.29%   94.29%
========================================
  Files          124      124
  Lines         1963     1963
========================================
  Hits          1851     1851
  Misses         112      112

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0341a1...d7a8b29. Read the comment docs.


Add an (approximate) expiry time for names

I did a blockstack whois vsund.id to see when I have to renew my name, but there are only blocks for creation and expiry
An (approximate) expire in time would be helpful to have a better understanding for when a renewal is necessary.

Related: https://forum.blockstack.org/t/will-you-remove-users-who-are-name-squatting/469/4

@vsund we merged the CLI repo with blockstack-core and I moved this issue there. Closing this copy of the issue.


Pre Request vars aren't updating until the request is saved

I have checked the following:

[X] I use the newest version of bruno.
[X] I've searched existing issues and found nothing related to my issue.

Describe the bug
One of the recent releases made it so my pre request vars aren't being updated when I send the request unless I save it first.
.bru file to reproduce the bug
No response
Screenshots/Live demo link

Having the same issue with Bruno v1.20.2 on Kubuntu 22.04. This bug has caused me some grief...

Hey @nicco09, thank you for taking the time to report the bug.
Unfortunately, I wasn't able to reproduce it in the latest version v1.20.4 of Bruno
https://github.com/user-attachments/assets/37c356e7-0a86-4404-8d5d-5cf5f05cc10b

I can
https://github.com/user-attachments/assets/4d228772-2b74-44ca-8ae1-e254a92040fb

Yeah @Moghul, I was also able to reproduce the issue. I notified the team, and we are working on a fix!

Thanks for taking care of this @sanjai0py , I appreciate it!


Filter should have a constructor that has a BooleanOperator parameter

Expected Behavior
Users should be able to instantiate filters with boolean operators in one line.
For example, perhaps something like:
Filter filter = new Filter(BooleanOperator.AND, "someProperty", ComparsionOperator.EQUALS, "foo");

Current Behavior
Boolean Operator has to be set after creation:
Filter filter = new Filter("someProperty", ComparsionOperator.EQUALS, "foo");
filter.setBooleanOperator(BooleanOperator.AND);

Possible Solutions

An additional constructor could be added.
Could put a FilterBuilder class alongside it.
Filters could have methods like and(Filter filter) and or(Filter filter) that append filters to the list while setting the boolean operator property on them.

Thanks for creating this feature request. We already had a look at this and we will probably go with the third suggestion and add basic chaining for filter concatenations.


Add placeholder plugins when auto-generating next version's manifests

Is your feature request related to a problem? Please describe
Coming from https://github.com/opensearch-project/opensearch-plugins/pull/219#issuecomment-1621215855 the problem we'd like to solve is to know which plugins are going to be in some future release of OpenSearch. This is useful to, for example, run tests for the plugins on every PR into OpenSearch core (https://github.com/opensearch-project/OpenSearch/pull/8486).
Describe the solution you'd like

Allow manifests to include placeholder component references. These would not be packaged, ie. simply skipped in all the packaging/testing/etc. workflows.

components:
  - name: OpenSearch
    repository: https://github.com/opensearch-project/OpenSearch.git
    ref: 2.x
  - name: common-utils
    repository: https://github.com/opensearch-project/common-utils.git
    ref: 2.x
    placeholder: true # this is not a real component!

When automatically creating a new manifest, copy all components from the previous manifest as placeholders.
Releasing should check that there are no placeholder components.

Describe alternatives you've considered
No response
Additional context
No response

more info at https://github.com/opensearch-project/opensearch-build/issues/441#issuecomment-1275192880
https://github.com/opensearch-project/opensearch-build/pull/414#discussion_r706329805
@dblock , would you think the above two would make it work ? thanks

The idea is to update the manifest file with all the plugins from the latest 2.x release. We should make some improvements to Manifest Workflow

One approach is to tweak CI manifest check workflow so only syntax of manifest file is checked instead of Gradle runs. CC: @peterzhuamazon

Will expand on the existing template on this.

We have a discussion on this and will go with approach 2 + Removing the input manifest checks section as the distribution build already taken care of the issue creation:
Approach 1 : Populate all the entries of the new input manifest, from corresponding templates

Steps:

Using the existing manifest templates located here: https://github.com/opensearch-project/opensearch-build/tree/main/manifests/templates
Update all templates to include every component from the latest release of their respective versions.

1.x uses 1.3.14 as base
2.x uses 2.11.1 as base
3.x uses 3.0.0 as base

Update src/manifests_workflow to generate a new manifest based on the corresponding template, then replace the version number of the release and git reference of each component

Pros:

Easy and simple approach, with PR ready: https://github.com/opensearch-project/opensearch-build/pull/4271
Consistent output, where component dependencies and build order is always maintained, and reviewed
Minimal changes on both the python code and the github action workflow
Allow each major version to have its own template, to avoid current situation where 1.x/2.x is using 3.x as base

Cons:

A manual PR is required to add a new component
Adding all components at once, when not all of them have bumped their versions, can lead to build failures if the --continue-on-error parameter is not set

Approach 2: Populate all the entries of the new input manifest, from latest input manifest of major version
(Will approach)

Steps:

Update Python code, to find the latest manifest path of corresponding major version. e.g. If 1.x, use 1.3.14
Copy that manifest, then replace the version number of the release and git reference of each component

Pros:

Even easier approach then 1, as you do not need to maintain templates
Even smaller code changes to both the python code and the github action workflow.
Just edit latest version search logic to look for the latest version within the same major version category
Do not need manual PR to add new components for new manifest

Cons:

Relies on the latest version of the manifest to be correct, and maintain dependencies
Not reliable enough
Adding all components at once, when not all of them have bumped their versions, can lead to build failures if the --continue-on-error parameter is not set

Approach 3: Use Approach 1 + Add Placeholder parameter

Steps:

Same as Approach 1
Check every component, to see if version bump is available
Add placeholder parameter as true to plugins that needs version bump
Update build_workflow to take placeholder parameter into account

components:
  - name: OpenSearch
    repository: https://github.com/opensearch-project/OpenSearch.git
    ref: 2.x
  - name: common-utils
    repository: https://github.com/opensearch-project/common-utils.git
    ref: 2.x
    placeholder: true # this is not a real component!

Pros:

Easy and simple approach, with PR ready: https://github.com/opensearch-project/opensearch-build/pull/4271
Consistent output, where component dependencies and build order is always maintained, and reviewed
Minimal changes on both the python code and the github action workflow
Allow each major version to have its own template, to avoid current situation where 1.x/2.x is using 3.x as base
Build and Assemble workflow will skip placeholder: true plugins

Cons:

A manual PR is required to add a new component
Placeholder: true is nothing different from --continue-on-error
Complicated implementation compares to both Approach 1 & 2

Approach 4: Use Approach 1 or 2 + Only adding components that has bumped the version already

Result will be the same as now, nothing changes

Close this in favor of issue #4389.


R warning: "needs @export or @exportS3method tag"

Rscript -e 'devtools::document()' is working, but early in the log there are a pair of warnings:
ℹ Updating opendp documentation
ℹ Loading opendp
Registered S3 method overwritten by 'opendp':
  method        from
  print.hashtab utils
✖ mod.R:390: S3 method `to_str.default` needs @export or @exportS3method tag.
✖ mod.R:391: S3 method `to_str.hashtab` needs @export or @exportS3method tag.

Same warning in CI
The build still works, so not critical, but it would be good not to have warnings in the log.

Fixed in #2082


Clarify how third party invites work

Rendered: see 'docs' commit status. This is highly recommended given the large sequence diagrams in the client-server spec.

This adds several diagrams to the Client-Server API about how invites are handled, including what the server is expected to do. This helps implementors know what they are supposed to do in the common cases, and infer where needed to get the more complex cases correct.
Although lacking in some areas, this is how third party invites work today.
A link to the now-improved client-server documentation for third party invites has been added to the server-server specification, alongside some documentation on how the /3pid/onbind endpoint works.
Fixes #1366
Fixes https://github.com/matrix-org/matrix-doc/issues/1422

@dbkr please take a look at the updated wording. It's slightly different than your suggestion as I want to make sure people don't cache the request as invalid forever, even if the key becomes valid at a later date.

Actually, hang on - I thought the point of this endpoint was mostly to support key revocation, so the point of it is that it might later say a key is not valid that previously was? Also, in your suggestion of it going invalid -> valid at some point, this also would violate your condition of it always being the same?

If the key can become invalid at a later point then third party invites would be rejected by servers as they attempt to verify them.
The updated wording for checking for valid keys should support only the case of the server publishing new keys. If the IS were required to always say the same thing for a given request then someone could easily reserve a ton of keys on the server that it then has to consider as invalid forever. With the condition that it has to consider a key that exists as (in)valid forever, the server isn't open to being abused.

Hmm, the thing is that the intro for that section says:

is, the In the event of key compromise, the identity service may revoke any of its keys. An HTTP API is offered to get public keys, and check whether a particular key is valid.

Which surely is saying it exists so the ISes can revoke keys? I was afraid it might be that subsequent servers validating the 3pid invite would get a different result - this seems a bit broken? Not sure how best to untangle this mess.

The ability to revoke keys is important. I'm sure there's a hole somewhere in this suggestion, but how does altering the auth rules slightly sound? The auth rules could consider the age of the event (by depth or ts or something) as a heuristic for "this was probably valid at one point, but it's so old now that an invalid key isn't unexpected". It should still check the key, and maybe just not send the event to clients if the key is valid and old, but otherwise accept it?
This does have the problem where if someone does a 3rd party invite and moments later the IS decides it needs to rotate keys - the fresh invite would be considered as invalid by a subset of servers and reject it.

I think the hole may be that a server can come up with an event supposedly from an old fork (but that actually it's just made up) referencing an old 3pid invite with a revoked key, and this will always be accepted. I was about to question why the validity endpoint doesn't return a not-before & not-after like an x.509 cert, but I think this applies here too in that whatever server is making the join can just pick whatever time at which the key was valid.
Might be good to get @erikjohnston / @richvdh 's thoughts here. There's a certain argument for speccing what we have and trying to unravel this later, even if that means something like saying you actually can't ever revoke keys (which presumably is currently the case).

I'm not quite sure I understand the discussion here. If the IS wants to revoke its key, it stops replying yes to "/isvalid". Does that break something?

Yes, otherwise valid 3rd party invite events get rejected by servers that weren't lucky enough to check the event when the key was valid.

Yes, otherwise valid 3rd party invite events get rejected by servers that weren't lucky enough to check the event when the key was valid.

Oh right. I hadn't grokked that other HSes needed to verify the signature at the point they see the 3pid invite - the only time 'isvalid' is mentioned in the spec is at the point H1 turns it into a normal invite. TBH this sounds like a questionable decision, but it's probably not worth debating at this point.
Given what you say, I agree that the correct decision is to spec it as-is, even if that means that keys can't be revoked, and then fix it asap.

@richvdh and/or @dbkr  - please take a look. I've opened https://github.com/matrix-org/matrix-doc/issues/1633 to cover the case of identity services not actually being able to revoke their keys.


Delete the Estimate and Actual Scan Data doc

Purpose of this pull request
This pull request is to pull back the docs related to Estimate and Actual scan data as this feature is undergoing some change from the dev side.
Select the type of change

[ ] Minor Changes - Typos, formatting, slight revisions
[ ] Update Content - Revisions, updating sections
[x] New Content - New features, sections, pages, tutorials
[ ] Site and Tools - .clabot, version updates, maintenance, dependencies, new packages for the site (Docusaurus, Gatsby, React, etc.)

Ticket (if applicable)

@kimsauce, since you're working on flex pricing, I'll leave review to you.

The intent of this PR is covered in the PR: https://github.com/SumoLogic/sumologic-documentation/pull/3796, so closing this PR.


Fixed #344 - Run tests on python 3.11

Fixed #344

Can I ask someone to review and eventually merge this PR?
Please use the "rebase and merge" button

Please also add the trove classifier to setup.py.

Done.
Thanks for the review.

@pauloxnet
Thanks!

Please use the "rebase and merge" button

Just out of curiosity, (why) do you prefer it over the squash button?  Or is it mainly about not using the default merge action?

Done.

Thanks!

Just out of curiosity, (why) do you prefer it over the squash button?

As you said it's mainly about not using the default merge action.


feat: add to_dict method to domain classes

This allows us to serialize the domain classes by first converting them to "dumb types". For example, to convert the objects to JSON.
import json

domain = BaseDomain.from_dict(domain_dict)
json.dumps(domain.to_dict())

I first intended to use this to help with the hetzner.hcloud ansible collection inventory caching implementation. But I choose to use another method.
This might still be useful for the users of the library, to be able to export the entire domain object to a dict.


[HOLD for payment 2024-04-09] [$250] Web - Video - Expand video button isn't highlighted when hovered on

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.56-0
Reproducible in staging?: y
Reproducible in production?: y
Issue found when executing PR: https://github.com/Expensify/App/pull/36945
Issue reported by: Applause - Internal Team
Action Performed:

Navigate to a conversation
Attach a video.
Play the video. The expand video button appears alongside the video
Hover the mouse icon on the expand button

Expected Result:
Expand video button is highlighted when hovered on as other buttons like pause, full screen etc.
Actual Result:
Expand video button is not highlighted when hovered on
Workaround:
n/a
Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/93399543/27f06830-49b7-45c1-af0d-a3cb0604c89c
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~019dec26faa8f28e37
Upwork Job ID:<PHONE_NUMBER>297416192
Last Price Increase: 2024-03-25
Automatic offers:

GandalfGwaihir | Contributor | 0

@sonialiap I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

Payment summary

@sobitneupane $250 - please request in NewDot and complete the checklist
@GandalfGwaihir $250 - paid ✔

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[ ]  [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

https://github.com/Expensify/App/pull/36945

[ ]  [@sobitneupane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/36945#issuecomment-2049314450

[ ]  [@sobitneupane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

This should have been caught in the previous PR review. We don't need to add a new checklist.

[ ]  [@sobitneupane] Determine if we should create a regression test for this bug.

Yes.

[ ]  [@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Propsal

Navigate to a conversation
Attach a video.
Play the video. The expand video button appears alongside the video
Hover the mouse icon on the expand button
Verify that the expand video button is highlighted when hovered on as other buttons like pause.

Do we agree 👍 or 👎

Thanks!

$250 approved for @sobitneupane


Listener and reactivity

Suppose I have a setup as follows:
const [val, setval] = createSignal();
function hChange()  {
   // do something
}
function Main() {
   return (
      <input type="text name="abc" onChange={hChange} value={val()} />
   );
}

Lets assume setVal is called by a resource that may take a second to load. The element is rendered, then the resource calls setVal and this updates the input value. Since the new value is reactive and no keyboard or mouse input was made, the onChange handler is not triggered. Is there any way  I can trigger the onChange listener in this instance ?

The browser on raises those events like onInput and onChange when the user makes changes.
There is nothing stopping you using createEffect to do something else when val() changes if you need to.

Sure, but I need access to the event object.

What brendan meant is probably along the lines of createEffect(() => { doSomething(val()) }) and with this I'm not sure if the event object is even necessary. But surely you can do unnecessary input.dispatchEvent(new Event('change')) if you still want.


Use structural sharing in addons if applicable

Currently we always normalize data after each query successful response. However, some libs like react-query uses structural sharing feature, which preserves data reference if it did not change. We could use this to normy advantage to give up normalization in such cases, as normalizating in this scenario will end up with the same normalized store.
This is especially useful for features like refetch on refocus, as then we can end up with multiple queries refetches at the same time, most of each will end up probably being the same.

Done in https://github.com/klis87/normy/releases/tag/%40normy%2Fcore%400.10.0 .
I will keep it open though, as unfortunately for now it does work with rtk-query. Awaiting https://github.com/reduxjs/redux-toolkit/issues/4073


Test fail 80-test_dtls.t on Gentoo Linux

This is on a Gentoo Linux sparcv9 server with gcc (Gentoo 12.3.1_p20230526 p2) 12.3.1 20230526
as well as the usual up to date tools. To be fair glibC could be brought up past 2.37 but such is life.

After a slight patch due to PR21718 the compile finishes as well as the testsuite. However :

Test Summary Report
-------------------
80-test_dtls.t                   (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
80-test_ssl_new.t                (Wstat: 512 (exited 2) Tests: 30 Failed: 2)
  Failed tests:  7, 11
  Non-zero exit status: 2

I have seen this sort of thing before when trying to use a specific C language standard. Back
in PR20379 and further back 8048 when I was trying strict C99. I don't know any more what
C language spec the OpenSSL project complies with. If any.

s$
s$ uname -a
Linux sphinx 5.15.88-gentoo-sparc64 #1 Sun Mar 12 18:36:19 GMT 2023 sparc64 sun4u TI UltraSparc IIi (Sabre) GNU/Linux
s$
s$ perl configdata.pm --dump

Command line (with current working directory = .):

    /usr/bin/perl ./Configure linux64-sparcv9 no-asm shared no-engine enable-weak-ssl-ciphers zlib --prefix=/opt/bw --libdir=lib --openssldir=/opt/bw/ssl --with-rand-seed=devrandom threads

Perl information:

    /usr/bin/perl
    5.36.1 for sparc64-linux

Enabled features:

    aria
    async
    autoalginit
    autoerrinit
    autoload-config
    bf
    blake2
    bulk
    cached-fetch
    camellia
    cast
    chacha
    cmac
    cmp
    cms
    comp
    ct
    deprecated
    des
    dgram
    dh
    dsa
    dso
    dtls
    ec
    ec2m
    ecdh
    ecdsa
    err
    filenames
    gost
    idea
    legacy
    makedepend
    md4
    mdc2
    module
    multiblock
    nextprotoneg
    ocb
    ocsp
    pic
    pinshared
    poly1305
    posix-io
    psk
    rc2
    rc4
    rdrand
    rfc3779
    rmd160
    scrypt
    secure-memory
    seed
    shared
    siphash
    siv
    sm2
    sm3
    sm4
    sock
    srp
    srtp
    sse2
    ssl
    ssl-trace
    static-engine
    stdio
    tests
    threads
    tls
    ts
    ui-console
    weak-ssl-ciphers
    whirlpool
    zlib
    tls1
    tls1-method
    tls1_1
    tls1_1-method
    tls1_2
    tls1_2-method
    tls1_3
    dtls1
    dtls1-method
    dtls1_2
    dtls1_2-method

Disabled features:

    acvp-tests          [cascade]        OPENSSL_NO_ACVP_TESTS
    afalgeng            [cascade]        OPENSSL_NO_AFALGENG
    asan                [default]        OPENSSL_NO_ASAN
    asm                 [option]         OPENSSL_NO_ASM
    buildtest-c++       [default]
    capieng             [cascade]        OPENSSL_NO_CAPIENG
    crypto-mdebug       [default]        OPENSSL_NO_CRYPTO_MDEBUG
    devcryptoeng        [default]        OPENSSL_NO_DEVCRYPTOENG
    dynamic-engine      [cascade]
    ec_nistp_64_gcc_128 [default]        OPENSSL_NO_EC_NISTP_64_GCC_128
    egd                 [default]        OPENSSL_NO_EGD
    engine              [option]         OPENSSL_NO_ENGINE (skip engines, crypto/engine)
    external-tests      [default]        OPENSSL_NO_EXTERNAL_TESTS
    fips                [default]
    fips-securitychecks [cascade]        OPENSSL_NO_FIPS_SECURITYCHECKS
    fuzz-afl            [default]        OPENSSL_NO_FUZZ_AFL
    fuzz-libfuzzer      [default]        OPENSSL_NO_FUZZ_LIBFUZZER
    ktls                [default]        OPENSSL_NO_KTLS
    loadereng           [cascade]        OPENSSL_NO_LOADERENG
    md2                 [default]        OPENSSL_NO_MD2 (skip crypto/md2)
    msan                [default]        OPENSSL_NO_MSAN
    padlockeng          [cascade]        OPENSSL_NO_PADLOCKENG
    rc5                 [default]        OPENSSL_NO_RC5 (skip crypto/rc5)
    sctp                [default]        OPENSSL_NO_SCTP
    trace               [default]        OPENSSL_NO_TRACE
    ubsan               [default]        OPENSSL_NO_UBSAN
    unit-test           [default]        OPENSSL_NO_UNIT_TEST
    uplink              [no uplink_arch] OPENSSL_NO_UPLINK
    zlib-dynamic        [default]
    ssl3                [default]        OPENSSL_NO_SSL3
    ssl3-method         [default]        OPENSSL_NO_SSL3_METHOD

Config target attributes:

    AR => "ar",
    ARFLAGS => "qc",
    CC => "gcc",
    CFLAGS => "-Wall -O0 -g",
    CXX => "g++",
    CXXFLAGS => "-Wall -O0 -g",
    HASHBANGPERL => "/usr/bin/env perl",
    RANLIB => "ranlib",
    RC => "windres",
    asm_arch => "sparcv9",
    bn_ops => "BN_LLONG RC4_CHAR",
    build_file => "Makefile",
    build_scheme => [ "unified", "unix" ],
    cflags => "-pthread -m64 -g -O0 -fno-builtin -fno-fast-math -mno-app-regs -mcpu=ultrasparc -mno-vis -mno-vis2 -mno-vis3 -mno-vis4 -mno-vis4b -mno-fmaf -mno-fsmuld -mno-popc -mno-subxc -Wl,-rpath=/opt/bw/lib,-enable-new-dtags",
    cppflags => "",
    cxxflags => "-std=c++11 -pthread -m64 -g -O0 -fno-builtin -fno-fast-math -mno-app-regs -mcpu=ultrasparc -mno-vis -mno-vis2 -mno-vis3 -mno-vis4 -mno-vis4b -mno-fmaf -mno-fsmuld -mno-popc -mno-subxc -Wl,-rpath=/opt/bw/lib,-enable-new-dtags",
    defines => [ "OPENSSL_BUILDING_OPENSSL", "ZLIB" ],
    disable => [  ],
    dso_ldflags => "-Wl,-z,defs",
    dso_scheme => "dlfcn",
    enable => [ "afalgeng" ],
    ex_libs => "-lz -ldl -pthread -latomic",
    includes => [  ],
    lflags => "",
    lib_cflags => "",
    lib_cppflags => "-DOPENSSL_USE_NODELETE -DB_ENDIAN",
    lib_defines => [  ],
    module_cflags => "-fPIC",
    module_cxxflags => undef,
    module_ldflags => "-Wl,-znodelete -shared -Wl,-Bsymbolic",
    multilib => "64",
    perl_platform => "Unix",
    perlasm_scheme => "void",
    shared_cflag => "-fPIC",
    shared_defflag => "-Wl,--version-script=",
    shared_defines => [  ],
    shared_ldflag => "-Wl,-znodelete -shared -Wl,-Bsymbolic",
    shared_rcflag => "",
    shared_sonameflag => "-Wl,-soname=",
    shared_target => "linux-shared",
    thread_defines => [  ],
    thread_scheme => "pthreads",
    unistd => "<unistd.h>",

Recorded environment:

    AR =
    ARFLAGS =
    AS =
    ASFLAGS =
    BUILDFILE =
    CC = /usr/bin/gcc
    CFLAGS =
    CPP =
    CPPDEFINES =
    CPPFLAGS = -D_LARGEFILE64_SOURCE
    CPPINCLUDES =
    CROSS_COMPILE =
    CXX = /usr/bin/g++
    CXXFLAGS =
    HASHBANGPERL =
    LD =
    LDFLAGS =
    LDLIBS =
    MT =
    MTFLAGS =
    OPENSSL_LOCAL_CONFIG_DIR =
    PERL =
    RANLIB =
    RC =
    RCFLAGS =
    RM =
    WINDRES =
    __CNF_CFLAGS =
    __CNF_CPPDEFINES =
    __CNF_CPPFLAGS =
    __CNF_CPPINCLUDES =
    __CNF_CXXFLAGS =
    __CNF_LDFLAGS =
    __CNF_LDLIBS =

Makevars:

    AR              = ar
    ARFLAGS         = qc
    CC              = /usr/bin/gcc
    CFLAGS          = -Wall -O0 -g
    CPPDEFINES      =
    CPPFLAGS        = -D_LARGEFILE64_SOURCE
    CPPINCLUDES     =
    CXX             = /usr/bin/g++
    CXXFLAGS        = -Wall -O0 -g
    HASHBANGPERL    = /usr/bin/env perl
    LDFLAGS         =
    LDLIBS          =
    PERL            = /usr/bin/perl
    RANLIB          = ranlib
    RC              = windres
    RCFLAGS         =

NOTE: These variables only represent the configuration view.  The build file
template may have processed these variables further, please have a look at the
build file for more exact data:
    Makefile

build file:

    Makefile

build file templates:

    Configurations/common0.tmpl
    Configurations/unix-Makefile.tmpl
s$

How can I drag out some more information on these failures ?

--
--
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional

Please can you include the output of running this command:
make TESTS=test_dtls test
Which version of OpenSSL is this?

I don't know any more what
C language spec the OpenSSL project complies with. If any.

OpenSSL project does not aim to comply with any exact particular C language spec version. It is mostly C90 but it can use some newer features if available.

So the dtls test is now passing???  If it is sometimes failing then can you capture the verbose (V=1) output when it fails?

> So the dtls test is now passing??? Is it consistently passing, e.g. if you run
>
> `make TESTS=test_dtls test`
>
> And then run that command several times, does it always pass or
> is it sometimes failing? If it is sometimes failing then can you capture
> the verbose (V=1) output when it fails?

I caught it !

.
.
.
    not ok 6 - test_swap_records
../../util/wrap.pl ../../test/dtlstest ../../apps/server.pem ../../apps/server.pem => 1
not ok 1 - running dtlstest

#   Failed test 'running dtlstest'
#   at test/recipes/80-test_dtls.t line 19.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

Test Summary Report
-------------------
80-test_dtls.t (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1, 50 wallclock secs ( 4.25 usr  0.02 sys + 45.11 cusr  0.40 csys = 49.78 CPU)
Result: FAIL
make[1]: *** [Makefile:2453: run_tests] Error 1
make[1]: Leaving directory '/opt/bw/build/openssl-3.1.2_gentoo_sparcv9.004'
make: *** [Makefile:2449: tests] Error 2

So now I have to figure out how to attach a gzip log file to this darn Microsoft Github trash.

not ok 6 - test_swap_records

Can you supply a bit more context from the log just before this?

Full logs would be helpful here, but this:
( 4.25 usr  0.02 sys + 45.11 cusr  0.40 csys = 49.78 CPU)
Suggests to me something failed during a cpu intensive operation.  Maybe you ran out of entropy generating a keypair for use in SSL?

> Full logs would be helpful here

I am trying to get those. Sadly I used tmux as a terminal and that thing seems
to hate the idea of a log file. I will get the data.

>
> ( 4.25 usr 0.02 sys + 45.11 cusr 0.40 csys = 49.78 CPU)
>
> Suggests to me something failed during a cpu intensive operation.
> Maybe you ran out of entropy generating a keypair for use in SSL?

I am not sure how I would know that. However, that is a good theory.

you could check /proc/sys/kernel/entropy_avail as you run the test, if you see it start to go down towards zero (admittedly it may be a hard thing to catch), that would suggest you are consuming entropy faster than you make it.  Newer linux kernels have a jitter source that allows for non-blocking entropy reads in most cases, but since you're building for sparc (and possibly running on solaris), you likely have an older, blocking pool.  If that turns out to be the case, you can build/run rngd (from my repo at https://github.com/nhorman/rng-tools ), and select a reasonable entropy source (jitter, or even a radio beacon if you have a sdr handy), and that should prevent entropy drains.

Actually this is Gentoo Linux where the kernel is :

s$
s$ uname -a
Linux sphinx 5.15.88-gentoo-sparc64 #1 Sun Mar 12 18:36:19 GMT 2023 sparc64 sun4u TI UltraSparc IIi (Sabre) GNU/Linux
s$

I have linux-6.4.8 all built and ready to go also.

Regardless I will find a way to keep an eye on /proc/sys/kernel/entropy_avail

Thank you for that cool info.

It may depend on kernel config.

jitter is supposed to be "always on" (see try_to_generate_entropy, available since 5.3.2).  Thats not to say it always works.  If you're on a system with a coarse cycle counter, it can fail, and you're back to blocking on entropy availability

Marking as inactive, to be closed if there is no update by the end of the 3.4 dev cycle

On 6/6/24 11:09, Neil Horman wrote:

Marking as inactive, to be closed if there is no update by the end of the 3.4 dev cycle

I will try to look into it within the week. The machine is doing a full
emerge/update cycle from Gentoo and that takes days on this old server.
--
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken


Change Withdrawn status to govuk-tag-yellow colour

Jira ticket URL
https://dfedigital.atlassian.net/browse/TEVA-2569

Review app deployed to https://teaching-vacancies-review-pr-3410.london.cloudapps.digital