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: remove extra call to setup_logger() #2809
base: main
Are you sure you want to change the base?
Conversation
@johanneskoester could you take a look at this when you get a chance? Would be good to get this fixed before moving forward with monitoring plugin. |
self.snakemake_api.setup_logger( | ||
stdout=executor_plugin.common_settings.dryrun_exec, | ||
mode=self.workflow_api.workflow_settings.exec_mode, | ||
dryrun=executor_plugin.common_settings.dryrun_exec, | ||
) |
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.
The reason for this extra call is that with calling execute, the dryrun info might be new, as well as the exec mode. One could of course offer a method to just update these in the logger api.
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.
Hmm, I'm not sure I follow. It seems setup logger is called here
Line 141 in 8a80bda
self.setup_logger(mode=workflow_settings.exec_mode) |
How can the the info be new by time execute is called? Regardless, I think you're right, we can provide setters for these in the logger
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.
Could we just do this instead of calling setup_logger()?
self.snakemake_api.setup_logger( | |
stdout=executor_plugin.common_settings.dryrun_exec, | |
mode=self.workflow_api.workflow_settings.exec_mode, | |
dryrun=executor_plugin.common_settings.dryrun_exec, | |
) | |
logger.stdout = executor_plugin.common_settings.dryrun_exec | |
logger.mode = self.workflow_api.workflow_settings.exec_mode | |
logger.dryrun = executor_plugin.common_settings.dryrun_exec |
Quality Gate passedIssues Measures |
Description
Removes seemingly extra call to setup_logger() in api, see #2797 for details.
Still not super familiar with API changes, so @johanneskoester please advise.
QC
docs/
) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).