-
Notifications
You must be signed in to change notification settings - Fork 56
chore(backend): enforce ruff TRY400 so error logs carry tracebacks #10220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
saltas888
wants to merge
9
commits into
develop
Choose a base branch
from
pha/INBOX-29
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
184057a
[Spec Kit] Specify TRY400 re-enable, TRY004 scoped out (INBOX-29)
saltas888 c7b6f0e
[Spec Kit] Plan + Phase 0 research: per-site TRY400 decisions (INBOX-29)
saltas888 e7e684d
[Spec Kit] Critique + tasks: 15 tasks, TRY004 handoff obligations (IN…
saltas888 2d339e1
[Spec Kit] Alignment check: ALIGNED, 0 remediation passes (INBOX-29)
saltas888 b3165cb
chore(backend): enforce ruff TRY400 so error logs carry tracebacks
saltas888 7a860c5
[Spec Kit] Implementation report — all gates green (INBOX-29)
saltas888 d24f9b0
docs(changelog): reword TRY400 fragment to satisfy Vale spelling
saltas888 d9893d2
fix(backend): keep log.error for routine ClientDisconnect, anchor spe…
saltas888 2660959
docs(spec): reconcile TRY400 site counts to 27 convert / 7 noqa
saltas888 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
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
1 change: 1 addition & 0 deletions
1
changelog/+ruff-try400-error-instead-of-exception.housekeeping.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| The TRY400 (error-instead-of-exception) ruff rule is now enforced — a `log.error` reporting a caught exception is either `log.exception`, so the traceback reaches the logs, or carries an explicit justified `# noqa: TRY400`. Nine now-redundant `# noqa: BLE001` comments were removed as a result. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Spec/Ask Alignment Check | ||
|
|
||
| **Date**: 2026-08-11 | **Feature**: `dev/specs/005-ruff-try400-tracebacks/` | ||
|
|
||
| ## Source | ||
|
|
||
| The source-of-truth ask is the **inline PRD** passed to this run, composed from Jira card | ||
| **INBOX-29** (Engineering Inbox, Tech Debt) by the platform-health drain pipeline. It carries the | ||
| card's Overview, Suggested solution, the measured ground truth, the TRY400-only scope decision, a | ||
| 4-item "WHAT TO BUILD", an acceptance list, and a hard-constraints block. | ||
|
|
||
| Two referenced URLs were **not** fetched: | ||
|
|
||
| - the card's provenance link (`opsmillworkspace.slack.com/...`) — an authenticated Slack | ||
| permalink, not reachable; its substance is already quoted in the card and carried into the ask. | ||
| - the card itself was read directly via the Jira API before this run, not re-fetched here. | ||
|
|
||
| Neither is requirement-bearing beyond what the inline ask already states, so the check runs | ||
| against the inline ask. | ||
|
|
||
| ## Verdict | ||
|
|
||
| **✅ ALIGNED** — 0 remediation passes used. | ||
|
|
||
| ## Findings | ||
|
|
||
| | Severity | Category | Ask reference | Spec reference | Description | | ||
| |----------|----------|---------------|----------------|-------------| | ||
| | info | expansion | WHAT TO BUILD #2 ("if `exception` would be wrong there, use a targeted `# noqa: TRY400`") | research.md §R4, FR-003 | The ask authorised per-site noqa in the abstract; the spec/research resolve it concretely into 27 conversions + 7 justified suppressions. Elaboration of an explicit instruction, not drift. | | ||
| | info | added | — | SC-007 | "Every remaining `# noqa: TRY400` carries a one-line justification" is a criterion the ask implied ("with a one-line reason") but did not list under ACCEPTANCE. Added as a verifiable gate. | | ||
| | info | added | — | research.md §R3 | The `TracebackSuppressionFilter` interaction was discovered during Phase 0, not present in the ask. It *narrows* scope at one site for a correctness reason and is documented. | | ||
| | info | changed | ask: "the 34 TRY400 violations ... EXCEPT auth/auth.py" | spec.md Context | The ask's own arithmetic (36 total, 2 in auth ⇒ 34 in scope) is preserved exactly; the spec additionally publishes the full 36-site distribution table. Presentation only. | | ||
|
|
||
| **No** missing requirements, **no** off-scope additions, **no** softened or dropped acceptance | ||
| criteria, **no** contradicted constraints. Specifically confirmed present in the spec: | ||
|
|
||
| - TRY004 out of scope, with the reason (spec "Out of Scope — TRY004", SC-003) | ||
| - `extend-select` mechanism and the TRY200-removed-rule warning (Assumptions, research.md §R1) | ||
| - no dependency-list edits (FR-002) | ||
| - `auth/auth.py` untouched, suppressed by file with a commented reason (FR-006) | ||
| - changelog fragment conditional on repo convention (FR-008) | ||
| - all four hard-constraint categories (FR-007, SC-005) | ||
| - structlog keyword-argument preservation (FR-004) | ||
| - test-assertion exposure for log records (research.md §R7) | ||
|
|
||
| ## Action | ||
|
|
||
| Proceed to implementation. No phases re-run. |
46 changes: 46 additions & 0 deletions
46
dev/specs/005-ruff-try400-tracebacks/checklists/requirements.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Specification Quality Checklist: Re-enable ruff TRY400 so error logs carry tracebacks | ||
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
| **Created**: 2026-08-11 | ||
| **Feature**: [spec.md](../spec.md) | ||
|
|
||
| ## Content Quality | ||
|
|
||
| - [x] No implementation details (languages, frameworks, APIs) | ||
| - [x] Focused on user value and business needs | ||
| - [x] Written for non-technical stakeholders | ||
| - [x] All mandatory sections completed | ||
|
|
||
| **Note on the first two items**: this is a lint/tooling feature, so its "user" is an Infrahub | ||
| developer and its subject matter is inherently the lint configuration. Naming ruff, TRY400 and | ||
| `pyproject.toml` is describing *what* the change is, not leaking a chosen implementation. The | ||
| spec still avoids prescribing per-site edits — those belong to plan/tasks. | ||
|
|
||
| ## Requirement Completeness | ||
|
|
||
| - [x] No [NEEDS CLARIFICATION] markers remain | ||
| - [x] Requirements are testable and unambiguous | ||
| - [x] Success criteria are measurable | ||
| - [x] Success criteria are technology-agnostic (no implementation details) | ||
| - [x] All acceptance scenarios are defined | ||
| - [x] Edge cases are identified | ||
| - [x] Scope is clearly bounded | ||
| - [x] Dependencies and assumptions identified | ||
|
|
||
| ## Feature Readiness | ||
|
|
||
| - [x] All functional requirements have clear acceptance criteria | ||
| - [x] User scenarios cover primary flows | ||
| - [x] Feature meets measurable outcomes defined in Success Criteria | ||
| - [x] No implementation details leak into specification | ||
|
|
||
| ## Notes | ||
|
|
||
| - Scope was narrowed from the source card (INBOX-29 named TRY004 **and** TRY400) to TRY400 only. | ||
| The spec documents this in "Out of Scope — TRY004" with the reason: TRY004's fix changes | ||
| caller-visible exception types on schema and GraphQL surfaces, which needs human design review. | ||
| This is an intentional, recorded scope reduction, not drift. | ||
| - Violation counts in the spec were measured on this branch rather than taken from the card | ||
| (card said ~56; actual is 76 = 36 TRY400 + 40 TRY004). | ||
| - FR-006's auth.py carve-out is a pipeline-permission boundary. The spec's Assumptions section | ||
| records that the merged BLE precedent edited the same file, so a reviewer can overrule it. | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.