-
Notifications
You must be signed in to change notification settings - Fork 652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for nested params with the same name #2485
Conversation
ce82477
to
7d39b2a
Compare
modules/nextflow/src/test/groovy/nextflow/config/ConfigParserTest.groovy
Show resolved
Hide resolved
…th the same names at different levels of nesting. Create a deepMerge for command line params to avoid deleting values at the same level on override. Signed-off-by: jay Carey <jay@fulcrumgenomics.com>
e849217
to
6f2a8ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome patch, this was really tricky!
Find attached a patch to fix also a failing test, plus one more test to check the dotted notation of the same snippet
Signed-off-by: jay Carey <jay@fulcrumgenomics.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks
Released along with version 21.12.1-edge |
- Clone `scope` if found for current key to avoid overwriting params with the same names at different levels of nesting. - Create a deepMerge for command line params to avoid deleting values at the same level on override. Signed-off-by: jay Carey <jay@fulcrumgenomics.com>
This fixes #1982, #2166, #2247 and #2358
The first three issues were caused by a lack of cloning which allowed mutability of values where they should have been immutable.
The last issue required a deep merge vs a
putAll
to avoid overwriting multiple values at the same level when you are overriding them on the command line.