Repository navigation
[OPIK-8379] [SDK] chore: enable ruff PLE1205/PLE1206 to catch logging format-arg mismatches - #8323
Merged
Conversation
Enable logging-too-many-args and logging-too-few-args so a logging call whose positional arguments do not match its format-string placeholders fails pre-commit and the Code Quality check. The defect these catch has shipped twice. A message with no %s that is passed a positional argument raises inside Handler.emit, so handleError() prints a "--- Logging error ---" block to stderr and the record, including its exc_info traceback, is never emitted. Nothing propagates to the caller, so the only diagnostic for a mid-stream failure is lost. It was swept once across twelve call sites, then reintroduced by an integration that landed after that sweep, because nothing but code review was enforcing it. The rules are configured in .ruff.toml rather than pyproject.toml: a .ruff.toml is present for this package and takes precedence, so ruff silently ignores a [tool.ruff] section in pyproject.toml. The SDK is already clean under both rules, so this is configuration only with no code changes required. Implements OPIK-8379: enable ruff PLE1205/PLE1206 to catch logging format-argument mismatches in the Python SDK Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
⏱️ pre-commit per-hook timing
⏭️ 42 skipped (no matching files changed)
|
JetoPistola
marked this pull request as ready for review
September 15, 2026 06:23
Collaborator
|
No test needed here. No product surface in this PR — only docs, CI, tests, lockfiles or other non-product files. Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review. Re-checked after a push on 15 Sep 06:37 UTC. |
JetoPistola
marked this pull request as draft
September 15, 2026 06:27
JetoPistola
marked this pull request as ready for review
September 15, 2026 06:34
petrotiurin
approved these changes
Sep 15, 2026
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.
Details
Enables ruff's
PLE1205(logging-too-many-args) andPLE1206(logging-too-few-args) for the Python SDK, so a logging call whose positional arguments don't match its format-string placeholders fails pre-commit and the 🐙 Code Quality check.This defect has shipped twice. When a message with no
%sis passed a positional argument,loggingdefersmsg % argsto the handler, so the interpolation raises insideHandler.emit,handleError()prints a--- Logging error ---block to stderr, and the record — including itsexc_infotraceback — is never emitted. Nothing propagates to the caller, so the only diagnostic for a mid-stream failure is silently lost.It was swept once across twelve call sites in #7850, then reintroduced in two new call sites by the Cerebras integration (#7342), which landed after that sweep and was fixed in #8238. Nothing but human code review was enforcing the invariant, so each recurrence cost a bug report, a fix PR, and a review cycle.
Configured in
.ruff.toml, notpyproject.toml. Worth calling out, because the obvious approach silently does nothing: a.ruff.tomlis present for this package and takes precedence, so a[tool.ruff]section added topyproject.tomlis ignored — the hook still passes and the rules never run.Deliberately scoped to
PLE1205/PLE1206. TheG/ flake8-logging-format ruleset does not catch this defect (verified against the exact Cerebras code) and reports ~200 pre-existing violations (111G004, 86G201, 2G002, 1G202), so it would be a separate cleanup rather than a guard against this bug.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
No new tests. This is lint configuration; there is no new code path to exercise, and the rules themselves are ruff's.
Verified by running the real pre-commit hooks rather than ruff directly:
🐍 python sdk,🤖 optimizer,🛡️ guardrails) over every tracked.pyfile in the repo → Passed. The SDK is already clean under both rules, so this lands with no code changes.PLE1205andPLE1206. This is the part that makes the green run meaningful — a clean pass alone would also be the result of the rules being silently ignored, which is exactly thepyproject.tomltrap described above.mistralandanthropicBeta stream wrappers pass onlyexc_info=Trueas a keyword rather than a positional arg, so they are correct as written → not flagged.logging-too-many-args (PLE1205)/logging-too-few-args (PLE1206).Measured against the two known-bad call sites: 2 violations before #8238, 0 after — matching an independent AST pass over the same tree.
Documentation
N/A — the change is a lint rule with an explanatory comment at the config site.