fix(logging): tag keboola-owned handlers; only remove those in set_default_logger#95
Merged
matyas-jirat-keboola merged 8 commits intomainfrom Apr 9, 2026
Merged
Conversation
TestPyPI publish now only runs on workflow_dispatch (manual). On release it was failing the pipeline unnecessarily. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
keboola-vcr requires Python >=3.10; the old >=3.8 floor caused uv dependency resolution to fail on 3.8/3.9 CI matrix legs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Minimum supported version is now 3.10 (keboola-vcr constraint). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fault_logger set_default_logger and set_gelf_logger iterated root.handlers while calling removeHandler in the same loop body, causing mutation-during-iteration skips (alternating handlers were missed). More importantly, all handlers were removed indiscriminately, dropping any test-infrastructure handlers (e.g. LogCaptureHandler from keboola.vcr) that were installed before the component initialised. Fix: mark handlers created by keboola.component with _keboola_owned = True and only remove those. External handlers installed by test frameworks survive set_default_logger calls. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The GELF handler added in set_gelf_logger was not tagged, so the cleanup loop (which only removes _keboola_owned handlers) left it on the root logger when set_default_logger or set_gelf_logger was called again, causing duplicate log delivery. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
soustruh
approved these changes
Apr 9, 2026
Contributor
soustruh
left a comment
soustruh
left a comment
There was a problem hiding this comment.
OK, thanks for your sideways explanation, LGTM ദ്ദി(•ᴗ•)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
set_default_loggerandset_gelf_loggeriteratedroot.handlerswhile callingremoveHandlerin the same loop body — a classic mutation-during-iteration bug that caused alternating handlers to be skipped.LogCaptureHandlerfromkeboola.vcrtest infrastructure).Fix: handlers created by keboola.component are tagged with
_keboola_owned = True. Both methods now iteratelist(root.handlers)(a copy) and only remove tagged handlers, leaving anything else untouched.Test plan
keboola.ex-oracle-hyperionfunctional test suite (20/20 tests pass) withkeboola.vcrlog-capture feature that relies onLogCaptureHandlersurvivingset_default_loggercalls🤖 Generated with Claude Code