Skip to content

fix: preserve preconfigured logging#755

Open
schultzjack wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
schultzjack:codex/388-preserve-configured-logging
Open

fix: preserve preconfigured logging#755
schultzjack wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
schultzjack:codex/388-preserve-configured-logging

Conversation

@schultzjack

Copy link
Copy Markdown

Summary

  • track successful configure_logging() calls in the logging module
  • skip interface default logging setup when logging was already configured
  • add regression coverage for DataDesigner() preserving a prior debug logging config

Testing

  • uv run --group dev pytest packages/data-designer-config/tests/test_logging.py packages/data-designer/tests/interface/test_data_designer.py -q
  • make check-config check-interface
  • make test-config test-interface

Closes #388

Signed-off-by: schultzjack <schultzjack@users.noreply.github.com>
@schultzjack schultzjack requested a review from a team as a code owner June 17, 2026 00:17
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@github-actions

Copy link
Copy Markdown
Contributor

Linked Issue Check

Issue #388 has not been triaged yet. A maintainer needs to review
the issue and add the triaged label before this PR can be merged.

You can continue working on the PR in the meantime. The check will
re-run automatically once the issue is triaged.

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where DataDesigner.__init__ would silently overwrite a caller's pre-configured logging setup (e.g. debug level) with the interface's default INFO config. The fix introduces a module-level _logging_configured flag in logging.py that is set after a successful configure_logging() call, and guards the default-logging call in _initialize_interface_runtime() behind an is_logging_configured() check.

  • logging.py: adds _logging_configured = False, sets it to True at the end of configure_logging(), and exposes is_logging_configured().
  • data_designer.py: wraps the default configure_logging() call in _initialize_interface_runtime() with if not is_logging_configured() so any prior configuration is preserved.
  • Tests: updates test_initialize_interface_runtime_runs_once to reset _logging_configured via monkeypatch (keeping the mock assertion reliable), and adds two new tests — one unit test for the flag itself, and one end-to-end regression that verifies a DEBUG config survives DataDesigner() construction.

Confidence Score: 5/5

Safe to merge — the change is minimal, targeted, and fully covered by new and updated tests.

The fix is a one-flag, one-guard change with no impact on the happy path (logging not pre-configured) and correct behaviour on the new path (logging pre-configured). The existing test_initialize_interface_runtime_runs_once was correctly patched to reset the new flag so its mock assertion stays valid. The regression test exercises the real code path end-to-end and cleans up logger state in a finally block.

No files require special attention.

Important Files Changed

Filename Overview
packages/data-designer-config/src/data_designer/logging.py Adds module-level _logging_configured flag, sets it at the end of configure_logging(), and exposes is_logging_configured() predicate — straightforward and correct.
packages/data-designer/src/data_designer/interface/data_designer.py Guards the default configure_logging() call in _initialize_interface_runtime() behind is_logging_configured() — clean one-line change with no unintended side effects.
packages/data-designer-config/tests/test_logging.py Adds a focused test for the new _logging_configured flag using monkeypatch for proper isolation.
packages/data-designer/tests/interface/test_data_designer.py Adds regression test for preconfigured-logging preservation and fixes the existing test_initialize_interface_runtime_runs_once by resetting _logging_configured — necessary to keep the mock assertion reliable.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant logging as data_designer.logging
    participant iface as _initialize_interface_runtime

    User->>logging: configure_logging(LoggingConfig.debug())
    logging->>logging: set handlers, levels
    logging->>logging: "_logging_configured = True"

    User->>iface: DataDesigner(...)
    iface->>iface: _interface_runtime_initialized? → False
    iface->>logging: is_logging_configured()
    logging-->>iface: True
    iface->>iface: skip configure_logging() ✓
    iface->>iface: resolve_seed_default_model_settings()
    iface->>iface: "_interface_runtime_initialized = True"
    iface-->>User: DataDesigner ready (DEBUG logging preserved)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant logging as data_designer.logging
    participant iface as _initialize_interface_runtime

    User->>logging: configure_logging(LoggingConfig.debug())
    logging->>logging: set handlers, levels
    logging->>logging: "_logging_configured = True"

    User->>iface: DataDesigner(...)
    iface->>iface: _interface_runtime_initialized? → False
    iface->>logging: is_logging_configured()
    logging-->>iface: True
    iface->>iface: skip configure_logging() ✓
    iface->>iface: resolve_seed_default_model_settings()
    iface->>iface: "_interface_runtime_initialized = True"
    iface-->>User: DataDesigner ready (DEBUG logging preserved)
Loading

Reviews (1): Last reviewed commit: "fix: preserve preconfigured logging" | Re-trigger Greptile

@schultzjack

Copy link
Copy Markdown
Author

recheck

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.

configure_logging() called before DataDesigner() is silently overwritten

1 participant