fix(logging): scope OPENROUTER_DEBUG to the openrouter logger instead of root - #594
Open
rajarshidattapy wants to merge 1 commit into
Open
fix(logging): scope OPENROUTER_DEBUG to the openrouter logger instead of root#594rajarshidattapy wants to merge 1 commit into
rajarshidattapy wants to merge 1 commit into
Conversation
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
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.
Fixes #590.
Problem
get_default_logger()calledlogging.basicConfig(level=logging.DEBUG), and it runs at clientconstruction — so instantiating
OpenRouter()withOPENROUTER_DEBUGset mutated the hostapplication's root logger, turning on DEBUG output for every library in the process.
Change
The issue's repro, on this branch:
One deviation from the patch suggested in #590:
hasHandlers()rather thanif not logger.handlers.hasHandlers()walks the ancestor chain, so when the host app hasalready configured logging the SDK attaches nothing and its records flow through the app's
existing handler —
if not logger.handlerswould add a competing one and print every recordtwice. It also makes repeated client construction idempotent for free.
On landing this in the generator
#590 notes the file carries a
DO NOT EDITheader and that the fix belongs in the generator.It does eventually, but that isn't something this repo can do, and without
.genignoretheedit would be reverted by the next regeneration regardless.
Freezing a generated file is a real risk — it's what deleted the OAuth PKCE helpers in #583 —
so it's worth stating why it's acceptable here:
utils/logger.pyhas changed exactly once inthe repo's entire history, in the initial commit. And if a future regen does add a symbol that
utils/__init__.pyimports from it, the failure is a loudImportErrorat import time ratherthan silent drift.
Worth noting
gen.yamlhasenableCustomCodeRegions: false. Turning that on would be thesanctioned way to keep a hand-written block inside a generated file, but I could not verify the
Python region syntax without the Speakeasy CLI, so I did not reach for it. Happy to switch if
you'd prefer that route.
Verification
models.count()callINFO:httpxno longer leaks to stderrbasicConfigline moves root from30 / 0handlers to10 / 1,which is exactly what
test_debug_mode_leaves_the_root_logger_aloneasserts againstmypy769 files clean,pyright0 errors,pylint10.00/10Full suite is 6 passed, 1 failed — the failure is
test_responses_namespace.py, which is#585 and already red on
mainbefore this branch.