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.

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

cluster_tools: In worker, use Python's defaults if logging_config is missing #1235

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

amotta
Copy link
Contributor

@amotta amotta commented Jan 14, 2025

This commit changes cluster_tools, such that Python's default values are used in the absence of a user-specified logging_config. In particular, this commit "increases" the default logging level from logging.DEBUG to Python's default of logging.WARNING. Without this change, some packages (e.g., H5py) are flooding the log. Not only does this result in large log files (tens of MBs per file for some of my jobs), but it also slows down execution.

In particular, Python's default log level is `logging.WARNING`, not
`logging.DEBUG`. Before this change, some packages (e.g., `h5py`) would
flood the log. Not only would this is large log files (hundreds of MBs),
but it would also slow down execution.
Copy link
Member

@hotzenklotz hotzenklotz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution.

"logging_config",
{"level": logging.DEBUG, "format": "%(asctime)s %(levelname)s %(message)s"},
)
logging_config = meta_data.get("logging_config", dict())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Using an empty dict will also reset our default logging format to Python's default ("%(levelname)s:%(name)s:%(message)s" Source Code). I think that is acceptable.

@hotzenklotz hotzenklotz enabled auto-merge (squash) January 16, 2025 12:24
@hotzenklotz hotzenklotz merged commit 38e3572 into scalableminds:master Jan 16, 2025
34 checks passed
@hotzenklotz
Copy link
Member

FYI, this change should be released as cluster_tools version v0.16.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants