-
Notifications
You must be signed in to change notification settings - Fork 685
Fix config for Trace level logging within the ConfigBuilder class #6040
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
base: master
Are you sure you want to change the base?
Conversation
…sue nextflow-io#6039) Signed-off-by: kdesnos <[email protected]>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
The unit test error seems to occur in some code I didn't touch at all.. |
Note that an alternative solution would simply be to remove the call to |
…IALS is not defined, following changes in commit d4fadd4 introduced in PR nextflow-io#5216 Signed-off-by: kdesnos <[email protected]>
|
||
if( isTowerEnabled || isDataEnabled || LoggerFactory.getLogger(nextflow.config.ConfigBuilder).isTraceEnabled() ) |
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.
I think we can just remove these conditions and resolve the config always
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.
I never understood why we resolve it here anyway, when it is already resolved here
nextflow/modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy
Lines 329 to 333 in 2c9dd9b
final builder = new ConfigBuilder() | |
.setOptions(launcher.options) | |
.setCmdRun(this) | |
.setBaseDir(scriptFile.parent) | |
final config = builder .build() |
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.
Because this it's done to render it as a string using
nextflow/modules/nextflow/src/main/groovy/nextflow/config/ConfigBuilder.groovy
Lines 908 to 909 in e4af230
.setShowClosures(true) | |
.setStripSecrets(true) |
maybe it can be done reusing the config object already build, but never looked into that
Fix issue #6039