Add configuration option to fail on warnings

See original issue on GitLab
In GitLab by [Gitlab user @jjardon] on Jul 30, 2018, 09:48
Background
It would be nice to have to a configuration parameter to make warnings errors (For example, for #483, we would like to make that an error)
Acceptance Criteria
It's possible to configure warnings to be fatal

In GitLab by [Gitlab user @tristanvb] on Jul 30, 2018, 10:37
NOTE: We already have this behavior for the overlapping files.
The configuration should be selective, so you should be able to:

Make warnings described in #483 fatal
Not make the overlapping file warnings fatal

The opposite is also possible.
It would be interesting to think up a better semantic for expressing this in the configuration, instead of using all separate keys, e.g.:
fatal-warnings:
- overlaps
- ref-not-in-track
- ...

Similarly, it would be interesting to also have a sweeping statement to make everything fatal, like gcc's -Werror:
# This could be a nice way of simply making every warning an error
fatal-warnings: True

Of course, if something nicer to express this is added, we should:

Deprecate the fail-on-overlap setting

Continue supporting it for backwards compatibility
But mark it as deprecated in the documentation, referring to the new preferred syntax

Ensure that either ways of expressing the overlap warnings being fatal, effect the cache key in exactly the same way

In GitLab by [Gitlab user @toscalix] on Jul 31, 2018, 16:02
marked this issue as related to #483

In GitLab by [Gitlab user @Qinusty] on Aug 3, 2018, 10:55
assigned to [Gitlab user @Qinusty]

In GitLab by [Gitlab user @Qinusty] on Aug 3, 2018, 15:28
Unfortunately, with the current _yaml implementation of composite_list().
Without a rethink of how we merge the default and project config, it will be impossible to have fatal-warnings be either a list or a boolean.
My proposed solution to this is simply have two possible configurations.
all-fatal-warnings: True
fatal-warnings:
- overlaps
  ref-not-in-track

An alternative solution would be to have a special warning which could be added to fatal-warnings to enable them all e.g.
fatal-warnings:
- ALL

In GitLab by [Gitlab user @tristanvb] on Aug 4, 2018, 10:33

Without a rethink of how we merge the default and project config, it will be impossible to have fatal-warnings be either a list or a boolean.

Ouch. Thanks [Gitlab user @Qinusty] for noticing this.
We have been using keys with multiple different value types as a matter of convenience in many aspects of the yaml format, starting with the depends list elements, since the beginning.
While it might be that you have stumbled on the first instance where composition can cause this to explode (dependency lists don't have this problem, and neither does the new string-or-list include directive ([[Gitlab user @]](https://gitlab.com/)) value as far as I can see), I wonder if this is really the only place that this problem is happening.
I do think it's well worthwhile to support these cases, though; making the format attractive and convenient is a big deal (especially since we guarantee backwards compatibility of the format, it means we can potentially deprecate things in the future, but we cannot ever stop supporting older expressions in the format).
Here is what I propose:

Lets not have a single switch to turn warnings into errors for now (i.e., let's only support the list for now)
Lets open a separate issue to ensure that the complication you've uncovered does not happen anywhere else, and explore how we can provide hints to _yaml.composite() to inform it how to behave in such a situation, if at all possible.

In any case, we will have around 6 months to make a more final call on this, when the newly added format API becomes frozen for 1.4.

In GitLab by [Gitlab user @tristanvb] on Aug 7, 2018, 11:21
Thinking further on this, this problem will also happen for the depends list, where the elements are either strings or dictionaries.
I think that:

When you use an ([[Gitlab user @]](https://gitlab.com/)) include directive, you have knowledge of what you are including and subsequently overriding in the including file.
In the case that there is a composition error in _yaml.composite(), we should be printing the provenance of both the target and source YAML nodes, such that the project author knows exactly what has gone wrong and can quickly fix it.

This means that when authoring a project, you have to choose how you express the fatal-warnings, and it seems like a pretty bad idea already to be including a file which overrides this value.
The default configuration in BuildStream's data/projectconf.yaml (which a user's project.conf is composited on top of), should not specify any fatal-warnings, so that should not cause any issues.
Does this make good sense ?

In GitLab by [Gitlab user @Qinusty] on Aug 7, 2018, 11:24
Yes, I was assuming that we would have examples within the default projectconf.yaml. This would cause composition errors.
If we are fine leaving these, perhaps in comment blocks then this should be fairly straight forward to implement it having Bool or List as a type.

In GitLab by [Gitlab user @Qinusty] on Aug 8, 2018, 14:47
mentioned in commit 70aa48caacb76aa0de0ff93357674efc4b273995

In GitLab by [Gitlab user @Qinusty] on Aug 9, 2018, 16:50
mentioned in commit c2f273d862b183c755db13386c0c4453ff45bbdc

In GitLab by [Gitlab user @Qinusty] on Aug 9, 2018, 17:02
mentioned in merge request !627

In GitLab by [Gitlab user @Qinusty] on Aug 9, 2018, 17:34
mentioned in commit 8ab05af5f6cb45b8a8d2aa5e676be549306b9c0a

In GitLab by [Gitlab user @Qinusty] on Aug 9, 2018, 17:53
mentioned in commit 2eeb5245ec71918090749a2de39bccf91f19249e

In GitLab by [Gitlab user @Qinusty] on Aug 9, 2018, 19:59
mentioned in commit 9ab0f33367347b696f9d450d8e1849a631486407

In GitLab by [Gitlab user @Qinusty] on Aug 10, 2018, 00:02
mentioned in commit 16b92edd67862ad3cda97d0f96f06db39b5ab3d7

In GitLab by [Gitlab user @Qinusty] on Aug 10, 2018, 14:04
mentioned in commit abe49b103c6d4b2cfdb9cc8474affa9199fc67c6

In GitLab by [Gitlab user @Qinusty] on Aug 10, 2018, 14:47
mentioned in commit c699371be69ceea8a788e08fbe455d0da8eda29b

In GitLab by [Gitlab user @Qinusty] on Aug 13, 2018, 08:44
mentioned in commit 657bc93203fe5525aa4ebea2063030ab2aee1b5f

In GitLab by [Gitlab user @Qinusty] on Aug 13, 2018, 08:48
mentioned in commit 8b5bdb7e50a721b44a2ed298ee971bfa5c7e5047

In GitLab by [Gitlab user @Qinusty] on Aug 13, 2018, 15:03
mentioned in commit 8c084159dc1f266b4a116b7aa4f10d22bf9cc2e3

In GitLab by [Gitlab user @Qinusty] on Aug 13, 2018, 15:53
mentioned in commit 5473346ed59fe6618b0ac98604eac73c3417ff02

In GitLab by [Gitlab user @Qinusty] on Aug 14, 2018, 18:02
mentioned in commit 52cb3c0952298138503cfdcade33c05f94dcafc9

In GitLab by [Gitlab user @Qinusty] on Aug 15, 2018, 10:08
mentioned in commit 1fc5598280619f021bc4c5eb1fa388dcb579ec92

In GitLab by [Gitlab user @Qinusty] on Aug 15, 2018, 10:24
mentioned in commit f8836416ec62bc34dc186d026740a1c2ae4db39d

In GitLab by [Gitlab user @Qinusty] on Aug 15, 2018, 12:16
mentioned in commit f3d58233793cf0249cbac51f72d5c459eb256d44

In GitLab by [Gitlab user @tristanvb] on Aug 15, 2018, 13:16
closed via merge request !627

In GitLab by [Gitlab user @tristanvb] on Aug 15, 2018, 13:16
mentioned in commit 76f34a633e43790eab6592c5f1385f00c5ba2e83

In GitLab by [Gitlab user @Qinusty] on Aug 2, 2019, 21:24
mentioned in commit e090d782a943ca4e81d37e0901b4dc1575a897d6

In GitLab by [Gitlab user @Qinusty] on Aug 2, 2019, 23:52
mentioned in commit 3832b6eb635d7dfb38c0f31596e3ee1859f0667a

In GitLab by [Gitlab user @Qinusty] on Aug 3, 2019, 22:33
mentioned in commit 9f37828ad0d652e43d13cac7895b0d671808e9cb


For Unix-style line endings for scripts, add .env to .gitignore

Description
This PR forces Unix-style checkout for .sh files, and adds the .env file to .gitignore.
Why is this needed
Cloning the sandbox repository on Windows and running vagrant up provisioner eventually fails like this:
[...]
    provisioner: + ./generate-envrc.sh eth1
    provisioner: /usr/bin/env: ‘bash\r’: No such file or directory
The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.

This is because the .sh files are treated as text files by git, and the line endings are converted to crlf on Windows. Because the .sh files are mounted into the Linux guest VMs, they would end up as with the crlf line ending on Linux, causing vagrant up to fail.
How Has This Been Tested?
Tested locally.
Because the scripts create symlinks in the /vagrant folder, you need to make sure that your Windows user has permissions to create symbolic links
How are existing users impacted? What migration steps/scripts do we need?
Fixes a bug.
Checklist:
I have:

[ ] updated the documentation and/or roadmap (if required)
[ ] added unit or e2e tests
[ ] provided instructions on how to upgrade

@qmfrederik please amend your commits to certify dco.


Migration problem

Error during migrations apply.
Migration not found: 2016_06_01_000005_create_oauth_personal_access_clients_table
Migration not found: 2016_06_01_000004_create_oauth_clients_table
Migration not found: 2016_06_01_000003_create_oauth_refresh_tokens_table
Migration not found: 2016_06_01_000002_create_oauth_access_tokens_table
Migration not found: 2016_06_01_000001_create_oauth_auth_codes_table
As a result can't login.

pull and rebuild the docker container and try again


feat: simplify and optimise

Dockerfile

removes unused stuff
Correctly sets $HOME
Plugin cache dir set

Scripts

New clean script to ensure sane environment prior to running tests and to tidy up afterwards

Makefile

clean target (and clean2) so we can ensure sane environment and clean up afterwards

Sorry. Good comments - responded


Lockdown fix 0 errors

1 $ ./Lockdown fix 0
[⚠ ] You are about to engage FIX mode which WILL make changes to your Mac 💻
[⚠ ] Do you want to continue? (y/N) y
[✅] mikey has chosen to continue

Results:
main:80: settings[${fix_setting_num}]: parameter not set
mikey 🐶 ~/Documents/Projects/mOSL

Probably related...
Executing ./Lockdown fix 1 tries to enable Automatic System Updates instead of  Automatic App Store Updates.
$ ./Lockdown fix 1
[⚠ ] You are about to engage FIX mode which WILL make changes to your Mac 💻
[⚠ ] Do you want to continue? (y/N) y
[✅] mikey has chosen to continue

Results:
  [✅] Enable Automatic System Updates

Automatic System Updates is setting 0 as show by ./Lockdown list
$ ./Lockdown list

Settings (21) that can be audited or fixed:
  (0) enable automatic system updates
  (1) enable automatic app store updates
...


NativeCommandError

I have a use case where we would like to apply a JEA configuration that allows an external program which is in the form of a command-line utility.  I have exposed the command in the  Visible External Commands section, but when I try to run the command from the configuration, I get an error:
.<command-line tool> --
+ CategoryInfo          : NotSpecified: (s:String) [], RemoteException
+ FullyQualifiedErrorId : NativeCommandError
Is there a workaround for this behavior?
Thanks!

@PaulHigin Do you know what can cause that Error ID?

I would need repro steps to investigate.  Have you tried running the command line tool in a normal remote session?  Do you get the error only on a JEA endpoint?

I found a thread regarding this issue: https://stackoverflow.com/questions/10666101/lastexitcode-0-but-false-in-powershell-redirecting-stderr-to-stdout-gives-n
This does in fact reproduce my error (exposing cmd.exe AS a VisibleExternalCommand - Note: I'm not using cmd.exe in that variable, this is just a sample to repro):

Enter-PSSession -ComputerName
cmd /c "echo Hello from standard error 1>&2"

I get the error:
[<server>]: PS C:\Users\<user>\Documents> cmd /c "echo Hello from standard error 1>&2"
cmd : Hello from standard error
    + CategoryInfo          : NotSpecified: (Hello from standard error :String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

This fails in a normal remote session also.  So it looks like the command line program I'm trying to allow writes to standard error and it's causing an issue when remoting with PowerShell.  I hope that helps to understand and hopefully there is a workaround.
Thanks!
Justin

Yeah, PowerShell remoting converts native error stream messages into PowerShell ErrorRecord objects.  This seems inconsistent with PowerShell a local PowerShell session.  Some people feel one way is correct and the other not correct.  There is an issue discussing this (mostly for PowerShell local session) on the PowerShell GitHub site.  (https://github.com/PowerShell/PowerShell/issues/4002)


Can you publish the source code? Thanks

Thank you for your interest to our work! The code has been released.


EACCES on generator-react-native-ignite block and i'm using sudo.

Hi, thank you for this project.
I'm facing issues when installing react-native-ignite with sudo npm install -g react-native-ignite
It seems the first part of react-native-ignite runs fine but when it starts to run the post install script npm i -g generator-react-native-ignite something goes terribly wrong and i get this stack trace.
> npm i -g generator-react-native-ignite

npm ERR! Linux 4.8.6-1-ARCH
npm ERR! argv "/usr/bin/node" "/usr/bin/npm" "i" "-g" "generator-react-native-ignite"
npm ERR! node v7.1.0
npm ERR! npm  v3.10.9
npm ERR! path /home/username/.npm/generator-react-native-ignite/1.11.0/package
npm ERR! code EACCES
npm ERR! errno -13
npm ERR! syscall mkdir

npm ERR! Error: EACCES: permission denied, mkdir '/home/username/.npm/generator-react-native-ignite/1.11.0/package'
npm ERR!  { Error: EACCES: permission denied, mkdir '/home/username/.npm/generator-react-native-ignite/1.11.0/package'
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'mkdir',
npm ERR!   path: '/home/username/.npm/generator-react-native-ignite/1.11.0/package' }
npm ERR!
npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:
npm ERR!     /usr/lib/node_modules/react-native-ignite/npm-debug.log
npm ERR! Linux 4.8.6-1-ARCH
npm ERR! argv "/usr/bin/node" "/usr/bin/npm" "i" "-g" "react-native-ignite"
npm ERR! node v7.1.0
npm ERR! npm  v3.10.9
npm ERR! code ELIFECYCLE

I tried to install sudo npm i -g generator-react-native-ignite alone, installing it worked but running $ ignite I still get command not found and installing still brings errors.
I also tried to create the folder it's failing to create, made it then installed react-native-ignite again but same error.
I'm running Archlinux OS and the stacktrace goes on by saying i need to update my node but i'm using nodejs: 7.1.0, npm: 3.10.9 and my system is updated.
I tried installing it with sudo yarn global add react-native-ignite it installed successfully but when i tried running ignite nothing ran, checking where it installs with yarn global bin it showed /usr/bin and i saw the ignite file in there.
Ran file /usr/bin/ignite found out it's a broken link to /usr/lib/node_modules/react-native-ignite/index.js which isn't there. :(
I also found this solution here didn't work, should i panic now?

Have you gone through these steps?
https://docs.npmjs.com/getting-started/fixing-npm-permissions
If so, maybe we can screen share when I return from vacation

@GantMan Thank you for your suggestions.
The prefix solution worked magic!


Add stable master branch

I would like a clean master branch.
So I've disabled auto-commits into master for all developers.
From now on we work this way:

Create a pull request for new features
Ask for review of a pull request (what I'm doing now, please sign off 💯 )
Merge into master after at least one review

@segfault16 @grigri kannst du den pull request mal reviewen und abzeichnen?


[API] Add missing parameters to NotebookOp

Resolves #147
Add parameters pipeline_name and experiment_name to NotebookOp() initialization.

@Tomcli -- FYI, I deployed this fix in a new mlx-api image to our "MLX prod" cluster (Kubeflow 1.3 + Datashim + MLX + multi-user) and Notebook with related DataSet PVC launch works now


Automatic Downloads

I Googled "git for windows", and the first result was http://git-scm.com/download/win. I clicked through and the download immediately started. I think it is inappropriate for the top Google result to result in an automatic download - perhaps the direct download page should not appear in Google search results, or perhaps the page should not automatically start the download when it is reached via Google (checking referrer URL maybe).
I apologize if I am just being picky and if this is not considered a real issue. But when I was looking for a URL to send a friend, I was instead greeted with an executable.
Thanks!

another issue that should be considered on #1179 @jasonlong

I think @masonsbro provided a very reasonable suggestion (inspect referring URL).  A correct one, in my opinion.  It is inappropriate for this page, or any other, to automatically download an executable when accessed from a Google search result, clicking on a link from an email, or even by clicking https://git-scm.com/download/win in a Github issue comment.  Not to mention basic usability issues for non-windows users.
Would all it take to fix this is for someone to generate a pull request?  I would be happy to if that's the way forward.

Thanks for your availability @mloyd .
I would say that a PR would solve this issue. As stated in the issue/comments, if we can reliably look at the referrer to disable the auto-download, then I would be all for it

Created pull request #1418


test: Add a sample generate_config yaml file.

Add a sample generation_config.yaml file for google-cloud-java.
This is intended to be a sample file to make sure we agree on the high level design of the config file. This does not intend to be a exhaustive list of parameters at this moment, we will refine the parameters along the way.

@suztomo @JoeWang1127 @diegomarquezp, I'm merging this PR now. Please add additional comments regarding the parameters in internal docs.
@diegomarquezp Please use this as a template and add more configs for libraries/GAPICs to the file, so that it can replace proto_path_list.txt in integration tests. Also, I'm expecting a separate config file for each handwritten library, looking at the content of proto_path_list.txt, we will need one for bigtable/logging/pubsub/storage. Supporting all handwritten libraries can be a separate PR, testing java-bigtable in this PR should be good enough.


feat(api): propagate spanContext only using API #1456

This one is a little bit tricky, since it was assume from the start that the API could only contains noop we put the context management utils into the core.
As you can see in this PR, we need to get the spanContext stored inside the context from the Noop implementation, there are two way do to this:

duplicating the declaration of the key, one in the API's noop tracer and one inside the core for general usage.
moving the key declaration inside the API, but while at it why not move them all ? Specially since they all use API interfaces.

Waiting for feedback before marking as non-draft !
Fixes #1456

I think the key shouldn't be duplicated. Having details like this in core results in further problems like various plugins depend on core.
As a result it's not possible to implement our own SDK but reuse plugins.
If the API is the entrypoint for plugins it should contain everything needed by them.

I was the one that originally put those context management functions in the SDK instead of API. The reason for this at the time was that I was thinking they were sdk-specific implementation details, but it has become clear since then that the API is not really that useful without them. I think they can be moved into the API.

I will update the PR to move them into the API then :+1:

I've rebased the PR


Cosmos DB ID must be all lowercase

Example name given in the tutorial contains capital letters.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: e931add5-8f69-f7ce-29df-31c3e4abaa96
Version Independent ID: b0cb60c0-dcf3-75e0-1929-18d9d6e4582e
Content: Create an instance of Azure Cosmos DB
Content Source: learn-pr/build-serverless-app/create-cosmosdb.yml
GitHub Login: @asajohnson
Microsoft Alias: asjohnso

Hi @charters - thanks for the feedback. We actually pulled this content down to rework it, but I'll make sure we keep that in mind in future revisions.


how to use nsq_to_file just generate one file per day

When I execute "nsq_to_file --topic=test --output-dir=/tmp --lookupd-http-address=<IP_ADDRESS>:4161"
It will generates one file per hour, twenty-four files everyday. Now I just want it to generate one file everyday. I don't find a parameter to work it. How should I do? Is there some suggestions.

try --datetime-format="%Y-%m-%d". nsq_to_file starts a new file when the effective/evaluated filename changes.

@ploxiln, Your suggestion is effective. I considered it just a parameter to modify file name, but It works.
nsq_to_file --topic=test --output-dir=/tmp --lookupd-http-address=<IP_ADDRESS>:4161 --datetime-format="%Y-%m-%d"


⚠ Bofrost.lu has degraded performance

In 3813065, Bofrost.lu (https://www.bofrost.lu) experienced degraded performance:

HTTP code: 200
Response time: 2705 ms

Resolved: Bofrost.lu performance has improved in 048c5d2 after 20 minutes.


vm: add start_index to iterator unwrap helpers

There's a 2048 max stack item count limit imposed by the VM which prevent our unwrap_iterator helpers from fetching all data from the iterator. By providing a start_index we can work around that limit.

coverage: 80.497% (-0.1%) from 80.593%
when pulling 62f2298459da0114393b1e37b1f0b50718f17f08 on unwrap_it
into eb6e410dec07be94f6d2143a3b0cf6ac32a0ce30 on master.


Presentation data

Create sensible data for use in the final presentation.

Coverage remained the same at 100.0% when pulling 1f2247fa7024acd48918b4873b496344d4e6b2fa on presentation_data into 63d661791b63e7de23faa7ad90128b155ac9a4b1 on dev.

Coverage remained the same at 100.0% when pulling 4c13c50a8e5a33d8361954d04702f1b8a0d98785 on presentation_data into 63d661791b63e7de23faa7ad90128b155ac9a4b1 on dev.

Coverage remained the same at 100.0% when pulling 65b03bc75505217d8a43da73fb1cba175401c9f1 on presentation_data into 63d661791b63e7de23faa7ad90128b155ac9a4b1 on dev.

Coverage remained the same at 100.0% when pulling e6c2bc9068272eebd8ad277f5e4a62a644260165 on presentation_data into 816c0d214c5fc79f74b4391b5ad151e90810225c on dev.

Coverage remained the same at 100.0% when pulling b80e92f1750661630c6bf25c3a764e90c0464d1a on presentation_data into 816c0d214c5fc79f74b4391b5ad151e90810225c on dev.

Coverage remained the same at 100.0% when pulling 259198a45a8a143e122bc65d547ef82380e7b7a7 on presentation_data into 816c0d214c5fc79f74b4391b5ad151e90810225c on dev.

Coverage remained the same at 100.0% when pulling fca7f4b141e4b7e05fa38ee49e5712216441fa6a on presentation_data into 4e3ae866952251f07625de389a9963d5241383cc on dev.

Coverage remained the same at 100.0% when pulling 1c7987daf559edbb2bb63429a5922517636b5407 on presentation_data into 3d9259217f1f285953effb05b879731911f96e16 on dev.

Coverage remained the same at 100.0% when pulling 081f5547a815bac212396ac29ebf77269bfd8fc6 on presentation_data into 69ba10381c37d4f9dfc076ab6ced7213e4dc629c on dev.


Implement Ioctx createReadStream

This patch also makes use of index.js. I think we should look at moving more stuff out of the native module and into index.js instead. We should try to make the native module as simple as possible and rely on pure javascript for doing trivial stuffs. Ex. the recent connection state checking could potentially have been implemented inside index.js, or in the future for implementing turning Rados errno codes into error objects.

Thanks. Yes we have already mentioned, I like the idea. This will allow more flexibility. For cons, I asked myself about the management of asynchronous operations still have an interest in the native module ? or are they better be moved to the js part?

I don't see how you can move the async operations outside the native module. However maybe the sync operations could be replaced by using something like node-sync (that would call the async methods). Not sure it's worth doing this though as it implies additional dependencies and probably some performance hit.

Can you merge? Or any changes you would like to see?

I have merged the PR. I'll test it. Thanks

I was thinking about your comment regarding simplifying the async/ sync operation in the native module. One way would be to make everything async, and the sync methods that have equivalent async methods would call the async method and block with a call to rados_wait_for_complete().
Also probably have a flag to block using rados_wait_for_safe() instead. That same flag could be used to decide when to call the user supplied callback in async methods (whether to call the callback when the operation is completed in memory, or safe to disk).


typo: contians -> contains

OPS Build status updates of commit 722a782:
:clock10: Preparing: average preparing time is 3 min(s) 17 sec(s)

OPS Build status updates of commit 722a782:
:clock10: Incremental building: average incremental building time is 54 min(s) 25 sec(s)

OPS Build status updates of commit 722a782:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

access/Concepts/Windows-API/retrieve-the-name-of-the-user-logged-on-to-the-network.md
:white_check_mark:Succeeded
View

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


attractor: add masori variants (and fix max cape typos)

Kept noticing my blowpipe was being tracked as draining charges too quickly so I took a look.
Also reworked the class a bit to reduce duplicate entries, and there were a couple typos with the max cape variants just using the ava's accumulator id:
https://github.com/geheur/weapon-charges/blob/9c368c8c183b1965107dc89d4dab6196738a5014/src/main/java/com/weaponcharges/AttractorDefinition.java#L37

added in 184c869158057f7201a54a51dbc7ff24a339e2cd


Allow Option to Change Image URL

Is your feature request related to a problem? Please describe.
The current options do not seem to provide a way to reference assets from a different URL than the bucket URL.
i.e. Assets may get uploaded to the bucket by referencing the bucket name but when retrieving the file from the bucket it would be nice if you could provide a different URL like a CDN you have setup that proxies the bucket. A very similar setup to GCPs documentation on adding a CDN to a bucket https://cloud.google.com/cdn/docs/setting-up-cdn-with-bucket.
I tried using the current baseUrl and basePath options but they do not seem to do what I'm suggesting.
Describe the solution you'd like
The solution I would like to see would make Strapi use a defined URL instead of the storage bucket URL https://cdn.some-site.com/{image-name} instead of  https://storage.googleapis.com/{bucket-name}/{image-name} when retrieving image. The admin panel of Strapi should reference the new image URLs when previewing the image and the API should return links with the new URL instead of the GCS bucket URL.

I would be happy to look into this feature myself if you think this is something you would want to add to this project but don't have time for at the moment.

Hi @itmayziii ,
baseUrl allow you to already do that, because  I'm using it :)
Keep your bucket name, and change your baseUrl with the subdomain / domain you want.
Per example :
baseUrl: 'https://static.domain.com'

Then, you will be able to access to your image with :  `https://static.domain.com/image-url.jpg`

All of the configuration needs to do on Google platform.
You need to configure CDN proxy with a load balancer.
If you follow the documentation, you will find, otherwise, let me know and I will try to help you to configure it.

Thanks,

@Lith you are 100% correct. I must have tested it wrong before. Thank you for your help.


networkmanager{,applet,plugins}, modemmanager: update

Motivation for this change
new versions everywhere
depends on #19304 and #19305
Things done

[x] Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)

[x] NixOS
[ ] OS X
[ ] Linux

[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
[ ] Tested execution of all binary files (usually in ./result/bin/)
[x] Fits CONTRIBUTING.md.

Tested with gnome 3.22 only. Plugins not tested for functionality since i dont have any of the required remotes.
I can report back when nox-review finishes.

I merged these three PR together and successfully bulit everything. Don't know if it actually works.

LGTM, but you should use the attribute names of the packages in the commit message as they appear in pkgs/top-level/all-packages.nix.

nox reported no errors (only webkitgtk214, but that's unrelated)

@fpletz fixed the commit messages


add initial support for tensorrt

What this PR does / why we need it:
Adds initial support for TensorRT Inference Server
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #117
Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

This change is

I imagine you tested this end to end

in-progress.. marked PR as wip :-)

Excellent question - yes it is a model format (in addition to the inference
server). You can convert from different frameworks into TensorRT format and
then serve using TensorRT Inference Server... TensorRT has its own layers
and everything, see:
https://docs.nvidia.com/deeplearning/sdk/tensorrt-developer-guide/index.html#layers
On Sun, Jun 2, 2019 at 8:08 PM Animesh Singh<EMAIL_ADDRESS>wrote:

@animeshsingh commented on this pull request.
Thanks for this @rakelkar https://github.com/rakelkar. Great addition -
only thing is the conceptual definition of a model vs a server here we may
need to call out
In ROADMAP.md
https://github.com/kubeflow/kfserving/pull/134#discussion_r289676768:

@@ -5,6 +5,7 @@
Objective: "Simplify the user experience and provide a low barrier to entry by minimizing the amount of YAML necessary to deploy a trained model."

High Level Interfaces

Deploy a Tensorflow model without specifying a Tensorflow Serving Technology.

Deploy a TensorRT model without specifying a TensorRT Serving Technology.

Is there a model of 'type' TensorRT? We have TF, PyTorch, scikit-learn
etc....seems a bit odd to say TensorRT 'model'.
In pkg/apis/serving/v1alpha1/kfservice_framework.go
https://github.com/kubeflow/kfserving/pull/134#discussion_r289676876:

@@ -29,9 +29,9 @@ type FrameworkHandler interface {

const (
// ExactlyOneModelSpecViolatedError is a known error message

ExactlyOneModelSpecViolatedError = "Exactly one of [Custom, Tensorflow, SKLearn, XGBoost] must be specified in ModelSpec"

ExactlyOneModelSpecViolatedError = "Exactly one of [Custom, Tensorflow, TensorRT, SKLearn, XGBoost] must be specified in ModelSpec"
// AtLeastOneModelSpecViolatedError is a known error message

Same as above.....TensorRT is a specialized inference server, rather than
a model 'type'.....
In pkg/apis/serving/v1alpha1/kfservice_types.go
https://github.com/kubeflow/kfserving/pull/134#discussion_r289677043:

@@ -63,6 +64,15 @@ type TensorflowSpec struct {
Resources v1.ResourceRequirements json:"resources,omitempty"
}

+// TensorRTSpec defines arguments for configuring TensorRT model serving.
+type TensorRTSpec struct {

ModelRepositoryURI string json:"modelUri"

The way I see it is when we support "tensorrt", we're supporting the model
format, not specifically the tensorrt model server. Our implementation
detail may be to point to an existing modelrepository or manually download
to a local one we create.
Is there a specific model format for TensorRT?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubeflow/kfserving/pull/134?email_source=notifications&email_token=AA2CTBIVRXCAJ3FEWQJB5JTPYSDMTA5CNFSM4HSB6MU2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2KU7KY#pullrequestreview-244666283,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA2CTBOWKXP34HDRVMTU6ITPYSDMTANCNFSM4HSB6MUQ
.

--
Rakesh Kelkar

This is failing with the following message :(
    message: "Revision \"simple-string-default-bjvmq\" failed with message: Unable
      to fetch image \"nvcr.io/nvidia/tensorrtserver:19.05-py3\": unsupported status
      code 500; body: \r\n<title>500 Internal Server Error</title>\r\n\r\n
500 Internal Server Error
\r\n
nginx/1.14.2\r\n\r\n\r\n."

@yuzisun @ellis-bigelow do you know why this is failing on the revision? If it cant get to nvcr.io I would have expected it to create a pod and have the pod fail on image pull issues...?

This is failing with the following message :(
    message: "Revision \"simple-string-default-bjvmq\" failed with message: Unable
      to fetch image \"nvcr.io/nvidia/tensorrtserver:19.05-py3\": unsupported status
      code 500; body: \r\n<title>500 Internal Server Error</title>\r\n\r\n
500 Internal Server Error
\r\n
nginx/1.14.2\r\n\r\n\r\n."

@yuzisun @ellis-bigelow do you know why this is failing on the revision? If it cant get to nvcr.io I would have expected it to create a pod and have the pod fail on image pull issues...?

Looks like just the pod image pulling error got propagated to the revision, did you see the pod ?

Seems that KN tries to resolve the container to SHA but nvcr doesnt support the API... tried pushing to dockerhub and trying from there, currently blocked on a different issue: https://github.com/NVIDIA/tensorrt-inference-server/issues/378

KN tries to resolve the container to SHA but nvcr doesnt support the API

Opened issue to investigate on knative: https://github.com/knative/serving/issues/4355

@ellis-bigelow  @yuzisun  appears we have modify knative config to get nvcr.io to work:
registriesSkippingTagResolving: nvcr.io

Any thoughts on where we should document this (or if we can automate it using some kustomize patch or something?)

@ellis-bigelow @yuzisun appears we have modify knative config to get nvcr.io to work:
registriesSkippingTagResolving: nvcr.io

Any thoughts on where we should document this (or if we can automate it using some kustomize patch or something?)

Ah I see knative revision controller does the image digest resolve, I think we can have a kfserving installation guide on our doc with the knative custom configurations?

@rakelkar I can find the shared library here /usr/local/cuda-10.1/targets/x86_64-linux/lib/stubs/libnvidia-ml.so but LD_LIBRARY_PATH is set to /opt/tensorrtserver/lib:/usr/local/cuda/compat/lib:/usr/local/nvidia/lib:/usr/local/nvidia/lib64.

@ellis-bigelow  @yuzisun @animeshsingh  this works now and is ready for review

@ellis-bigelow I uploaded the nvidia sample model to a bucket I created on GCS, if you can move that to some official location and let me know the URL I can update and delete my personal bucket :)

/retest

/retest

Hey guys - this is fairly small PR compared to the other open ones... would appreciate a quick review :) @ellis-bigelow @yuzisun

@ellis-bigelow resolved all your comments...

@ellis-bigelow can take a look?

I think it's mostly there but I want someone else's comments too :). Will
look tonight or tom morning.
On Wed, Jun 19, 2019, 4:31 PM rakelkar<EMAIL_ADDRESS>wrote:

@ellis-bigelow https://github.com/ellis-bigelow can take a look?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubeflow/kfserving/pull/134?email_source=notifications&email_token=AATXLCHAMH42X6DBSJ5AFYDP3K6UVA5CNFSM4HSB6MU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYDSBXA#issuecomment-503783644,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AATXLCBEPU6HUGR3IGWGLQTP3K6UVANCNFSM4HSB6MUQ
.

/lgtm
/approve


Handle Project producing zero columns

It would be nice if the user could provide a pipeline with more preprocessing subpipelines than necessary. For example, if a pipeline contains a branch with one-hot encoding for string columns, but the data only has numeric columns, it would be convenient if it worked anyway. Unfortunately, some sklearn operators raise an exception when their input data has zero columns. This issue proposes preventing that exception during fit, and possibly even pruning them from the pipeline returned by fit.
Example:
import sklearn.datasets
X, y = sklearn.datasets.load_digits(return_X_y=True)

from lale.lib.lale import Project, ConcatFeatures
from lale.lib.sklearn import LogisticRegression, OneHotEncoder

proj_nums = Project(columns={"type": "number"})
proj_cats = Project(columns={"type": "string"})
one_hot = OneHotEncoder(handle_unknown="ignore")
prep = (proj_nums & (proj_cats >> one_hot)) >> ConcatFeatures
trainable = prep >> LogisticRegression()

print(f"shapes: X {X.shape}, y {y.shape}, "
      f"nums {proj_nums.fit(X).transform(X).shape}, "
      f"cats {proj_cats.fit(X).transform(X).shape}")

trained = trainable.fit(X, y)

This prints:
shapes: X (1797, 64), y (1797,), nums (1797, 64), cats (1797, 0)
Traceback (most recent call last):
  File "~/tmp.py", line 17, in <module>
    trained = trainable.fit(X, y)
  File "~/git/user/lale/lale/operators.py", line 3981, in fit
    trained = trainable.fit(X=inputs)
  File "~/git/user/lale/lale/operators.py", line 2526, in fit
    trained_impl = trainable_impl.fit(X, y, **filtered_fit_params)
  File "~/git/user/lale/lale/lib/sklearn/one_hot_encoder.py", line 145, in fit
    self._wrapped_model.fit(X, y)
  File "~/python3.7venv/lib/python3.7/site-packages/sklearn/preprocessing/_encoders.py", line 385, in fit
    self._fit(X, handle_unknown=self.handle_unknown)
  File "~/python3.7venv/lib/python3.7/site-packages/sklearn/preprocessing/_encoders.py", line 74, in _fit
    X_list, n_samples, n_features = self._check_X(X)
  File "~/python3.7venv/lib/python3.7/site-packages/sklearn/preprocessing/_encoders.py", line 43, in _check_X
    X_temp = check_array(X, dtype=None)
  File "~/python3.7venv/lib/python3.7/site-packages/sklearn/utils/validation.py", line 72, in inner_f
    return f(**kwargs)
  File "~/python3.7venv/lib/python3.7/site-packages/sklearn/utils/validation.py", line 661, in check_array
    context))
ValueError: Found array with 0 feature(s) (shape=(1797, 0)) while a minimum of 1 is required.

Martin, we are exploring if we can add constraints to the planner after using the Lale Project operators to customize the search space for the dataset's characteristics.  If that works out, this has lower priority.  However we very much would like the ability to project text.  Thanks much!

One thing that is not clear to me is what is the expected behaviour here. scikit-learn's answer is to explicitly fail because we are doing something that is not valid here. Do we want to automatically correct the pipeline in a data-dependent manner?
Also +1 on text and maybe datetime. I wonder what pandas data types we can leverage here.


Feature: Select and move multi elements

Description
Select and move multiple elements
https://github.com/brmodeloweb/brmodelo-app/assets/881231/26693d53-c5df-42d6-8823-a5b52a540c2b
closes #412 #408 #395 #429 #370

Thank you for accept and implement my idea.


Fail to create a new environment in another region

Describe the bug
When I try to create a new environment located in another region (different than the original data.all deployment region), although the prerequisites are completed (e.g. cdk bootstrap, CloudFormation stack creation, etc.), it will prompt the error "CDKToolkitNotFound".
How to Reproduce
*P.S. Please do not attach files as it's considered a security risk. Add code snippets directly in the message body as much as possible.*

data.all deployed in ap-southeast-1 (Singapore)
create a new environment in ap-east-1 (Hong Kong)
it will fail and prompt the error “CDKToolkitNotFound”
if i change the new environment region to use ap-southeast-1 (Singapore), then it can be registered successfully

Expected behavior
It should allow users to have the deployment infra in one region, and the linked environments in other regions.
Your project
No response
Screenshots
OS
AWS Deployment
Python version
3.8
AWS data.all version
1.2.2
Additional context
No response

Hi @aaronchong888 , I tried to re-create your error but I could not. It looks like you bootstrapped in the wrong region. Can you go to the show account and check in CloudFormation that the CDK toolkit stack is in the Honk Kong region.
btw, I removed your image it was showing an AWS Account Id


Fixing JSON output from notebook functions

What

Fixing convert_to_json and rubicon_to_json functions so the JSON output structure is always {"project" = [list of projects], "experiment" = [list of experiments]} with "project" and "experiment" as the only top level keys
Can only see this update as I forgot to cut a branch for the original notebook updates and committed to jsonpath-poc branch directly

How to Test

Run cells to see output

one last thing - you're doubling up the "experiment" key in either convert_to_json or experiment_to_json:
{'project': [{'name': 'jsonpath',
   'id': '087af64d-bcd2-493b-9c49-b5eb9cdcd382',
   ...
   'experiment': [{'experiment': [{'project_name': 'jsonpath',
       'id': '76bb9d43-aa4f-4a20-bf65-8962c0c7bf38',
       ...
}

it should just be
{'project': [{'name': 'jsonpath',
   'id': '087af64d-bcd2-493b-9c49-b5eb9cdcd382',
   ...
   'experiment': [{'project_name': 'jsonpath',
       'id': '76bb9d43-aa4f-4a20-bf65-8962c0c7bf38',
       ...
}

you probably just wanna remove the top level "experiment" key from the beginning of experiment_to_json and only return a list of [experiment jsons], rather than a dict of {"experiment": experiment jsons}


Mapping by bwa error

Hi Lijia Yu,
When I was running sveditor.py script with an inversion variant as an input, the job was killed when the step of mapping by bwa starts
File "/home/asmmahmoud/ctg/varben/VarBen-master/bin/../varben/common/bamconvert.py", line 14, in _call
raise Exception("Cmd Error: %s" % cmd)
Exception: Cmd Error: bwa mem -R "@RG\tID:A4-NA24143-WEST_L1\tLB:RGLB\tPU:1\tSM:A4-NA24143-WEST" -t 1 /home/asmmahmoud/ctg/ref/GRCh38_full_analysis_set_plus_decoy_hla.fa /home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.to_1.fq /home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.to_2.fq >/home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.sam
Could you help me fix it ?

Hi,
this is error msg: [E::bwa_set_rg] the read group line is not started with @RG

I think the problem is the read group tag should be @RG but not @rg.
can you run the line below to see if it works?
bwa mem -R "@RG\tID:A4-NA24143-WEST_L1\tLB:RGLB\tPU:1\tSM:A4-NA24143-WEST" -t 1 /home/asmmahmoud/ctg/ref/GRCh38_full_analysis_set_plus_decoy_hla.fa /home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.to_1.fq /home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.to_2.fq >/home/asmmahmoud/ctg/varben/inv_test/tempDir/edit.remap.sam

It gave this output
[M::bwa_idx_load_from_disk] read 0 ALT contigs
[M::process] read 2 sequences (0 bp)...
[M::mem_pestat] # candidate unique pairs for (FF, FR, RF, RR): (0, 0, 0, 0)
[M::mem_pestat] skip orientation FF as there are not enough pairs
[M::mem_pestat] skip orientation FR as there are not enough pairs
[M::mem_pestat] skip orientation RF as there are not enough pairs
[M::mem_pestat] skip orientation RR as there are not enough pairs
[mem_sam_pe] paired reads have different names: "/1", "/2"
Could you help me to run the script without changing the read group tag ?
I used same bam file with another variant list, it was completed successfully without this error at mapping by bwa step
bwa mem -R "@RG\tID:A4-NA24143-WEST_L1\tLB:RGLB\tPU:1\tSM:A4-NA24143-WEST" -t 1 /home/asmmahmoud/ctg/ref/GRCh38_full_analysis_set_plus_decoy_hla.fa /home/asmmahmoud/ctg/varben/A4/Hom1_mut/tempDir/edit.remap.to_1.fq /home/asmmahmoud/ctg/varben/A4/Hom1_mut/tempDir/edit.remap.to_2.fq >/home/asmmahmoud/ctg/varben/A4/Hom1_mut/tempDir/edit.remap.sam

I think the problem is similar to #23 now.
I will check with the demo in a few weeks.
But I cannot help you generate the edited data in a short time.
If you still need to get the edited data recently, can you try using bamsurgeon?
https://github.com/adamewing/bamsurgeon

Hi Lijia Yu,
Thank you for your reply. I can't use bamsurgeon because I need to be consistent with other edited bam files generated by VarBen. I will wait for your update :)

I have tried to edit a CNV in the example data EGFR_sort.bam, and I didn't encounter any error. My bwa version is  0.7.17-r1198-dirty.
chr7    55248067        55249038        cnv     2.5     gain

I am not able to help you check what's the problem in your bam file.


Remove "experimental" from open systems simulator, begin supporting continuous angle rotations.

This PR parallels that at https://github.com/microsoft/qsharp-runtime/pull/1009 by:

Removing "experimental" from Python APIs for the open systems simulator.
Updating dependencies modified in https://github.com/microsoft/qsharp-runtime/pull/1009.
Misc code cleanup, reduction of warnings.

Retargeting to main to match https://github.com/microsoft/qsharp-runtime/pull/1009.


Check styles from layer when app is initialized

So basically the problem found is:

Create a map with a dataset of points (so many of them).
Apply a complex analysis, it will take several hours.
Close the builder, because you need that analysis finished in order to keep going.
Open the builder after X hours, and check that the analysis has finished but the styles are pink, because the style type is the NONE one (@saleiva's favourite).

We used to change the style when an analysis has finished, but if the tab is closed... 🙈 .
About the approach, I don't like the idea of change the style when the application has started and without a user change, but looks like the "way" of doing it.
CR: @alonsogarciapablo
cc @rochoa @saleiva @javisantana
Fixes #9684.

[x] Pending tests.

Left some comments/questions. About the approach, It's scary, but I couldn't think about anything better to solve it this. I was worried that this approach wouldn't work if the user went directly to see the embed after X hours, but I guess the map needs to be re-published anyway... so 👍
I guess you'll be adding some minimal testing @xavijam. Thanks!

Added tests and applied feedback

A bit better now (although I still think that _resetStylesIfNoneApplied  has too much detail).
👍

:joy: come on! it is really simplified, and If it is possible to create more helper methods, I'd do it.


User can choose a style (minimal, journal, scrapbook)

What does this entail? A different font and background.


[Go Live][Nov 29]

Push to live

Learn Build status updates of commit d8ea0d8:
:x: Validation status: errors
Please follow instructions here which may help to resolve issue.

File
Status
Preview URL
Details

:x:Error

Details

[Error: CannotMergeCommit] Cannot merge commit d8ea0d86e68f2d4e03bd9ab82630647146803f84 in branch main of repository https://github.com/MicrosoftDocs/msteams-docs into branch live (commit 650fafbf31465808cc33afb45a75351c86a9e5a4). Please follow this documentation: https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/ to use git.exe to resolve you content conflicts locally and then push to remote.

For more details, please refer to the build report.
If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Learn Build.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:Try searching the learn.microsoft.com contributor guidesPost your question in the Learn support channel

Learn Build status updates of commit c8ee11b:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

msteams-platform/concepts/design/overview-analytics.md
:white_check_mark:Succeeded
View

msteams-platform/concepts/design/plan-code-instrumentation.md
:white_check_mark:Succeeded
View

msteams-platform/concepts/design/strategize-measure.md
:white_check_mark:Succeeded
View

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching the learn.microsoft.com contributor guidesPost your question in the Learn support channel


SVar doc improvements

@PyPablo mentioned that he succeeded in using the SVar service but that the documentation did not necessarily explain everything clearer. Let's consult on this, and get the documentation improved somewhat.

Reviewing the current documentation, it seems reasonably complete.


tpm1: clean up of extend command

Removes unused variables.
Signed-off-by: Daniel P. Smith<EMAIL_ADDRESS>

Reviewed-by: Ross Philipson<EMAIL_ADDRESS>


Makefiles now check for virtualenv and pkg-config.

When installing Neon on a brand-new MacBook Pro running OS X Sierra, I encountered an absence of pkg-config as well as virtualenv. I added checks and error messages to appropriate Makefiles to cover these cases.

Thank you Milind for trying neon!
http://neon.nervanasys.com/docs/2.0.0/installation.html  indeed shows both virtualenv and pkg-config as pre-requisites. But you have done more to let users know this easily and avoid having to go to the page which the user might not be aware of. Your change is very helpful, thank you!


[$1000] Dev: Web - Edit popup menu hides the Paypal username while open first time

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

Action Performed:

Go to Settings > Payments
Add Paypal payment method if not added
Click on Paypal username to edit or delete it

Expected Result:
Popup menu should not hide the Paypal username
Actual Result:
Popup menu hides the Paypal username while open first time(second time it's working fine)
Workaround:
Unknown
Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.46-2
Reproducible in staging?: n/a
Reproducible in production?: n/a
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/93399543/85dea8ae-32b9-43fb-9dae-2bd9c984c916
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690606718500609
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01acb5ccd86249930e
Upwork Job ID:<PHONE_NUMBER>369784832
Last Price Increase: 2023-08-01

Thanks! Triaging this now.

Heading ooo we're still looking for proposals, thanks in advance for the help moving this along BZ team!

@huzaifa-99's proposal looks good to me.  We need to ensure that it doesn't add extra space when the expensifyWallet is not enabled.
🎀 👀 🎀  C+ Reviewed

Yeah, let's wait for @johnmlee101 to assign and then you can start with the PR.

I am currently testing/reviewing the revised PR, which fixes this current issue, as well as popover position on the payment button, when wallet is enabled.
@zanyrenney @muttmuure @johnmlee101 I would like to point out that while @huzaifa-99 fixed this current issue, we were blocked in testing it as the Popover was overlapping the Button in the main branch/staging for the wallet enabled accounts. They investigated the issue, spent time in working out the solution , and have raised the revised PR that fixes the popover position issue in general. Considering, they had to fix the additional scope, I think we should double the price for this job.

I've reviewed the PR. Thanks @huzaifa-99 for the patience here.
@zanyrenney @muttmuure @johnmlee101 did you folks get a chance to look at my previous comment?

I want to make sure @muttmuure @zanyrenney @johnmlee101 got a chance to look at this comment. Thanks.

@johnmlee101 @muttmuure @zanyrenney gentle bump on the previous messages.

@mananjadhav

Considering, they had to fix the additional scope, I think we should double the price for this job.
Thanks. I will take this to the team for internal discussion.

Hi @mananjadhav we discussed this internally. This does not qualify for an increased payment based on the PR. I will proceed with the manual payment for @huzaifa-99

Okay thanks for clarifying.
Would've appreciated if it did qualifiy. The popover was broken when we created the PR, and instead of taking a route of creating a fresh issue, we intended to fix the issue as it blocked this current PR. I think for next time would it make sense to create a fresh issue, put the current one on Hold, then revisit?

@zanyrenney Can you post the payment summary? I need to raise a request on NewDot for this.

Awaiting payment summary to get this paid out in NewDot.

Payment summary
@mananjadhav $1000 as contributor+ [TO BE PAID VIA ND]
@huzaifa-99 $1000 for contributor [INVITED ON UPWORK - please accept job invite]
no urgency bonus as merge took 6 days
@gadhiyamanan $250 fore reporting bonus [PAID]

Reviewed the details for @mananjadhav. $1,000 approved for payment via NewDot based on BZ summary.

@huzaifa-99 i sent you an offer! can you accept please?

@mananjadhav and @huzaifa-99 now paid.
@mananjadhav $1000 as contributor+ [TO BE PAID VIA ND]
Closing!


Isn't the softmax of the attention matrix calculated incorrectly?

The softmax is currently calculated as follows:
attn = dots.softmax(dim=-1)
The softmax is only applied on the last dimension instead of the whole matrix. Instead, I think it should be implemented like that:
attn = rearrange(dots, 'b h i j -> b h (i j)').softmax(dim=-1).view_as(dots)
Is this correct or am I missing anything?

@PascalHbr Hello! So I believe it is correct the way it is
The image has already been cut into patches and linearized into tokens. The dimensions of i and j are actually the similarity of every patch to every other patch including itself. You then force the network to pick (with softmax) the patch that should be attended to. This is done on the j dimension, so that each patch i picks one of the other patches that is most important.
Perhaps you were thinking there is some correspondence between i, j and the height and width, but i and j are actually height * width.

Thanks a lot for the clarification. You are right indeed :)


Gson TypeAdapters and Factories for TypeHandlers

Motivation
Terasology uses a custom serialization system loosely based on the Gson 1.x architecture. To specify custom serialization for specific types, Terasology requires the use of implementations of TypeHandler<T> -- a Terasology-specific interface. As such, serialization handlers for Terasology-introduced types like Vector3i and Color implement TypeHandler and are only compatible with the Terasology serialization system.
To help use existing TypeHandlers for serializing Terasology types to JSON via Gson, the JsonTypeHandlerAdapter class, a legacy Gson 1.x serialization adapter, was created. JsonTypeHandlerAdapter can be used to register Gson type adapters for each type manually; however, this approach is suboptimal for reasons stated below.
Since manually registering a TypeHandler/JsonTypeHandlerAdapter for every possible generic variant of popularly serialized generic types like Set, Map and List is very cumbersome (if not downright impossible), Terasology uses a TypeSerializationLibrary to create/load TypeHandlers for types on the fly. This allows TypeHandlers for generic types like Map to be easily created on demand, rather than having to hardcode handlers for each generic variant.
Gson 2.x included the TypeAdapterFactory interface which, much like TypeSerializationLibrary creates TypeAdapters for types on demand. However, it is currently not possible to levy the convenience of TypeAdapterFactory/TypeSerializationLibrary in Gson serialization with existing TypeHandlers because JsonTypeHandlerAdapter, a legacy Gson 1.x type adapter, is not supported by Gson 2.x TypeAdapterFactory.
To work around this, this PR introduces a new GsonTypeHandlerAdapter -- a Gson 2.x serialization adapter very similar in functionality to the old JsonTypeHandlerAdapter (renamed in this PR to LegacyGsonTypeHandlerAdapter) -- which allows the use of an existing TypeHandler as a Gson 2.x TypeAdapter. This PR also introduces two TypeAdapterFactory implementations -- GsonTypeHandlerAdapterFactory and GsonTypeSerializationLibraryAdapterFactory -- which create GsonTypeHandlerAdapter instances for types from their handlers on the fly.
Since GsonTypeHandlerAdapter instances are meant to be used only via a TypeAdapterFactory implementation, the JsonTypeHandlerAdapter is retained and renamed to LegacyGsonTypeHandlerAdapter in cases where individual TypeHandlers/TypeAdapters are required.
Impact
With the Terasology serialization system, there is no way to arbitrarily serialize objects of any compound type purely containing serializable data (like class GameMap { Map<String, Rect2i> regions; }) -- you must create a TypeHandler for the class (GameMap in our example) which manually (de)serializes all fields.
With the introduction of JsonTypeHandlerAdapter, you could levy the automagic of Gson by registering a JsonTypeHandlerAdapter in a Gson object for types having a TypeHandler. However, this would still be very restrictive as individual type handlers/adapters would not be able to handle all generic variants for a generic type like Map or Set (the very reason for which TypeSerializationLibrary was created).
With the introduction of the new TypeAdapter and TypeAdapterFactory implementations in this PR, it is now possible to levy the convenience of on-demand TypeHandler creation of TypeSerializationLibrary by using a GsonTypeSerializationLibraryAdapterFactory, which internally uses a TypeSerializationLibrary to create TypeHandlers for types on the fly (which are then wrapped by a GsonTypeHandlerAdapter). This also means that if a TypeHandler for the type of a given object is not found, the object will be serialized using the default Gson rules, allowing for more flexibility.
This makes serializing compound types using Terasology's handlers very easy:
class GameMap {
    Map<String, Rect2i> regions;
    EntityRef mainEntity;
}

static Gson createGson(TypeSerializationLibrary typeSerializationLibrary) {
    return new GsonBuilder()
        .registerTypeAdapterFactory(
            new GsonTypeSerializationLibraryAdapterFactory(typeSerializationLibrary)
        )
        .create();
}

String serializeGameMap(GameMap gameMap) {
    Gson gson = createGson(entityManager.getTypeSerializerLibrary());    // Includes EntityRef handler
    return gson.toJson(gameMap);
}

GameMap deserializeGameMap(String json) {
    Gson gson = createGson(entityManager.getTypeSerializerLibrary());    // Includes EntityRef handler
    return gson.fromJson(json, GameMap.class);
}

Changes

Added GsonTypeHandlerAdapter
Added GsonTypeHandlerAdapterFactory
Added GsonTypeSerializationLibraryAdapterFactory
Added ReflectionUtil.readField helper
Renamed JsonTypeHandlerAdapter to LegacyGsonTypeHandlerAdapter

Caveats

When GsonTypeSerializationLibraryAdapterFactory encounters a type which does not have a TypeHandler in TypeSerializationLibrary, the TypeSerializationLibrary.getHandlerFor logs errors. Logging errors, however, is unnecessary in this case because when a TypeHandler is not found, Gson switches to its default serialization handlers and is (more often than not) able to serialize the type correctly. This is specifically noticeable when serializing an object of a compound type with no existing TypeHandler (like GameMap in the above example). This can be worked around by making getHandlerFor log errors only on request.
TypeSerializationLibrary.getHandlerFor returns null when a Map type with non-String keys is passed to it, raising an error in the stock Terasology serializer and ensuring that only Maps with String keys are serialized. In Gson, however, when a non-String keyed Map is serialized, the keys are serialized using toString(), effectively breaking deserialization. This can be prevented by either a) making Gson exclude non-String key Maps manually (using an ExclusionStrategy or similar); or b) Enabling complex Map key serialization in Gson, which serializes each key to a string, thus allowing deserialization.

Considerations

[ ] Add a static helper method (like in the example above) that creates a Gson object for a TypeSerializationLibrary while keeping in mind the above caveats, especially the second one.
[ ] Either a) make Gson exclude non-String key Maps manually (using an ExclusionStrategy or similar); or b) enable complex Map key serialization in Gson, which serializes each key to a string, thus allowing deserialization. Refer to the link for more details.

Comments and suggestions (especially on the Considerations) are welcome.

Hooray Jenkins reported success with all tests good!

This can be worked around by making getHandlerFor log errors only on request.

TypeSerializationLibrary logged at "error" severity when the process failed.   I'd recommend logging  at "warning" severity when TypeAdapterFactory uses a default serialization handler.

@mkienenb I greatly appreciate your recommendation, but I think it will be better to not log anything in this case. TypeSerializationLibrary.getHandlerFor does log errors when it cannot find/create a TypeHandler for the given type, which is what I assume you mean by "when the process failed". This is understandable when using the Terasology serialization system since type serialization is impossible in the absence of a TypeHandler. However, when the method is used in this context, the absence of a TypeHandler for a type does not signify a failure -- Gson can still serialize the type using its own default object serialization. As such, I do not see the need to log warnings when getHandlerFor fails to return a TypeHandler; there is nothing to warn the user about. If no TypeHandler is found and Gson fails to serialize the object on its own, Gson will throw an exception.
Furthermore, if we continue logging warnings/errors in this case, the logs will be spammed with these (unnecessary, according to me) warnings/errors. Errors will be logged for every type that Gson encounters which does not have a TypeHandler, and this will be a quite frequent phenomenon. For example, the work in progress record and replay module serializes thousands of events during a record, and each of these events is of a type that does not have a corresponding TypeHandler and has to go through the Gson object serialization process. For each event object that is serialized, getHandlerFor will log an error for the absence of a TypeHandler. This will lead to the log being spammed with thousands of missing TypeHandler messages which are irrelevant since the serialization will continue anyway.

@eviltak It wasn't clear to me whether having the Gson 2.x  default serialization triggered was intended as a band-aid temporary workaround until the mod author got around to fixing it himself, or if Gson 2.x default serialization was intended to be the right way for serializing those objects.    The quoted line (and the fact it was in the caveats section) made me think that the mod author was supposed to fix these.   Your response seems to indicate that is not the case.

@mkienenb so that's where the misunderstanding stemmed from :smile: I listed this point under caveats because I hadn't gotten around to fixing it yet and wanted to discuss it with others. Gson 2.x default serialization is meant to be the right way for serializing these objects, as you correctly noted.

Some testing has revealed that deserialization of a Map with a primitive (but non-String) key type like Integer fails with the GsonTypeSerializationLibraryAdapterFactory. As per valid JSON, Gson serializes primitive keys (like 31) to a string by including quotes around the serialized primitive value (like "31"). When these keys are deserialized, the GsonTypeSerializationLibraryAdapterFactory ensures that the Terasology type handler is used for the primitive type and passes the string-serialized value ("31") to the type handler. However, primitive type handlers in Terasology expect to simply get the numeric value (31), take one look at the quotes ("31"), consider it a string and return a null value. The way I see it, this has two workarounds:

Make Gson exclude non-String key Maps manually (using an ExclusionStrategy or similar) -- Caveat 2 option a); or
Extend the primitive type handlers in Terasology to deserialize serialized primitive values enclosed in a string/quotes.

Good find. I don't know if this is much of an answer as much as an obvious preference but the less custom code and the more in compliance we are with the frameworks we use (including upgraded versions) the better, IMHO. And the harder it is to screw up code the better as well :-)
If authors have to remember to add in enableComplexMapKeySerialization somewhere or take special steps to deal with values inside maps (or end up with unusually ugly JSON like the second example in the link) then that's bad. If a helper method can hide all that and make it easy (and pretty) then great. Especially if it can be enforced somehow.
We did just do a major release, but are still in alpha, so if we should aim to overhaul this area further (rewriting all type handlers) for improvements that'll be worth it then by all means aim for it. Should LegacyGsonTypeHandlerAdapter or anything related maybe be marked as @deprecated  to make it clearer what may need effort to be fully up to date?
Trying to poke people occasionally to get more review on this.

I do plan on wrapping all required settings like calling enableComplexMapSerialization or adding an ExclusionStrategy for non-String keyed maps in a helper method that creates a "Terasology recommended" Gson object for serialization (consideration 1).
Regarding the ugliness of enableComplexMapSerialization -- Gson will, by default, serialize maps with complex keys to a list of key value pairs (the ugly second example). However, to retain a map-like structure, we could serialize the keys to JSON, and store them as a string JSON key. This will retain the key: value structure of JSON maps and should, in theory, not require us to extend type handlers to deserialize serialized primitive values enclosed in a string/quotes.
Marking LegacyGsonTypeHandlerAdapter as deprecated may be a good idea. GsonTypeHandlerAdapterFactory fulfills more or less the same functionality as multiple registeredLegcayGsonTypeHandlerAdapters (mapping serialization of specified types to their TypeHandlers), and we do not know if Gson will continue to support legacy Gson 1.x type adapters in later releases.

Hooray Jenkins reported success with all tests good!

Thanks for all the extra care, attention, and patience on this one :-)
Merged it locally in a mega-workspace and tried a variety of ways to provoke errors but couldn't find any. All appears to be well!


Added JavaScript Sample in CONTRIBUTING.md

Fixes #288
By submitting this pull request I confirm I've read and complied with the below declarations.

[x] I have read the Contribution guidelines and I am confident that my PR reflects them.
[x] I have followed the coding guidelines for this project.
[x] My code follows the skeleton code structure.
[x] This pull request has a descriptive title. For example, Added {Algorithm/DS name} [{Language}], not Update README.md or Added new code.
[x] This pull request will be closed if I fail to update it even once in a continuous time span of 7 days.

@singhpratyush please review again, do correct me if I've gone wrong somewhere.
Thanks

@singhpratyush done the requested changes, switched to TAB spacing but didn't get the point of

2 TAB spacing throughout

Have a look if this is appropriate.

@Anshuman-Verma This looks ok now.

@aviaryan @singhpratyush I think this  should do it.

Switched back to TAB indentation of 2

Finally ! :laughing:


Update to 2.4 nor luma 10.0.1 works

What model of system are you using?

[ ] Old 3DS
[ ] Old 3DS XL
[ ] 2DS (non-XL)
[ ] New 3DS
[x] New 3DS XL
[ ] New 2DS XL

What is your current Luma3DS version?
If you are running an hourly version of Luma3DS, please manually update to a stable version as I will no longer deal with people who are using hourlies.

[ ] Luma3DS v7.1 + boot9strap v1.0/v1.1
[ ] Luma3DS v8.0 + boot9strap v1.2+
[ ] Luma3DS v8.1 + boot9strap v1.2+
[ ] Luma3DS v8.1.1 + boot9strap v1.2+
[ ] Luma3DS v9.0 + boot9strap v1.2+
[x] Luma3DS v9.1 + boot9strap v1.2+

What version of Luma Updater are you using?

[ ] v2.0
[ ] v2.1
[ ] v2.1.1
[ ] v2.1.2
[ ] v2.2
[x] v2.3

How are you running Luma Updater?

[ ] .cia
[x] .3dsx + HBL/Rosalina

What problems are you experiencing?
Please tell us what problems you are facing.
Update to updater 2.4 nor update to Luma 10.0.1 work. In both cases the extract of the zip file fails. For updater 22.4 :
Extracting lumaupdater.3dsx[ERR]
FATAL: Could not find 3Ds/lumaupdater/lumaupdater.3dsx in zip file
With Luma 10.0.1 (with skipping updater update):
Top screen:
Reason for failure: DOWNLOAD FAILED
Bottom screen:
Extracting payload [ERR]
FATAL: Could not open archive (SzArEx_Open)
FATAL
Could not get sighax payload...
What is the exact error code you are receiving?
Please give the exact error code the app gives you.
See above
Steps to reproduce
Just run 2.3 and try to either update to 2.4 or use 2.3 to update luma.

Should be fixed now. It was a mistake on my part as I didn't upload all of the files in the release to match how the app looks for it.

Update to 2.4 worked and update to 10.0.1 afterwards worked aswell. Thanks :)

You're welcome! :)


graph: change Edge interface to include ID method

This is a major breaking change in the graph packages.
Updates gonum/graph#109.
Please take a look.

Would this change allow graph with self-loops?

There is currently no in principle restriction on self-edges, the choice is by policy, but yes, the multigraphs should allow self-edges.

Something that we had not considered previously was using the same kind of approach that we now use for weighting edges; the equivalent here would have been to keep Edge as it is and add a new MultiEdge (or something with a less horrible name) with the ID() int64 method. This would have had the advantage of not breaking the current API and not requiring the useless ID method for edges used in simple graphs, but the disadvantage of making interop between simple graphs and multi graphs require a wrapper adding an ID method.
Having merged this change and the rudeness that it brought, I'm reluctant to revert this and do that, but if @mewmew and @vladimir-ch think that that is a better approach, I'll go ahead and do it. I'm pretty evenly balanced on which is better even without considering the breakage.
Thoughts?


Laravel 10

To upgrade to Laravel 10 (before this package is tagged for 10), use the following:
composer config minimum-stability dev
composer config prefer-stable true
composer require "laravel-json-api/laravel:^3.0" --no-update
composer require "laravel-json-api/testing:^2.0" --dev --no-update
composer up "laravel-json-api/*" cloudcreativity/json-api-testing

These are the only breaking changes at the moment - though I'm not planning any more. It's just a major version bump for new version of Laravel plus dropping PHP 7.4 and 8.0.
Changed

Upgraded to Laravel 10 and set minimum PHP version to 8.1.
BREAKING If using the laravel-json-api/cursor-pagination package, you now need to passed the schema's id field
to the paginator's make() method. I.e. use CursorPagination::make($this->id())

Fixed

BREAKING #190 The JSON:API media type now needs to be
sent in the Accept header for a delete resource request. Previously there was no checking of the Accept media
type, so anything could be sent. This is incorrect as the JSON:API specification shows the Accept header as
application/vnd.api+json for delete resource requests.

Released as 3.0.0. To upgrade:
composer require laravel-json-api/laravel --no-update
composer require laravel-json-api/testing --dev --no-update
composer up "laravel-json-api/*" cloudcreativity/json-api-testing


add missing error code context

I believe this accomplishes the goal you were seeking. This is what I did:
git@github.com:jugglinmike/jshint-0x005.git
cd jshint-0x005
npm install
npm test

The approach I used was thus:

Isolate test (./node_modules/nodeunit/bin/nodeunit tests/unit -t testRawOnError)
Insert logging line showing JSHINT errors object.
Find location in codebase where errors are populated.
Add missing context.

I feel like I'm flying 10,000 feet above the problem you're trying to solve and that I must've missed something (this was too easy).
I hope this is truly all you needed my friend!

This is exactly what we needed, Tyler :). The brevity of the change is what made it hard for me to write myself. As someone with prior familiarity with the codebase, it's important for anything I write to be distinct from what came before. That's not easy to do with such a small change set!
However, work from someone without that prior knowledge doesn't warrant the same level of scrutiny. If they write a patch (even the very same patch), then their contribution would be a novel work as a matter of course.
...which is how I'd describe this change. Thank you!


Question: Integration within Eclipse IDE

For this maven-plugin to automatically run inside Eclipse, I guess a m2e connector is needed.
I found one https://github.com/masterzen/m2e-protoc-connector, but this plugin is not supported https://github.com/masterzen/m2e-protoc-connector/issues/7
Can you reference in README proper use within Eclipse (so that developers don't need to run mvn generate-source from command line after edits)

It runs automatically, give it a try. (m2e mapping is included)

It worked
http://stackoverflow.com/questions/40426366/automatically-generate-java-from-proto-with-maven-m2e-in-eclipse-ide

I think README should be updated to mention that.

@os72 when I modify a proto file the java files aren't being updated. If I clean the Eclipse project they are correctly rebuilt. Is this the correct behavior or is there something I'm missing? It's almost like it's not scanning for changes.
(I can open a separate issue if you want, it just seemed sort of on topic here.)

Try <cleanOutputFolder>true</cleanOutputFolder>
It's not super-reliable without running project-clean. I'm not an Eclipse expert, if it can be improved please let me know

@os72 I created wiki page https://github.com/os72/protoc-jar-maven-plugin/wiki/Eclipse . It would be better place for now.
@JacksonBailey For re-generation just added http://stackoverflow.com/questions/40575671/automatically-refresh-generated-source-in-eclipse-and-maven
I guess maven plugin can't do that is purely IDE features like incremental build or compiling/re-generated on for files being edited.

Closing


There should be specialized normalizers for E-Mails

Is your feature request related to a problem? Please describe.
Given I have an E-Mail address that includes unicode characters. When its normalized in the database using the UpperInvariantLookupNormalizer, those unicode characters are not properly escaped into Punycode using the System.Globalization.IdnMapping class.
When registering a custom normalizer, it will apply to email addresses and user names, even in use cases when the user name is not an email address.
Describe the solution you'd like
I propose to have specialized versions of ILookupNormalizer for e-mail addresses and user names:
public interface IUserNameLookupNormalizer : ILookupNormalizer {}
public interface IEmailLookupNormalizer : ILookupNormalizer {}

as well as appropriate default implementations that remove diacritic symbols, or escape punycode respectively.
Describe alternatives you've considered
The current workaround is to implement custom IUserEmailStore<TUser>, UserManager<User>, and RoleManager<Role> derived classes and overwrite the methods which use the normalized username or the normalized email address. One can that implement the interfaces above and provide the normalizers using Dependency Injection.

btw: the class Microsoft.AspNetCore.Identity.UpperInvariantLookupNormalizer should probably be sealed, since the implementation is trivial.

@blowdart yes, that's correct.

@blowdart @ajcvickers
How about we make a breaking change and just add NormalizeName and NormalizeEmail (and getting rid of the old Normalize) in ILookupNormalizer instead of adding two new marker interfaces.  Our default implementation can continue to just ToUpper both, but then its an easy drop in to tweak the normalization of either/both however people like.

@HaoK  that would be a possible solution. I'm just unsure if it aheres to the SRP to do so. And to have four interfaces (INormalizerBase, IEmailNormalizer, IUserNameLookupNormalizer, AND INormalizer) seems like overkill.

@HaoK Sounds okay to me.

Works for me, SRP be damned. There's clean, and then there's pragmatic and usable :)

@MovGP0 , I am facing a situation with unicode characters that I never had to deal with. Do you mind saying what is the common strategy when dealing with unicode, as it seems you are accustomed to that? For instance, do you store the emails as punycode ascii in the db, and then display them as unicode in the UI, or do you store them as unicode? As far as I can say some browsers will send you punycode while others send you content as is, so there must be some sanitation in place to homogenize the data.
Thanks
https://github.com/aspnet/AspNetCore/issues/9107

@ponant I store the email twice. once as unicode (nvarchar), exactly how the user typed it in. I use this address for presenting it in the UI.
Then I store it a second time normalized as punycode and uppercased (varchar). I use this one for authentication and searching.

@MovGP0 , assuming you use type=email, I don't think this works because of the browsers specific behavior. In all  my tests if I enter<EMAIL_ADDRESS>then firefox will pass as unicode up to the server but chrome will send you the punycode. So I expect that many of your db entries have punycode in nvarchar. How did you deal with that then?

@MovGP0 furthermore I find it tricky to find a solution that remains consistent. For instance if you allow user to enter an email or url and he enters say http://faß.de . This link in FF will display as is and route to it. But inm Chrome and Edge http://faß.de will redirect to https://www.bayern-fass.de/ .

if I enter<EMAIL_ADDRESS>then firefox will pass as unicode up to the server but chrome will send you the punycode.

There are multiple methods to work around this.

First you can use <input type="text"> instead of <input type="email">. This will give you the correct formatting, but will mess up autofill.

Convert the email address to the unicode format before viewing it to the user.

Convert the email address to the unicode format before storing it in the database.

Don't do anything of this sort and only handle this case when the customer complains.

Note that there is no right/wrong here. It's more of an personal choice.

This link in FF will display as is and route to it. But inm Chrome and Edge http://faß.de will redirect to https://www.bayern-fass.de/ .

I would not resolve domain redirects, since they may change. Instead, you only need to do two things:

parse the mail address using the System.Uri class, in order to make sure it looks valid (remember to prefix the mailto: protocol when the user has not provided it). Don't use a hack like Regex for this.

use the address the user has entered and send an e-mail with an one-time token, in order to verify that the address exists and can be properly resolved.I usually just write an URL with the token as query parameter that the user can click; as well as preformatted instructions (where the user has to navigate to and copy & paste the token), in case the spam filter has filtered the URL.

I agree but the main issue is that the user thinks he typed an email but you may receive its puny-coded value on your server because the browser decided to do so. Since chrome is the dominant in the market you will get<EMAIL_ADDRESS>instead.  In addition he might in some rare occurrence register to the website using chrome and the day after try to log on with FF. In that case he will not be able to login because FF will send to your server the umlaut ü.
So I think for your strategy 1 or 2 to be complete you would need to
-Receive input on the server and assume it is either unicode (Firefox) or punycode ascii or normal ascii
-Check if it has been punycoded by the browser or not, e.g. by using the IdnMapping class with try and catch (as its methods to convert back and forth to punycode will throw).
-Save to the db this unicode value (which again is not necessarily what the browser has sent you, but which is what the user wanted) and save the punycode value.
In short it would be the same as you proposed except that you cannot just take the value sent by the browser and assume this is what he meant.
If you agree with all of this then I understood you.

how can I remove any ILookupNormalizer injection in identity core3 preview4?
I use identity 2.2 and can't use ILookupNormalizer on ApplicationUserManager.
My contracture ApplicationUserManager :
public ApplicationUserManager(
            IApplicationUserStore store,
            IOptions<IdentityOptions> optionsAccessor,
            IPasswordHasher<TblUsers> passwordHasher,
            IEnumerable<IUserValidator<TblUsers>> userValidators,
            IEnumerable<IPasswordValidator<TblUsers>> passwordValidators,
            ILookupNormalizer keyNormalizer,
            IdentityErrorDescriber errors,
            IServiceProvider services,
            ILogger<ApplicationUserManager> logger,
            IHttpContextAccessor contextAccessor,
            IUsedPasswordsService usedPasswordsService)
            : base((UserStore<TblUsers, TblOrganizationChart, AbfaContext, int, TblUserClaim, TblUserOrganizationChart, TblUserLogin, TblUserToken, TblRoleClaim>)store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger)

When I add ILookupNormalizer on AddCustomServices, I have this error:

Method 'NormalizeName' in type 'Project.Core.Identity.CS.CustomNormalizers' from assembly 'Project.Core, Version=<IP_ADDRESS>, Culture=neutral, PublicKeyToken=null' does not have an implementation.

after remove "services.AddScoped<ILookupNormalizer, CustomNormalizers>();" I have this error:

'Method 'NormalizeKey' in type 'Project.Core.Identity.ApplicationUserManager' from assembly 'Project.Core, Version=<IP_ADDRESS>, Culture=neutral, PublicKeyToken=null' does not have an implementation.'

I don't need to use Normalizer.
How to solve this?


Mapbox Version 5

Mapbox has moved to 5.x version numbers - this PR updates the podspec to use newer mapbox versions (a quick test has not revealed any issues with clusterkit and 5.x mapbox)

Thanks!


Feat/td 25871

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.94%.  Consider uploading reports for the commit b92de57 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #247      +/-   ##
==========================================
+ Coverage   80.81%   80.94%   +0.13%
==========================================
  Files          20       20
  Lines        2710     2724      +14
==========================================
+ Hits         2190     2205      +15
+ Misses        520      519       -1

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


📦⚠ NPM Dependencies Issues- Missing/Unused packages - Update

💬 Various NPM packages need to be updated/removed/or fixed. Did npm update and was able to fix some but still have more to go through.
Used depcheck to check for unused dependencies. Here are the results-
Unused dependencies
* @fortawesome/fontawesome-free
* @testing-library/user-event
* bootstrap
* sass
* sass-loader
Missing dependencies
* eslint-config-react-app: ./package.json
* @typescript-eslint/parser: ./package.json
* eslint-plugin-import: ./package.json
* eslint-plugin-flowtype: ./package.json
* eslint-plugin-jsx-a11y: ./package.json
* eslint-plugin-react-hooks: ./package.json
* @typescript-eslint/eslint-plugin: ./package.json
* babel-eslint: ./package.json
* eslint-plugin-react: ./package.json
* eslint-plugin-jest: ./package.json
* eslint-plugin-testing-library: ./package.json

☞ Good issue for anyone that knows node packages

I can take a look at this so  basically we need to update the packages if possible and remove the unused ones am i right?

@Siddharth9890 yes the old unused dependencies can be removed.

Assigning issue #46 to @Siddharth9890.


Error with base dir <empty> : Found absolute local link

I'm checking html files passing a folder arg to Lychee :
      - name: Link checker
        uses<EMAIL_ADDRESS>        id: lychee
        with:
          args: --verbose --no-progress --exclude=w3.org --exclude=schema.org --exclude-mail build/client/**/*.html
      - name: Fail if there were link errors
        run: exit ${{ steps.lychee.outputs.exit_code }}

Since today, i've the following error :
Error: Error with base dir `<empty>` : Found absolute local link "/favicons/apple-touch-icon.png" but no base directory was set. Set with `--base`.

I never used the base arg and it's worked fine until today.
Anything related to this issue ?

Even I face this error since quite some time now. See here: https://github.com/joesan/joesan-me/runs/3627064361?check_suite_focus=true

Yeah this is the same as https://github.com/lycheeverse/lychee/issues/320. There is a fix at https://github.com/lycheeverse/lychee/pull/338. Once that is merged it will work again.

Sorry for the inconvenience. This should be fixed now without requiring any changes in the workflow files. See #45.
We pinned lychee to a slightly older version before we introduced file checking. On top of that, there is a PR for fixing the issue upstream, so that the new behavior will be opt-in in the future: https://github.com/lycheeverse/lychee/pull/338.
If you still face any issues, please reopen the issue. 😅


Added 3 participants

Added 3 participants to the Dataspace

Added a participants.json containing the participants name
Added a system test involving company2 and company3

Follow up story
Right now, if a user updates the participants.json, it will break the Verify step because Verify uses outputs from the deploy job with hardcoded names. Participants names are mentioned in different places and it would be better if they are mentioned in only one place. I created a Spike to investigate how to fix the problem.

@ouphi PR description mentions Added a system test involving company2 and company3. But I do not see it in PR diff. is it missing ?

@ouphi PR description mentions Added a system test involving company2 and company3. But I do not see it in PR diff. is it missing ?

@cpeeyush, I got comments about removing this test because company3 has the same setup as company1 and company2 so we agreed to remove it. Thanks for letting me know I updated the description now.