Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: remove extra call to setup_logger() #2809

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: remove extra call to setup_logger() #2809

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c406b69
remove extra call to setup_logger()
cademirch Apr 13, 2024
38895eb
Merge branch 'main' into fix-issue#2797
johanneskoester Apr 28, 2024
3a02178
Merge branch 'main' into fix-issue#2797
fgvieira May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions snakemake/api.py
View file
Open in desktop
Expand Up @@ -523,12 +523,6 @@ def execute_workflow(
"For local execution, --shared-fs-usage has to be unrestricted."
)

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,
)
Comment on lines -526 to -530
Copy link
Contributor

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.

Copy link
Contributor Author

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

snakemake/snakemake/api.py

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

Copy link
Contributor Author

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()?

Suggested change
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


if executor_plugin.common_settings.local_exec:
if (
not executor_plugin.common_settings.dryrun_exec
Expand Down