Skip to content

Support stdlib-style logger arguments#66

Open
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/logger-exception-propagation
Open

Support stdlib-style logger arguments#66
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/logger-exception-propagation

Conversation

@fallintoplace

Copy link
Copy Markdown

Summary

  • teach the shared logging wrappers to accept stdlib-style % arguments and exc_info= while preserving rank0_only= behavior
  • cover the wrapper compatibility directly with a hermetic logger test
  • cover the HF export worker guarantee with a regression that proves _worker_exception is recorded when the background export fails

Why

HFExportCallback._save_and_upload() logs worker failures with stdlib-style logger arguments. The project logger wrappers only accepted (message, rank0_only=True), so that error path could raise TypeError before _worker_exception was assigned.

I chose to fix the shared wrapper instead of only rewriting the callback call site because the repo already has other callers using the same %-style logging pattern. This keeps the callback guarantee intact and removes the same footgun for those existing call sites too.

Validation

  • uvx ruff check /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/utils/log.py /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/utils/log_test.py /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/callbacks/hf_export_test.py
  • uvx ruff format --check /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/utils/log.py /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/utils/log_test.py /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/callbacks/hf_export_test.py
  • PYTHONPATH=/Users/hoangvu/Code/OSS/cosmos-framework uvx pytest --noconftest /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/utils/log_test.py /Users/hoangvu/Code/OSS/cosmos-framework/cosmos_framework/callbacks/hf_export_test.py -o addopts=

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant