Skip to content

fix(emissions_tracker): raise ValueError for LOGGER output without logging_logger (#1412) - #1416

Open
Prasanthmax wants to merge 1 commit into
mlco2:masterfrom
Prasanthmax:fix/logger-issue-1412
Open

Prasanthmax wants to merge 1 commit into
mlco2:masterfrom
Prasanthmax:fix/logger-issue-1412

Conversation

@Prasanthmax

Copy link
Copy Markdown

Closes #1412

Root cause

_init_output_methods() unconditionally appended self._logging_logger
to self._output_handlers when OutputMethod.LOGGER was requested.
Since self._logging_logger defaults to None unless the caller
explicitly passes logging_logger=LoggerOutput(...), forgetting that
argument silently put None into the handlers list.

The actual failure is worse than a visible crash — it's swallowed
inside a suppress() block in _persist_data(), so tracker.stop()
raises nothing and just silently produces no output.

Fix

Raise a clear ValueError at construction time when
OutputMethod.LOGGER is requested without a logging_logger, instead
of letting None propagate.

Testing

  • Added test_logger_output_raises_without_logging_logger and
    test_logger_output_succeeds_with_logging_logger.
  • Updated test_save_to_flags_map_to_output_methods_and_warn and
    test_decorator_flush, which relied on the old silent-None behavior
    without ever supplying a real logger.
  • Full test suite passes locally (651 passed) excluding pre-existing
    Windows-only failures unrelated to this change (RAPL's Linux
    /sys/class/powercap assumptions, and one CLI test that shells out
    to a Unix-only command).

Thanks @sohammishra864-wq for reporting this.

…gging_logger

Fixes mlco2#1412. OutputMethod.LOGGER silently appended None to
_output_handlers when logging_logger wasn't provided, causing an
AttributeError inside a suppress() block later in flush()/stop() —
swallowed silently, giving no error and no output.

Now raises a clear ValueError at construction time instead.

Added test_logger_output_raises_without_logging_logger and
test_logger_output_succeeds_with_logging_logger in
test_emissions_tracker.py.

Updated two existing tests that relied on the old silent-None
behavior without ever supplying a real logger:
test_save_to_flags_map_to_output_methods_and_warn and
test_decorator_flush.
@Prasanthmax
Prasanthmax requested a review from a team as a code owner September 18, 2026 15:32

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OutputMethod.LOGGER without logging_logger appends None to handlers, crashes on flush()

1 participant