prep release 1.23.0 - #1259
Merged
Merged
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
eabb609
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ac97603.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://prep-release-1-23-0.infrahub-sdk-python.pages.dev |
infrahub-sync now ships update-infrahub-sdk.yml listening for the trigger-infrahub-sdk-python-update repository_dispatch event, so add it to the release fan-out matrix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors the server-side status so SDK clients can read a branch left in MERGE_FAILED by failed-merge detection instead of crashing on validation. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pyarrow was imported at module top-level in the line-delimited JSON importer, which is reached from ctl.cli_commands at CLI startup. That forced every infrahubctl command to require pyarrow (the 'ctl' extra), so a slim install without it — e.g. the Infrahub server image — could not run even `infrahubctl schema load`. Import pyarrow lazily inside LineDelimitedJSONImporter.import_data, the only code path that uses it, and raise a clear install hint if it is missing. Now only `infrahubctl object load` needs the 'ctl' extra. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Specify phase for IHS-249. Adds spec.md (user journeys P1-P3 + tune, FR-001..009, success criteria, edge cases, out-of-scope) and the requirements quality checklist. Resolves the PRD open question by chaining the underlying transport error as the RateLimitError cause. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plan phase for IHS-249. Adds plan.md, research.md, data-model.md, contracts/ (Config fields, RateLimitError, RateLimitRetryHandler), and quickstart.md. Points the agent-context plan reference at the new plan. Key design finding: the retry chokepoint is not singular — login routes through _request, but _request_multipart and _get_streaming bypass it, so the retry driver is applied at all three send sites per client to satisfy FR-006. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Verdict: PROCEED WITH UPDATES. Records the dual-lens critique and applies its findings: - E2/X1 (Must-Address): multipart retry could re-send a consumed file body; plan + data-model now require rewinding/re-materializing the payload per attempt, plus a regression test. - P3: build-vs-buy rationale (custom vs tenacity/httpx) in research. - P4: worst-case cumulative wait documented in plan. - E4: retry logs constrained to URL/attempt/delay (no secrets). - E6: multipart full-body-on-retry validation added to quickstart. - E3: mutation-retry assumption accepted (PRD pre-processing 429). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tasks phase for IHS-249: 21 tasks across setup, foundational retry machinery (handler, RateLimitError, Config fields, drivers on all three send sites of both clients incl. the E2/X1 multipart re-read fix), four user-story validation phases, and polish (FR-006 coverage, E2 regression test, towncrier fragments, docs/lint gates). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Compares spec.md against the Jira IHS-249 PRD. Verdict: ALIGNED. All FR-001..009, journeys, acceptance criteria, success criteria, and out-of-scope boundaries carried over faithfully. Only additions are the authorized open-question resolution (RateLimitError __cause__) and SC-006 (derived from FR-009). No remediation needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Verified against client.py that InfrahubClient and InfrahubClientSync are symmetric: each has three direct send sites (_request, _request_multipart, _get_streaming) and multipart/streaming bypass _request on both. Send-site audit confirms exactly six sites total, no fourth path. Tightened plan source-code section, research R1, and tasks T009/T010 with concrete per-client method line refs so the retry driver is wired on both clients (FR-008). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add transparent retry-with-backoff on HTTP 429 across both clients (T001-T010): - New pure RateLimitRetryHandler (Retry-After parsing, jittered/clamped exponential backoff, retry-budget decision) in infrahub_sdk/rate_limit.py - Four rate_limit_* fields on ConfigBase; new RateLimitError(Error) - Async/sync _send_with_rate_limit_retry drivers wired into _request, _request_multipart (with per-attempt file rewind, critique E2/X1), and _get_streaming (retry on stream initiation) - Handler unit tests; client-level test skeleton Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add async/sync parametrized tests exercising the real _request -> _send_with_rate_limit_retry path: a scripted [429, 200] sequence retries transparently and returns the 200 after exactly two sends, and non-429 responses pass through untouched with a single send. Driver sleep is patched via monkeypatch to avoid real waits. Covers T011 and T012 (SC-001). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover honouring Retry-After on 429 across async and sync clients: delta-seconds, HTTP-date, zero/past-date (~0s), clamp above rate_limit_backoff_max, and malformed header falling back to computed jittered backoff while still retrying. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add parametrized async+sync tests asserting persistent 429 exhausts the retry budget: exactly max_retries + 1 sends, one RateLimitError with url/attempts/retry_after and a chained httpx.HTTPStatusError cause, and one WARNING log per retry carrying url, attempt number, and delay. Covers T014; T015 verified — driver synthesizes and chains the terminal error and tracks last_retry_after with no source change needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add client-level tests for the rate-limit retry disabled path, the rate_limit_max_retries budget, and explicit async/sync parity on an identical 429 sequence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add FR-006 all-paths coverage (regular request, multipart upload, streaming initiation on both async and sync clients) and the E2/X1 regression test that asserts a retried multipart upload re-sends the full body byte-for-byte (modulo the random boundary), driving 429->200 at the httpx transport layer via pytest-httpx. Add towncrier fragments for the transparent 429 retry feature and regenerate the Config reference for the four new rate_limit_* fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
parse_retry_after now floors negative delta-seconds at 0.0 (preventing a negative wait that would crash the sync driver's time.sleep while asyncio tolerated it) and returns None on OverflowError from pathological digit strings so the caller falls back to computed backoff. Adds handler unit tests for both cases, driver-level tests proving exponential backoff growth/clamping and per-instance jitter divergence, and a direct unit test of _rewind_multipart_files across its files shapes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Records the implement + review tail: 6 impl chunks (T001-T021, all ticked), 3-agent review of a55cbaa..HEAD, and inline fixes for the high-severity findings (negative/overflow Retry-After crash; unguarded SC-003 growth + jitter tests; direct multipart-rewind guard test). 62 tests passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
markdown-lint: pad table delimiter rows (MD060 compact style), add blank lines around headings/tables/lists (MD022/MD032/MD058), and replace emphasis-as-heading with plain text (MD036) in the generated spec artifacts under dev/specs/ihs-249-sdk-429-retry/. vale: add "backoff" to the spelling exception vocabulary so the generated config.mdx retry docs pass Infrahub.spelling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses code-review findings on the 429 retry driver: - Exhaustion no longer leaks RuntimeError when the final 429 response has no attached request (e.g. a custom requester): the driver now captures the cause (HTTPStatusError or none) and always raises RateLimitError, removing the previously-unreachable trailing raise. - Streaming init: the ExitStack/AsyncExitStack is now closed via try/finally so a raise during the failed-429 read cannot leak the stream. - compute_backoff caps the exponent (2 ** min(attempt, 63)) so a very large rate_limit_max_retries can no longer overflow float before the clamp. - next_delay drops a redundant no-op clamp on the jittered branch. - Adds a regression test: request-less 429 exhaustion raises RateLimitError (cause None), async and sync. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This is speckit working-state (a local pointer to the active feature directory), not a project artifact; untrack it while keeping the local copy so speckit tooling still resolves the feature dir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…m comments Addresses PR review feedback: - Move the retry driver loop out of both clients into RateLimitRetryHandler.send / .asend, and construct the handler once per client (self._rate_limit_handler) instead of building it per request on each of the two code paths. - Trim verbose/low-value comments and docstrings on the retry code: drop the over-explained Retry-After parse comments, docstrings that restated the code (jittered_delay), and the redundant no-op-clamp note; tighten the class/driver docstrings. - Tests: rewrite the stale module docstring, drop the unused __all__ re-export block, strip internal spec identifiers (SC-/FR-/E2/X1) from test docstrings per dev/rules/python-testing.md, and trim an over-long test docstring. - Revert the stray CLAUDE.md SPECKIT plan-pointer edit (not part of this PR). Behaviour unchanged; 64 unit tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidate to one towncrier fragment (1124.added.md) per review feedback; it already notes RateLimitError is raised when retries are exhausted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a get_deployment_id() method to the async and sync clients and surface the value in `infrahubctl info` (both the simple and --detail views), making the Infrahub deployment identifier easy to retrieve. Closes #1017 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d [INFP-234] The upstream IPAddress attribute-kind tests build a schema with kind="IPAddress", but this branch generates AttributeKind from the backend, which does not yet define an IPAddress attribute type. Skip until the backend adds it and the generated enum includes it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The write root declared `version: str | None = None`, so `validate_schema()` reported a payload without `version` as valid while `POST /api/schema/load` rejected it. Making the field required restores the offline/server parity the published write contract promises. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The field-override comment on NodeSchema packed three ideas into two lines and ended on an unrelated remark about server-side validation; it now states only why the override exists. The _SchemaNodeBase docstring no longer argues against a mixin, since the mixin it replaced is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add an opinionated, offline formatter for Infrahub schema YAML files whose job is to normalise the ordering of keys within each node, generic, attribute, relationship and dropdown choice, so hand-authored schemas read consistently and produce small diffs. - New infrahub_sdk/ctl/schema_format.py with the pure formatting logic: canonical key orders, restricted-namespace filtering (core nodes only), list-item order preserved, a PyYAML dumper matching the schema-library layout, literal-block multiline handling, and a semantic-equality guard that aborts rather than risk changing a file's meaning. - New `format` subcommand in schema.py: in-place by default, plus --check (CI gate) and --diff, with warnings for comments that PyYAML cannot preserve. - Unit + CLI tests and the regenerated infrahubctl CLI reference.
Switch the schema formatter from PyYAML to ruamel.yaml round-trip mode so that reordering keys no longer discards comments. This also preserves quoting and inline (flow) sequences (e.g. `[manufacturer, name__value]`) for free, so the diff a format run produces is now purely key-ordering. - schema_format.py: reorder keys in place with move_to_end so the comments ruamel attaches to each key travel with it; keep the semantic-equality guard, restricted-namespace filtering, and canonical key orders. The header is preserved (or added when missing). - Drop the comment-drop warning and count_droppable_comments, which existed only because PyYAML lost comments. - schema.py: format from the raw file text; update the command help. - Add ruamel.yaml to the `ctl` / `all` dependency sets.
The formatter's canonical key ordering is written against a known set of schema properties. When Infrahub adds or removes a property in the published JSON schema, that ordering may need updating (an unrecognised key is preserved, but not ideally placed). Track this without gating releases: - infrahub_sdk/ctl/schema_drift.py compares the live schema's property sets to a committed baseline (schema_properties.json) and reports added/removed properties. It never raises on drift. - `invoke schema-drift-check` emits GitHub ::warning:: annotations for any drift and always exits 0; `invoke schema-drift-update` refreshes the baseline. - .github/workflows/schema-drift.yml runs the check on release publish and manual dispatch, warn-only. - Baseline snapshot + offline unit tests for the drift logic.
…er detection Address code-review findings: - _format_entity: only iterate `attributes`/`relationships` when they are lists, so a parseable-but-malformed schema (e.g. `attributes: 5`) is left untouched instead of crashing. - _ensure_schema_header: detect a real `# yaml-language-server:` directive line via regex rather than an arbitrary substring, so the header is still added when the string only appears in a scalar or unrelated comment. - test_format_preserves_comments: assert exit_code == 0 so the test can no longer pass silently if the format command fails. Add regression tests for the malformed-section and substring-in-scalar cases.
CI lints with ruff 0.15.12 (develop's pinned version), which enforces pydocstyle D413; the branch's local ruff 0.15.0 did not, so this passed locally but failed in CI. Add the required blank line after the final docstring section in the schema formatter/drift modules and the schema format command.
_print_schema_diff used markup=False (needed so bracketed diff content like [manufacturer, name] stays literal) together with inline [green]/[red] tags, which then printed verbatim instead of colouring the line. Apply the colour with the style= argument instead.
…a format Three off-by-default transforms for `infrahubctl schema format`, keeping the base command purely key-ordering: - --strip-defaults: remove node/attribute/relationship keys whose value equals the schema default (context-aware; grounded in the published JSON-schema defaults). Consequential/internal fields (branch, state, inherited, display) are intentionally not stripped. - --sort-by-order-weight: sort attributes and relationships ascending by order_weight; items without one keep their authored order and go last. - --backfill-order-weight: give attributes/relationships lacking an order_weight a single constant value (1000). The semantic guard now neutralises exactly the requested transforms on both sides of the comparison, so an intended change is allowed while any unintended corruption still aborts. Verified guard-safe and idempotent across all schema-library files for every flag combination.
…guard Address PR review comments: - _format_one_schema_file now returns a FormatOutcome enum instead of loose string literals, tightening the contract with the format command loop (per review feedback). - The semantic guard neutralised list reordering by sorting on `name`, which spuriously aborted when two items shared a name but differed in weight. Sort by full item content instead — a total, content-based order permits any intended reorder while still catching a dropped or corrupted item.
Raise patch coverage on the new modules: - schema_drift: test fetch_live_properties (mocked HTTP) and the write/load baseline round-trip — module now fully covered. - schema format CLI: cover the multi-document, invalid-file, and FormatError branches, plus non-dict list items / extension entries. - Drop a now-dead isinstance guard in _strip_default_keys (both callers already pass a mapping). Type-only test imports moved under TYPE_CHECKING to match the repo convention; no private helpers are imported from tests.
Round-trip ruamel YAML is stricter than the PyYAML safe_load used to discover schema files — notably it rejects duplicate keys, which `schema load` tolerates (last wins). That raised ruamel's YAMLError, which escaped the per-file `except FormatError`, hit @catch_exception, printed a traceback and exited 1 — aborting a whole folder run midway. Catch YAMLError on load and re-raise as FormatError so it is reported per file and the remaining files still format.
…1222) The IPAddress attribute kind now exists in the backend, so the generated schema models gain the enum member and the two attribute-kind unions accept it. That unblocks the node tests covering a bare address, which were skipped because the schema fixture could not be built without the enum member. Also re-export IP_ADDRESS_TYPES alongside IP_TYPES, and mention IPAddress in the attribute docstring listing the IP-typed kinds. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The write models set extra="ignore", so a field the user may not set never reaches the server. That decided the field has no effect but left the author with no feedback, so a misspelled key produced a schema quietly different from the one they wrote. Classify every extra key instead. A name the contract knows at that location but the user may not set is reported as a warning and still dropped, so a schema read back from Infrahub keeps loading; any other name is an error. The split is driven by a new generated artifact, schema/generated/contract.py, holding the non-settable field names of each write class. Applying it needs to know which model governs each place in the payload, so _collect_extra_fields walks the raw payload alongside the validated write document: the document resolves the model at every location, including which member of a discriminated union an attribute matched. One consequence is that extra fields surface only once the payload is otherwise valid. validate_schema() now returns warnings alongside errors, and client.schema.validate() reaches the same verdict -- raising ValueError rather than a pydantic ValidationError, and returning the verdict when the payload is accepted. infrahubctl validate schema reports both offline; schema load/check report errors locally and leave the warnings to the server response, which already carries them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A finding carried the bare key, so `parameters.id` was reported as `id` against the owning attribute -- claiming a field is read-only that is in fact settable there -- and collided with an `id` reported from another block when consumers group findings by name. Qualify the name with the fields walked since the last kind or element, which re-anchor the identity a finding is reported against. `inherited` on an attribute is unchanged; `parameters.id` and `extensions.id` now say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The table held each class's own fields and the lookup unioned them across the model's MRO, reaching into the generated hierarchy from the consumer side. Resolve the inheritance in the generator instead, so the emitted table is already complete per class and the lookup is a plain dict access. Fold the paired defensive isinstance checks on the raw payload into one contract guard at the top of the walk, which also covers the context resolution now that it happens there. Every remaining isinstance dispatches on the validated value's shape rather than second-guessing the input. Record on the walk why it pairs the payload with the validated model: neither side alone carries both the dropped keys and the model governing each location. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Remove broken client.branch.diff_data() The method targeted GET /api/diff/data, a REST endpoint that does not exist in Infrahub, so every call returned a 404 (and the URL builder was also missing the ? separator). Instead of adding a server endpoint for it, drop the method and point users at the existing GraphQL-based client.get_diff_tree() / client.get_diff_summary(). Also removes InfraHubBranchManagerBase, whose only content was the diff_data URL builder, and updates the branches guide accordingly. Closes #325 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add include_properties to get_diff_tree for value-level diff details The DiffTree GraphQL query exposes previous/new values per property but the SDK only fetched summary counts, so removing diff_data() would have left no way to retrieve the data-level diff it was meant to provide. With include_properties=True the diff tree now includes value-level details per attribute property and peer id/label per relationship element. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: regenerate SDK reference docs for get_diff_tree signature change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Expose peer_id/peer_label on cardinality-one relationship diff elements The query already fetched them but the parser dropped them for ONE relationships, leaving the IS_RELATED property as the only way to identify the changed peer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Simplify relationship diff parsing and merge changelog fragments Extract the element-to-peer conversion into a helper shared by both cardinality branches, and stop silently dropping trailing elements when a cardinality-one relationship unexpectedly carries several: they now come back as peers, same shape as cardinality-many. The include_properties addition is folded into the removal changelog entry since it exists as the diff_data() replacement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: unit test _diff_element_to_node_diff_peer and reuse it for cardinality-one flattening Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: share peer field extraction between peer diffs and cardinality-one flattening Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * revert: drop the include_properties replacement, keep the plain diff_data removal Nobody uses the value-level diff data, so the broken method is deleted without a replacement API. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ml (#1251) * feat: warn when a Python transform or generator has no watch block The JSON schema generated for .infrahub.yml now marks 'watch' as required on Python transforms and generator definitions, so YAML language servers warn when a definition has no watch block and explain what to list under watch.files. The requirement is advisory only. It is injected through json_schema_extra as an allOf branch, so the models still accept a definition without watch, and an explicit 'files: []' records that nothing extra needs watching. An allOf branch is used rather than a top-level required because json_schema_extra keys replace the ones pydantic generates, which would drop the genuinely required fields. The schema generated from these models now also carries the watch block on generator definitions, which the last published version rejected as an unknown property. Adds jsonschema to the tests dependency group so the tests validate real .infrahub.yml documents against the generated schema. * feat: warn when a watch block does not say what to watch A 'watch' key on its own was enough to silence the missing-watch warning, so a half-written block passed as an answer. The generated JSON schema now requires 'files' inside the block and narrows the field to an object, which also catches the bare 'watch:' and 'watch: null' forms that pydantic accepts through the null half of the generated anyOf. This applies to Jinja2 transforms too. They are still not required to declare watch, but once they do, the block has to be complete. Still advisory only, and 'files: []' remains clean as the way to record that nothing extra needs watching. * feat: stop requiring an explicit files key inside a watch block An empty 'watch: {}' now validates cleanly. 'files' defaults to an empty list, so the block already records the author's "nothing extra needs watching" without the key being spelled out, and demanding it only made .infrahub.yml more verbose for no gain. A bare 'watch:' still warns. That parses to None, which is indistinguishable from never having declared the block, so unlike 'watch: {}' it records nothing. The field-level message now covers every non-mapping value rather than claiming a 'files' key is missing, which was wrong for 'watch: [a, b]' and 'watch: "text"'. * docs: correct the watch test prose left stale by the last change The module comment and the flagged_watch_paths docstring still described a rule that flagged a block for omitting 'files', which the tests directly beneath them now assert is clean. Renamed test_incomplete_watch_stays_valid_at_runtime too, since neither form it covers is incomplete: both are values that record nothing.
wvandeun
force-pushed
the
prep-release-1.23.0
branch
from
August 19, 2026 14:56
f1217e4 to
c7313d5
Compare
github-actions
Bot
added
group/ci
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## stable #1259 +/- ##
==========================================
+ Coverage 82.54% 84.16% +1.61%
==========================================
Files 138 147 +9
Lines 12186 13045 +859
Branches 1833 1930 +97
==========================================
+ Hits 10059 10979 +920
+ Misses 1568 1503 -65
- Partials 559 563 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
23 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/specs/ihs-249-sdk-429-retry/contracts/config.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/contracts/config.md:10">
P2: The contract documents `rate_limit_max_retries` default as 5, but the actual field in infrahub_sdk/config.py:73 defaults to 10, and the spec/plan both say 5. Since this contract is meant to be the authoritative public surface for the 1.23.0 release, consumers will be told the wrong default. Reconcile the two: either change the field default to 5 or update the contract to 10.</violation>
</file>
<file name=".github/workflows/release.yml">
<violation number="1" location=".github/workflows/release.yml:74">
P2: A valid `v0.0.0.devN` or local tag is rejected because `base_version` identifies only `0.0.0`, not whether Hatch used the fallback. Check exact tag reachability before rejecting the fallback version.</violation>
</file>
<file name="tasks.py">
<violation number="1" location="tasks.py:448">
P2: If the committed baseline is missing or malformed, this warn-only task exits nonzero before reporting drift. Wrap baseline loading and drift computation in the warning-and-return path so the task honors its documented always-exit-0 contract.</violation>
</file>
<file name="infrahub_sdk/protocols.py">
<violation number="1" location="infrahub_sdk/protocols.py:154">
P2: When `infrahubctl protocols` generates sync protocols for a node inheriting `CoreIPPool`, `syncify()` leaves `CoreIPPool` unchanged, so the generated class uses the async base and loses the intended sync protocol inheritance. Add `CoreIPPool` to `CORE_BASE_CLASS_TO_SYNCIFY` and cover this inheritance in the generator tests.</violation>
</file>
<file name="infrahub_sdk/schema/main.py">
<violation number="1" location="infrahub_sdk/schema/main.py:48">
P2: Existing callers importing `NodeExtensionSchema` from this module now fail at import time, contradicting the compatibility promise in this comment. Preserve a compatibility shim for the old name or document this breaking API removal explicitly.</violation>
</file>
<file name="infrahub_sdk/node/node.py">
<violation number="1" location="infrahub_sdk/node/node.py:287">
P2: When callers pass `request_context=RequestContext(..., priority=...)` to node CRUD, this line drops the priority before header resolution. Resolve the per-call context priority and pass it through to the mutation or file-upload request.</violation>
</file>
<file name="dev/specs/ihs-259-sdk-x-priority-header/contracts/x-priority-header.md">
<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/contracts/x-priority-header.md:10">
P2: The contract's value set does not match the SDK it documents. Line 10 lists the SDK-emitted values as `high`, `normal`, `low`, but the `Priority` enum in `infrahub_sdk/constants.py:12-21` emits `high`, `medium`, `low`, and the SDK has no `normal` member at all (tests assert `medium`, e.g. tests/unit/sdk/test_priority.py:697). Per this contract's own server semantics, `medium` is an unknown value that the server treats as `normal`, so a caller selecting `Priority.MEDIUM` would be silently no-op'd. Reconcile the implemented enum and this contract: either the contract must document `medium`, or the SDK enum must be `HIGH/NORMAL/LOW` as the spec/plan describe. Choose one source of truth and align both.</violation>
</file>
<file name="infrahub_sdk/task/manager.py">
<violation number="1" location="infrahub_sdk/task/manager.py:76">
P2: When `include_actions` or `include_diagnostics` is enabled, the server rejects the query because the current task schema at opsmill/infrahub/backend/infrahub/graphql/types/task.py:36-49 does not expose these selections. Add the matching server schema before shipping these SDK flags.</violation>
<violation number="2" location="infrahub_sdk/task/manager.py:293">
P2: `retry()` and `cancel()` fail GraphQL validation against the current mutation root at opsmill/infrahub/backend/infrahub/graphql/schema.py:90-95. Register both task mutations server-side before exposing these SDK methods.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/checklists/requirements.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/checklists/requirements.md:39">
P3: The Notes claim that concrete names like `RateLimitError` are deferred to plan.md, but spec.md already uses `RateLimitError` directly (FR-005 at line 94, and lines 46/84) and names the `max_retries` config (lines 50/54). If the checklist's intent is a spec with no implementation/API detail, either reconcile the note or update the checklist items to match the spec's actual content.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/tasks.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/tasks.md:123">
P3: The recommended execution order 'P1 → P2 → P3 → P3' lists P3 twice and never names the fourth story. Since the phases run US1–US4 and both US3 and US4 carry priority P3, spell the order out by story so it unambiguously covers all four: change it to 'US1 → US2 → US3 → US4'.</violation>
</file>
<file name="infrahub_sdk/rate_limit.py">
<violation number="1" location="infrahub_sdk/rate_limit.py:74">
P2: When callers pass a naive `now` to an HTTP-date parse, datetime subtraction raises `TypeError`. Treat naive `now` as UTC before subtracting so this parser remains safe for either datetime form.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/contracts/rate_limit_retry_handler.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/contracts/rate_limit_retry_handler.md:14">
P2: The HTTP-date formula can raise TypeError when implemented literally: `parsedate_to_datetime` returns a timezone-aware datetime while the `now` default (`datetime.now()`) is naive, and subtractng aware from naive raises. State in the contract that `now` must be tz-aware (`datetime.now(timezone.utc)`) and that a naive `parsed` is normalized, so the 'never raises' guarantee (FR-004) holds for the HTTP-date path.</violation>
</file>
<file name="infrahub_sdk/ctl/cli_commands.py">
<violation number="1" location="infrahub_sdk/ctl/cli_commands.py:425">
P2: Switching `info` from `get_version()` to `get_server_information()` makes `infrahubctl info` query `InfrahubInfo { deployment_id }`, a field that only recently appeared on the server (`deployment_id` is new on `InfrahubInfo` in opsmill/infrahub/backend/infrahub/graphql/queries/internal.py:15). GraphQL rejects unknown fields, so when the SDK connects to an older server without `deployment_id`, the whole query fails and the caught exception turns `infrahubctl info` into an error status instead of reporting the version that `get_version()` handled across versions. Consider falling back to `get_version()` when the `InfrahubInfo` query fails, or negotiating the query by server version, to keep `infrahubctl info` working against older servers.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/spec.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/spec.md:90">
P3: FR-001 says the SDK retries "up to a configurable maximum number of attempts," but the rest of the spec (US3, SC-004, Key Entities) defines the configurable value as the maximum number of retries, with total attempts = retries + 1 (default five retries, so six attempts). Read literally as "maximum attempts," FR-001 contradicts the acceptance criteria and could lead an implementer to make the budget off by one. Reword FR-001 to "up to a configured maximum number of retries (total attempts = max_retries + 1)."</violation>
</file>
<file name="infrahub_sdk/client.py">
<violation number="1" location="infrahub_sdk/client.py:221">
P2: When password authentication is enabled, the configured default priority is missing from the login and token-refresh requests because those paths bypass the header merge. Add the resolved `X-Priority` header to both authentication requests so the client-wide priority applies consistently.</violation>
<violation number="2" location="infrahub_sdk/client.py:1492">
P1: When a multipart upload uses a non-seekable stream and receives HTTP 429, the retry sends an already-consumed stream because `_rewind_multipart_files` cannot reset it. Buffer non-seekable streams before retrying, or skip rate-limit retries for uploads that cannot be rewound.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/research.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/research.md:9">
P3: The line-number citations in the R1 send-site audit do not match the code. In infrahub_sdk/client.py the symbols have since been integrated at new positions: `_request` is at L1607 (doc says ~L1486), `_request_multipart` at L1480 (~L1383), `_get_streaming` at L1553 (~L1455), `InfrahubClientSync` at L2179 (L2053), and the sync `_request`/`_request_multipart`/`_get_streaming` at L3776/L2466/L3695 (doc says L3583/L2343/L3545). Each is 100-250 lines off, so a reader following the citations lands in unrelated code. Refresh these to the current locations.</violation>
</file>
<file name="dev/specs/ihs-249-sdk-429-retry/opsmill-implement-report.md">
<violation number="1" location="dev/specs/ihs-249-sdk-429-retry/opsmill-implement-report.md:81">
P3: The report recommends opening the feature PR with base `stable`, but this is a product feature (new retry-with-backoff on HTTP 429, with a caller-visible `429 → RateLimitError` behavior change), not a hotfix or tooling change. Per the release-vehicle rule, product features belong on `develop`; `stable` is for hotfixes and repo-only changes. If this feature must ship in the 1.23.0 minor, that should be decided explicitly rather than defaulting to `stable` in the suggested next steps.</violation>
</file>
<file name="dev/specs/ihs-259-sdk-x-priority-header/plan.md">
<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/plan.md:9">
P3: The documented resolution rule is incomplete. The Summary says the header resolves 'exactly' as `per_request if per_request is not None else client_default`, and that application lives only in `execute_graphql`/`_execute_graphql_with_file`. In the shipped implementation `_request_headers` (infrahub_sdk/client.py:256-261) adds a third source: when the per-request `priority` kwarg is None but `self._request_context.priority` is set, that value overrides the client-wide default. Since this plan is merged post-implementation as part of the release prep, update the resolution rule and the 'single points' claim to mention `request_context` so the doc matches the released behavior.</violation>
</file>
<file name="dev/specs/ihs-259-sdk-x-priority-header/opsmill-implement-report.md">
<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/opsmill-implement-report.md:34">
P3: The claim that `client.create` "issues no HTTP request" is inaccurate: `client.create` calls `await self.schema.get(...)` (infrahub_sdk/client.py:467), a real HTTP request, before returning the unsaved node. The reasoning still holds for the *create* mutation (that fires at `node.save`), but the sentence should read "issues no create request" rather than "issues no HTTP request".</violation>
<violation number="2" location="dev/specs/ihs-259-sdk-x-priority-header/opsmill-implement-report.md:103">
P3: Per release-vehicle guidance, a product feature (the X-Priority request-header API, not a fix) should target develop, not stable. §7 step 1 instructs opening this public-API feature to `stable`; that is the one base-branch case worth flagging. Low-severity note only.</violation>
</file>
<file name="docs/docs/python-sdk/reference/config.mdx">
<violation number="1" location="docs/docs/python-sdk/reference/config.mdx:139">
P3: `priority` is a string enum (high|medium|low, defined in infrahub_sdk/constants.py), but the new docs line labels its **Type**: `object`, which gives readers no hint that the value is a string. This mirrors how mode/transport are also rendered as `object` because the config.mdx generator (docs/docs_generation/helpers.py `build_config_properties`) falls back to `"object"` when the JSON-schema type resolves through a $ref. The mismatch is user-facing; rendering the real `string` type for enum fields would fix all three. If left to the generator, this line is correct as generated, but the generator's fallback is what makes the label wrong.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| raise ServerNotResponsiveError(url=url, timeout=timeout) from exc | ||
|
|
||
| async def send() -> httpx.Response: | ||
| _rewind_multipart_files(files) |
Contributor
There was a problem hiding this comment.
P1: When a multipart upload uses a non-seekable stream and receives HTTP 429, the retry sends an already-consumed stream because _rewind_multipart_files cannot reset it. Buffer non-seekable streams before retrying, or skip rate-limit retries for uploads that cannot be rewound.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/client.py, line 1492:
<comment>When a multipart upload uses a non-seekable stream and receives HTTP 429, the retry sends an already-consumed stream because `_rewind_multipart_files` cannot reset it. Buffer non-seekable streams before retrying, or skip rate-limit retries for uploads that cannot be rewound.</comment>
<file context>
@@ -1390,14 +1487,18 @@ async def _request_multipart(
- raise ServerNotResponsiveError(url=url, timeout=timeout) from exc
+ async def send() -> httpx.Response:
+ _rewind_multipart_files(files)
+ async with httpx.AsyncClient(**self._build_proxy_config(), verify=self.config.tls_context) as client:
+ try:
</file context>
| default=True, | ||
| description="Retry requests that receive HTTP 429 using backoff. Set False to disable.", | ||
| ) | ||
| rate_limit_max_retries: int = Field( |
Contributor
There was a problem hiding this comment.
P2: The contract documents rate_limit_max_retries default as 5, but the actual field in infrahub_sdk/config.py:73 defaults to 10, and the spec/plan both say 5. Since this contract is meant to be the authoritative public surface for the 1.23.0 release, consumers will be told the wrong default. Reconcile the two: either change the field default to 5 or update the contract to 10.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ihs-249-sdk-429-retry/contracts/config.md, line 10:
<comment>The contract documents `rate_limit_max_retries` default as 5, but the actual field in infrahub_sdk/config.py:73 defaults to 10, and the spec/plan both say 5. Since this contract is meant to be the authoritative public surface for the 1.23.0 release, consumers will be told the wrong default. Reconcile the two: either change the field default to 5 or update the contract to 10.</comment>
<file context>
@@ -0,0 +1,37 @@
+ default=True,
+ description="Retry requests that receive HTTP 429 using backoff. Set False to disable.",
+)
+rate_limit_max_retries: int = Field(
+ default=5,
+ ge=0,
</file context>
|
|
||
| - name: "Publish guard: reject unreleased fallback version" | ||
| # fallback_base is read from pyproject.toml at run time; the fallback is a static sentinel (0.0.0.dev0) | ||
| if: steps.release.outputs.base_version == steps.release.outputs.fallback_base && (steps.release.outputs.is_devrelease == 1 || steps.release.outputs.is_local == 1) |
Contributor
There was a problem hiding this comment.
P2: A valid v0.0.0.devN or local tag is rejected because base_version identifies only 0.0.0, not whether Hatch used the fallback. Check exact tag reachability before rejecting the fallback version.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 74:
<comment>A valid `v0.0.0.devN` or local tag is rejected because `base_version` identifies only `0.0.0`, not whether Hatch used the fallback. Check exact tag reachability before rejecting the fallback version.</comment>
<file context>
@@ -53,16 +59,23 @@ jobs:
+ - name: "Publish guard: reject unreleased fallback version"
+ # fallback_base is read from pyproject.toml at run time; the fallback is a static sentinel (0.0.0.dev0)
+ if: steps.release.outputs.base_version == steps.release.outputs.fallback_base && (steps.release.outputs.is_devrelease == 1 || steps.release.outputs.is_local == 1)
+ run: |
+ echo "Resolved version (${{ steps.release.outputs.version }}) is an unreleased fallback (base ${{ steps.release.outputs.fallback_base }}, dev/local build): no v* tag is reachable. Refusing to publish."
</file context>
| print(f"::warning title=Schema drift check::Could not fetch the Infrahub schema: {exc}") | ||
| return | ||
|
|
||
| drift = compute_drift(live=live, baseline=load_baseline()) |
Contributor
There was a problem hiding this comment.
P2: If the committed baseline is missing or malformed, this warn-only task exits nonzero before reporting drift. Wrap baseline loading and drift computation in the warning-and-return path so the task honors its documented always-exit-0 contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tasks.py, line 448:
<comment>If the committed baseline is missing or malformed, this warn-only task exits nonzero before reporting drift. Wrap baseline loading and drift computation in the warning-and-return path so the task honors its documented always-exit-0 contract.</comment>
<file context>
@@ -427,3 +427,41 @@ def generate_repository_jsonschema(context: Context) -> None:
+ print(f"::warning title=Schema drift check::Could not fetch the Infrahub schema: {exc}")
+ return
+
+ drift = compute_drift(live=live, baseline=load_baseline())
+ if not drift:
+ print("Infrahub schema is in sync with the committed baseline; no drift detected.")
</file context>
Suggested change
| drift = compute_drift(live=live, baseline=load_baseline()) | |
| try: | |
| drift = compute_drift(live=live, baseline=load_baseline()) | |
| except Exception as exc: | |
| print(f"::warning title=Schema drift check::Could not load the schema baseline: {exc}") | |
| return |
| children: RelationshipManager | ||
|
|
||
|
|
||
| class CoreIPPool(CoreNode): |
Contributor
There was a problem hiding this comment.
P2: When infrahubctl protocols generates sync protocols for a node inheriting CoreIPPool, syncify() leaves CoreIPPool unchanged, so the generated class uses the async base and loses the intended sync protocol inheritance. Add CoreIPPool to CORE_BASE_CLASS_TO_SYNCIFY and cover this inheritance in the generator tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/protocols.py, line 154:
<comment>When `infrahubctl protocols` generates sync protocols for a node inheriting `CoreIPPool`, `syncify()` leaves `CoreIPPool` unchanged, so the generated class uses the async base and loses the intended sync protocol inheritance. Add `CoreIPPool` to `CORE_BASE_CLASS_TO_SYNCIFY` and cover this inheritance in the generator tests.</comment>
<file context>
@@ -151,6 +151,10 @@ class CoreGroup(CoreNode):
children: RelationshipManager
+class CoreIPPool(CoreNode):
+ pass
+
</file context>
|
|
||
| ## 7. Suggested next steps | ||
|
|
||
| 1. **Open a PR** for branch `dga/feat-409-retry-ivj0i` (base `stable`) — the feature is complete, tested (62 passing), and reviewed. Ensure both towncrier fragments (`changelog/1124.added.md`, `1124.changed.md`) are included; the `429 → RateLimitError` behaviour change is a caller-visible change flagged in the changed fragment. |
Contributor
There was a problem hiding this comment.
P3: The report recommends opening the feature PR with base stable, but this is a product feature (new retry-with-backoff on HTTP 429, with a caller-visible 429 → RateLimitError behavior change), not a hotfix or tooling change. Per the release-vehicle rule, product features belong on develop; stable is for hotfixes and repo-only changes. If this feature must ship in the 1.23.0 minor, that should be decided explicitly rather than defaulting to stable in the suggested next steps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ihs-249-sdk-429-retry/opsmill-implement-report.md, line 81:
<comment>The report recommends opening the feature PR with base `stable`, but this is a product feature (new retry-with-backoff on HTTP 429, with a caller-visible `429 → RateLimitError` behavior change), not a hotfix or tooling change. Per the release-vehicle rule, product features belong on `develop`; `stable` is for hotfixes and repo-only changes. If this feature must ship in the 1.23.0 minor, that should be decided explicitly rather than defaulting to `stable` in the suggested next steps.</comment>
<file context>
@@ -0,0 +1,84 @@
+
+## 7. Suggested next steps
+
+1. **Open a PR** for branch `dga/feat-409-retry-ivj0i` (base `stable`) — the feature is complete, tested (62 passing), and reviewed. Ensure both towncrier fragments (`changelog/1124.added.md`, `1124.changed.md`) are included; the `429 → RateLimitError` behaviour change is a caller-visible change flagged in the changed fragment.
+2. **Optional hardening** (deferred LOW findings): decide whether a 429 retry on a non-seekable multipart stream should raise a clear error instead of silently sending an empty body; add the small missing tests (`retry_after is None` exhaustion, Config defaults/validators).
+3. **Separate follow-up** for the repo's `docs-generate`/markdownlint tooling breakage (missing `.markdownlint.yaml`; unrelated `.mdx` drift) — outside this feature's scope.
</file context>
|
|
||
| ## Summary | ||
|
|
||
| Add a first-class request-priority concept to the SDK, emitted as an `X-Priority: high|normal|low` HTTP header. Two configuration surfaces: a client-wide default via `Config.priority` (rides every transport by being injected into the client's base `self.headers`) and a per-request `priority=` keyword argument on the covered public methods (resolved as `per_request if per_request is not None else client_default`). When nothing is configured, no header is emitted — byte-for-byte identical to today. Both `InfrahubClient` (async) and `InfrahubClientSync` (sync) behave identically. No server-side logic, no 429 handling. |
Contributor
There was a problem hiding this comment.
P3: The documented resolution rule is incomplete. The Summary says the header resolves 'exactly' as per_request if per_request is not None else client_default, and that application lives only in execute_graphql/_execute_graphql_with_file. In the shipped implementation _request_headers (infrahub_sdk/client.py:256-261) adds a third source: when the per-request priority kwarg is None but self._request_context.priority is set, that value overrides the client-wide default. Since this plan is merged post-implementation as part of the release prep, update the resolution rule and the 'single points' claim to mention request_context so the doc matches the released behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ihs-259-sdk-x-priority-header/plan.md, line 9:
<comment>The documented resolution rule is incomplete. The Summary says the header resolves 'exactly' as `per_request if per_request is not None else client_default`, and that application lives only in `execute_graphql`/`_execute_graphql_with_file`. In the shipped implementation `_request_headers` (infrahub_sdk/client.py:256-261) adds a third source: when the per-request `priority` kwarg is None but `self._request_context.priority` is set, that value overrides the client-wide default. Since this plan is merged post-implementation as part of the release prep, update the resolution rule and the 'single points' claim to mention `request_context` so the doc matches the released behavior.</comment>
<file context>
@@ -0,0 +1,115 @@
+
+## Summary
+
+Add a first-class request-priority concept to the SDK, emitted as an `X-Priority: high|normal|low` HTTP header. Two configuration surfaces: a client-wide default via `Config.priority` (rides every transport by being injected into the client's base `self.headers`) and a per-request `priority=` keyword argument on the covered public methods (resolved as `per_request if per_request is not None else client_default`). When nothing is configured, no header is emitted — byte-for-byte identical to today. Both `InfrahubClient` (async) and `InfrahubClientSync` (sync) behave identically. No server-side logic, no 429 handling.
+
+**Technical approach** (grounded in the existing `X-Infrahub-Tracker` prior art):
</file context>
|
|
||
| ## 7. Suggested next steps | ||
|
|
||
| 1. **Open a PR** for `dga/feat-x-priority-aa2nd` → `stable` (this is a public-API change per IHS-259 governance; the PR description should call that out). |
Contributor
There was a problem hiding this comment.
P3: Per release-vehicle guidance, a product feature (the X-Priority request-header API, not a fix) should target develop, not stable. §7 step 1 instructs opening this public-API feature to stable; that is the one base-branch case worth flagging. Low-severity note only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ihs-259-sdk-x-priority-header/opsmill-implement-report.md, line 103:
<comment>Per release-vehicle guidance, a product feature (the X-Priority request-header API, not a fix) should target develop, not stable. §7 step 1 instructs opening this public-API feature to `stable`; that is the one base-branch case worth flagging. Low-severity note only.</comment>
<file context>
@@ -0,0 +1,106 @@
+
+## 7. Suggested next steps
+
+1. **Open a PR** for `dga/feat-x-priority-aa2nd` → `stable` (this is a public-API change per IHS-259 governance; the PR description should call that out).
+2. (Optional) Address the deferred Low findings: add one-line docstrings with the `priority` Args entry to `get`/`create_diff`/`get_diff_summary`, and consider the `_apply_priority` helper to drop the two `# noqa: PLR0912`.
+3. The 8 pre-existing `ctl`/`pytest_plugin` unit failures are unrelated to this feature but exist on `stable`'s merge-base — worth a separate ticket if not already tracked.
</file context>
|
|
||
| None. All 35 tasks are `[X]`. | ||
|
|
||
| - **Nuance (not incomplete)**: T016/T018 deliberately excluded `client.create` from the `priority=` kwarg. Reason (from the subagent, confirmed by the code review): `client.create` only constructs an unsaved `InfrahubNode` and issues no HTTP request — the create request is made by `node.save()`/`node.create()`, which DO carry `priority` (T019/T020, tested). Adding an unused kwarg to `client.create` would be a misleading no-op and a lint error. FR-005's "create" surface is therefore satisfied at the request-issuing layer. |
Contributor
There was a problem hiding this comment.
P3: The claim that client.create "issues no HTTP request" is inaccurate: client.create calls await self.schema.get(...) (infrahub_sdk/client.py:467), a real HTTP request, before returning the unsaved node. The reasoning still holds for the create mutation (that fires at node.save), but the sentence should read "issues no create request" rather than "issues no HTTP request".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ihs-259-sdk-x-priority-header/opsmill-implement-report.md, line 34:
<comment>The claim that `client.create` "issues no HTTP request" is inaccurate: `client.create` calls `await self.schema.get(...)` (infrahub_sdk/client.py:467), a real HTTP request, before returning the unsaved node. The reasoning still holds for the *create* mutation (that fires at `node.save`), but the sentence should read "issues no create request" rather than "issues no HTTP request".</comment>
<file context>
@@ -0,0 +1,106 @@
+
+None. All 35 tasks are `[X]`.
+
+- **Nuance (not incomplete)**: T016/T018 deliberately excluded `client.create` from the `priority=` kwarg. Reason (from the subagent, confirmed by the code review): `client.create` only constructs an unsaved `InfrahubNode` and issues no HTTP request — the create request is made by `node.save()`/`node.create()`, which DO carry `priority` (T019/T020, tested). Adding an unused kwarg to `client.create` would be a misleading no-op and a lint error. FR-005's "create" surface is therefore satisfied at the request-issuing layer.
+
+## 4. Local-pass evidence
</file context>
| ## priority | ||
|
|
||
| <!-- vale on --> | ||
| **Description**: Default request priority emitted as the X-Priority header on every request; one of high|medium|low (case-insensitive). When unset, no header is sent.<br /> |
Contributor
There was a problem hiding this comment.
P3: priority is a string enum (high|medium|low, defined in infrahub_sdk/constants.py), but the new docs line labels its Type: object, which gives readers no hint that the value is a string. This mirrors how mode/transport are also rendered as object because the config.mdx generator (docs/docs_generation/helpers.py build_config_properties) falls back to "object" when the JSON-schema type resolves through a $ref. The mismatch is user-facing; rendering the real string type for enum fields would fix all three. If left to the generator, this line is correct as generated, but the generator's fallback is what makes the label wrong.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/docs/python-sdk/reference/config.mdx, line 139:
<comment>`priority` is a string enum (high|medium|low, defined in infrahub_sdk/constants.py), but the new docs line labels its **Type**: `object`, which gives readers no hint that the value is a string. This mirrors how mode/transport are also rendered as `object` because the config.mdx generator (docs/docs_generation/helpers.py `build_config_properties`) falls back to `"object"` when the JSON-schema type resolves through a $ref. The mismatch is user-facing; rendering the real `string` type for enum fields would fix all three. If left to the generator, this line is correct as generated, but the generator's fallback is what makes the label wrong.</comment>
<file context>
@@ -133,6 +133,14 @@ The following settings can be defined in the `Config` class
+## priority
+
+<!-- vale on -->
+**Description**: Default request priority emitted as the X-Priority header on every request; one of high|medium|low (case-insensitive). When unset, no header is sent.<br />
+**Type**: `object`<br />
+**Environment variable**: `INFRAHUB_PRIORITY`<br />
</file context>
ogenstad
approved these changes
Aug 19, 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
9 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 by cubic
Prepares SDK 1.23.0 with automatic HTTP 429 backoff, request priority, a comment‑preserving schema formatter with offline validation, and server deployment ID exposure. Also removes the dead branch‑diff path, updates protocols (Infrahub 1.11), adds the
IPAddresskind, improves task APIs/diagnostics, optimizes pagination, and switches versioning tohatch-vcs.Networking/resilience: transparent retry on HTTP 429 across regular, multipart, and streaming requests; configurable via rate_limit_*; default enabled with a retry budget of 10; raises RateLimitError when exhausted.
Request priority: new
Priorityenum (high|medium|low) andConfig.priority; emits X‑Priority when set; per‑requestpriority=andRequestContext.prioritywith precedence per‑request > context > config; async/sync parity.CLI/schema:
infrahubctl schema format(comment‑preserving viaruamel.yaml) with canonical key ordering and optional--strip-defaults,--sort-by-order-weight,--backfill-order-weight;schema validateruns offline against generated write models and reports warnings for non‑settable fields; warn‑only schema‑drift check and baseline added.Schema contract/models: commit generated write/read models under
infrahub_sdk/schema/generated/used by the offline validator.Server info: new
get_server_information()returns version and deployment ID;infrahubctl infoshows the deployment ID.Branch/protocols/types: removed
client.branch.diff_data()(always 404); addBranchStatus.MERGE_FAILED; update protocols for Infrahub 1.11; supportIPAddressattribute kind.Tasks: add retry/cancel helpers, surface available actions, and optional webhook delivery diagnostics.
Performance: pagination offset/limit now pass as GraphQL variables to improve cache reuse.
Release/CI: derive package version from git tags via
hatch-vcs; CI workflows fetch full history and tags; repository‑dispatch fan‑out includesopsmill/infrahub-sync.Repository config: add advisory messages encouraging a
watchblock in.infrahub.ymlfor generators.Migration and rollout
rate_limit_retry_enabled,rate_limit_max_retries,rate_limit_backoff_base,rate_limit_backoff_max.client.branch.diff_data()withget_diff_tree()orget_diff_summary().Config.priority(high|medium|low) to emit X‑Priority; override per request withpriority=or viaRequestContext.priority. When unset, no header is sent.hatch-vcs; ensure releases fetch tags (actions/checkoutwithfetch-depth: 0andfetch-tags: true). The generatedinfrahub_sdk/_version.pyis ignored by git.infrahubctl schema format --checkto CI; optional flags are safe and idempotent; the schema‑drift check is warn‑only.infrahubctl object loadimportspyarrowlazily; install thectlextra if needed:pip install 'infrahub-sdk[ctl]'.Written for commit eabb609. Summary will update on new commits.