Skip to content

fix: preserve consumer logging ownership and routing - #415

Merged
codeforester merged 17 commits into
mainfrom
bug/387-20261003-bug-configure-logger-closes-consumer-owned-handlers-and-forc
Oct 5, 2026
Merged

codeforester merged 17 commits into
mainfrom
bug/387-20261003-bug-configure-logger-closes-consumer-owned-handlers-and-forc

Conversation

@codeforester

@codeforester codeforester commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

CLI setup and teardown now close only lifecycle-owned handlers. Consumer handlers, explicit levels, and parent logging configuration survive an invocation; unconfigured loggers keep duplicate-free terminal output. The public logger helper accepts an explicit propagation policy.

Fixes #387.

Branch maintenance

Refs #426. Targets the branch for #414. Retarget and refresh after that parent is squash-merged; preserve the ordered stack.

The branch was refreshed without rewriting history to include main at a576cc279739eae5e4cfc33ffab2a7fb56de24de. The documentation conflict preserves both logger-ownership and timestamp configuration guidance.

Current-head validation

At b48e4443a1e7bde586679e975f28ad9713a916b7: uv lock freshness and baseline, runtime, strict typing, style, and contracts passed locally with all declared extras. Runtime result: 611 passed, 1 warning, 262 subtests passed in 8.75s.

Hosted checks: 7/7 required checks passed; 0 checks pending; 0 unsuccessful checks at 2026-10-04T14:19:40.760978+00:00. See the PR Checks tab and #426 for subsequent results.

Comment thread lib/python/base_cli/logging.py Outdated
Comment thread lib/python/base_cli/logging.py

@codeforester codeforester left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed against #387's acceptance criteria. Handler ownership via _base_cli_owned is the right shape, and consumer handlers are no longer closed or detached. CI is green.

I found one verified behavioral regression, inline. There are also two acceptance criteria without a test:

  • dictConfig() routing: the criterion says logging.config.dictConfig()-configured base_cli handlers continue to receive records. test_parent_consumer_configuration_is_preserved uses addHandler on the parent, not dictConfig. I checked dictConfig by hand and it does work, partly because dictConfig resets child propagate itself. A test would pin that behavior.
  • #341 nested run_app: the criterion says the nested-run_app guarantee still holds. No new test asserts it. Please confirm the existing #341 tests cover the new ownership-filtered cleanup path in context.py.

Comment thread lib/python/base_cli/logging.py Outdated
@codeforester

Copy link
Copy Markdown
Contributor Author

Re-verified at 3f58457: all review feedback addressed ✅. I re-ran the original repro against the stack tip: with a consumer handler on base_cli.demo and no level set, configure_logger(..., debug=True) now leaves the effective level at DEBUG, and DEBUG reaches both the Base stream and the persistent log. The consumer handler is preserved, and an explicit consumer WARNING level is kept. Minor and non-blocking: _CONFIGURE_LOGGER_LOCK covers only the handler remove/add, while the foreign_handlers/level/propagate inspection still runs before the lock. Moving the with up a few lines would make the reply's 'inspect/remove/add serialized' claim literally true. Full suite passes at the tip. The threads can be resolved.

Base automatically changed from ci/393-20261003-ci-ruff-check-and-mypy-do-not-cover-the-compatibility-consum to main October 5, 2026 13:33
@codeforester
codeforester merged commit f701958 into main Oct 5, 2026
138 of 140 checks passed
@codeforester
codeforester deleted the bug/387-20261003-bug-configure-logger-closes-consumer-owned-handlers-and-forc branch October 5, 2026 13:36
codeforester added a commit that referenced this pull request Oct 5, 2026
Persistent logging now reuses its private sidecar descriptor and caches
human-log source paths per invocation. Repeated source paths avoid
filesystem resolution, forked children reopen their lock descriptor, and
sidecar failures use logging's error handler so commands can complete.

Fixes #381.

## Branch maintenance

Refs #426. Targets the branch for #415. Retarget and refresh after that
parent is squash-merged; preserve the ordered stack.

The branch was refreshed without rewriting history to include `main` at
`a576cc279739eae5e4cfc33ffab2a7fb56de24de`.

## Current-head validation

At `66ce2689a0b5859ad25927aaa9274c7a4ff5ab19`: uv lock freshness and
baseline, runtime, strict typing, style, and contracts passed locally
with all declared extras. Runtime result: 616 passed, 1 warning, 262
subtests passed in 8.57s.

Hosted checks: 7/7 required checks passed; 0 checks pending; 0
unsuccessful checks at 2026-10-04T14:19:40.760978+00:00. See the PR
Checks tab and #426 for subsequent results.
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.

bug: configure_logger closes consumer-owned handlers and forces propagate=False

1 participant