Skip to content

Merge 'stable' to 'develop' with resolved conflicts - #1260

Merged
ogenstad merged 113 commits into
developfrom
pog-stable-to-develop-20260820
Aug 20, 2026
Merged

Merge 'stable' to 'develop' with resolved conflicts#1260
ogenstad merged 113 commits into
developfrom
pog-stable-to-develop-20260820

Conversation

@ogenstad

@ogenstad ogenstad commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Why

Bring in latest changes from 'stable' to 'develop'.

Replaces #1257

What changed

  • All changes from stable
  • Two merge conflicts
    • infrahub_sdk/config.py:72 Config.rate_limit_max_retries (kept the value from infrahub-develop of 10 it was initially set to 5
    • docs/docs/python-sdk/reference/config.mdx: Regenerated the file completely (same conflict as the config)

Summary by cubic

Brings the latest stable into develop and resolves two conflicts. Notable behavior changes: requests can carry an X-Priority header (new), HTTP 429 now retries with backoff (default retries raised from 5 to 10), schema write/read models are generated and validated offline, task actions are exposed, and a dead branch.diff_data API is removed.

  • Resolved conflicts: kept Config.rate_limit_max_retries = 10 (was 5 on stable); regenerated docs/python-sdk/reference/config.mdx.
  • Priority header: new Priority enum (high|medium|low), Config.priority default, per-call priority= kwarg, and RequestContext.priority. Precedence: per-call > request context > config. Header rides GraphQL, multipart, and blob transports; relogin now preserves refreshed auth while honoring per-request overrides.
  • Rate limiting: new retry handler applies across regular, multipart (with safe re-reads), and streaming requests. Emits warnings per retry and raises RateLimitError on exhaustion.
  • Schema: committed generated write/read models and an offline validate_schema(); CLI schema load/check validates locally first. Attribute kind standardized to Text (replaces String). Export preserves non-default ordered: false.
  • Tasks: add retry()/cancel(), surface available_actions, optional diagnostics (error, webhook HTTP request/response), and subtype for webhook deliveries.
  • Protocols and types: add bare IPAddress attribute support and new core fields; BranchStatus gains MERGE_FAILED.
  • Performance and docs: pagination uses GraphQL variables for better caching; docs updated for diff methods (get_diff_tree/get_diff_summary) and priority params; lazy pyarrow import limits ctl dependency surface.

Bolded actions required for migration:

  • Replace client.branch.diff_data() with client.get_diff_tree() or client.get_diff_summary().
  • If you relied on the old 5-retry behavior, set Config.rate_limit_max_retries explicitly to 5; default is now 10.
  • If you authored schemas with kind: "String", change them to kind: "Text".
  • To tag request priority, set Config.priority, pass priority= per call, or set RequestContext.priority; unconfigured clients emit no X-Priority header.

Written for commit f210dde. Summary will update on new commits.

Review in cubic

wvandeun and others added 30 commits August 19, 2026 14:31
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>
petercrocker and others added 21 commits August 19, 2026 16:51
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.
Bumps [infrahub-testcontainers](https://github.com/opsmill/infrahub) from 1.10.6 to 1.10.8.
- [Release notes](https://github.com/opsmill/infrahub/releases)
- [Changelog](https://github.com/opsmill/infrahub/blob/stable/CHANGELOG.md)
- [Commits](opsmill/infrahub@infrahub-v1.10.6...infrahub-v1.10.8)

---
updated-dependencies:
- dependency-name: infrahub-testcontainers
  dependency-version: 1.10.7
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…estcontainers-1.10.7

chore(deps-dev): bump infrahub-testcontainers from 1.10.6 to 1.10.8
@github-actions github-actions Bot added the type/documentation Improvements or additions to documentation label Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.27273% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/schema/validate.py 94.44% 3 Missing and 2 partials ⚠️
infrahub_sdk/ctl/schema.py 80.00% 3 Missing ⚠️
infrahub_sdk/ctl/validate.py 81.81% 1 Missing and 1 partial ⚠️
infrahub_sdk/node/node.py 89.47% 2 Missing ⚠️
infrahub_sdk/transfer/importer/json.py 50.00% 2 Missing ⚠️
infrahub_sdk/client.py 97.72% 1 Missing ⚠️
infrahub_sdk/constants.py 90.90% 0 Missing and 1 partial ⚠️
infrahub_sdk/object_store.py 83.33% 1 Missing ⚠️
infrahub_sdk/schema/__init__.py 80.00% 1 Missing ⚠️
infrahub_sdk/schema/generated/write.py 99.37% 1 Missing ⚠️
... and 2 more
@@             Coverage Diff             @@
##           develop    #1260      +/-   ##
===========================================
- Coverage    85.21%   84.16%   -1.06%     
===========================================
  Files          147      147              
  Lines        15822    13045    -2777     
  Branches      2705     1930     -775     
===========================================
- Hits         13483    10979    -2504     
+ Misses        1674     1503     -171     
+ Partials       665      563     -102     
Flag Coverage Δ
integration-tests 39.01% <13.50%> (-2.44%) ⬇️
python-3.10 56.99% <28.05%> (-1.14%) ⬇️
python-3.11 56.97% <28.05%> (-1.17%) ⬇️
python-3.12 56.97% <28.05%> (-1.17%) ⬇️
python-3.13 56.99% <28.05%> (-1.15%) ⬇️
python-3.14 56.99% <28.05%> (-1.15%) ⬇️
python-filler-3.12 23.68% <68.57%> (-0.92%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/branch.py 82.11% <100.00%> (+4.22%) ⬆️
infrahub_sdk/config.py 91.46% <100.00%> (-3.30%) ⬇️
infrahub_sdk/context.py 100.00% <100.00%> (+33.33%) ⬆️
infrahub_sdk/ctl/config.py 67.85% <ø> (ø)
infrahub_sdk/node/__init__.py 100.00% <ø> (ø)
infrahub_sdk/node/attribute.py 100.00% <100.00%> (ø)
infrahub_sdk/node/constants.py 100.00% <100.00%> (ø)
infrahub_sdk/node/related_node.py 91.09% <100.00%> (-1.66%) ⬇️
infrahub_sdk/protocols.py 100.00% <100.00%> (ø)
infrahub_sdk/protocols_base.py 78.29% <100.00%> (+2.78%) ⬆️
... and 25 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 issues found across 77 files

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-259-sdk-x-priority-header/data-model.md">

<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/data-model.md:14">
P3: The data-model spec defines the middle Priority member as `NORMAL` with wire value `normal`, but the shipped implementation `Priority` enum (infrahub_sdk/constants.py) defines `MEDIUM = "medium"` — there is no `NORMAL` member and the wire value is `medium`, not `normal`. The resolution truth table and config example in this spec repeat the same `NORMAL`/`normal` naming. Update the spec to `MEDIUM`/`medium` so the documented contract matches the code, or the spec will mislead implementers of this feature.</violation>
</file>

<file name="dev/specs/ihs-259-sdk-x-priority-header/research.md">

<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/research.md:10">
P3: The rationale sentence says `Priority.LOW.value == "high"`, but `Priority.LOW.value` is `"low"`; the exact wire token example is wrong and should reference `Priority.HIGH.value == "high"`. This is a factual error in a doc that claims to confirm the wire contract against current code.</violation>

<violation number="2" location="dev/specs/ihs-259-sdk-x-priority-header/research.md:19">
P3: Decision 1 proposes the enum member `NORMAL = "normal"`, but the implementation this doc claims to confirm already uses `MEDIUM = "medium"` (`infrahub_sdk/constants.py:20`), and the `Config.priority` description says "one of high|medium|low". The wire token the doc fixes ("normal") does not match what the SDK sends ("medium"), so the doc would mislead any implementer who follows it. Align the enum shown here with the implemented HIGH/MEDIUM/LOW.</violation>
</file>

<file name="dev/specs/ihs-259-sdk-x-priority-header/quickstart.md">

<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/quickstart.md:45">
P2: The quickstart's examples reference `Priority.NORMAL` and expect an `X-Priority: normal` header, but the implemented enum infrahub_sdk/constants.py:12-23 has `HIGH/MEDIUM/LOW` with no `NORMAL` member (config.py and tests/unit/sdk/test_priority.py use `medium`). Following this guide raises `AttributeError: type object 'Priority' has no attribute 'NORMAL'` and the asserted wire value `normal` never matches. Update every `NORMAL`/`normal` reference here (and the matching spec docs) to `MEDIUM`/`medium`, or correct the enum if NORMAL is the intended design.</violation>
</file>

<file name="infrahub_sdk/node/node.py">

<violation number="1" location="infrahub_sdk/node/node.py:287">
P2: When `RequestContext(priority=...)` is supplied directly to a node mutation, this removes the priority before building the body but never forwards it to the request header. The client default can therefore win, or no priority can be sent; resolve the effective priority from the explicit request context, with the `priority` keyword taking precedence.</violation>
</file>

<file name="infrahub_sdk/schema/main.py">

<violation number="1" location="infrahub_sdk/schema/main.py:49">
P2: Existing callers importing `NodeExtensionSchema` now fail during module import. Retain a compatibility class or alias before removing this public model name.</violation>
</file>

<file name=".vale/styles/Infrahub/sentence-case.yml">

<violation number="1" location=".vale/styles/Infrahub/sentence-case.yml:48">
P3: The new `IPAddress` exception breaks the alphabetical ordering of the list. Case-insensitively, `ipaddress` sorts before `ipam` (the `d` in IPAddress precedes the `m` in IPAM), but it is inserted after `IPAM`. Move it above `IPAM` to match the list's ordering convention.</violation>
</file>

<file name="dev/specs/ihs-259-sdk-x-priority-header/contracts/priority-api.md">

<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/contracts/priority-api.md:12">
P2: This contract document names the public enum member `NORMAL = "normal"` and says config accepts `high|normal|low`, but the actual shipped `infrahub_sdk.constants.Priority` defines `MEDIUM = "medium"` (constants.py:20) and `Config.priority`'s description lists `high|medium|low` (config.py:63), matching the tests' `Priority.MEDIUM`. A consumer following this contract would reference `Priority.NORMAL`, which does not exist, so the contract misstates the public API surface. Align the doc with the implementation: use `MEDIUM = "medium"` and `high|medium|low`.</violation>
</file>

<file name="dev/specs/ihs-259-sdk-x-priority-header/spec.md">

<violation number="1" location="dev/specs/ihs-259-sdk-x-priority-header/spec.md:111">
P2: The spec documents the middle priority as `normal`/`NORMAL` throughout (overview, acceptance 4, edge cases, FR-001/006/009, Key Entity, assumptions), but the SDK that ships this feature defines the closed set as `high | medium | low`: `Priority` in infrahub_sdk/constants.py:17-28 has `HIGH`, `MEDIUM`, `LOW` (values `high`/`medium`/`low`), config.py:59-66 documents "one of high|medium|low", and tests/unit/sdk/test_priority.py uses `Priority.MEDIUM -> "medium"`.

Because the enum's `_missing_` only matches `high`/`medium`/`low`, an operator who follows this spec and configures `normal` (or passes `Priority.NORMAL`) gets a validation error and never emits a header, so the documented behavior cannot be achieved as written. Align the spec with the implemented enum: replace `normal`/`NORMAL` with `medium`/`MEDIUM` (e.g. FR-001 "`high | medium | low`", FR-009 "(`high`, `medium`, `low`)`, Key Entity members `HIGH`, `MEDIUM`, `LOW`) so the documented value matches the wire value the SDK actually sends.</violation>

<violation number="2" location="dev/specs/ihs-259-sdk-x-priority-header/spec.md:115">
P2: FR-005's covered-method list conflicts with the rest of the spec docs in this batch. It lists `create` as a covered per-request-override method, but the contract (priority-api.md) and the data-model coverage table both state `client.create` is intentionally NOT extended (it builds an unsaved node and issues no HTTP request). FR-005 also omits `filters`, `count`, and node `update`/`delete`, which the contract and data-model do extend. Align FR-005 with the contract so implementers get one authoritative surface.</violation>
</file>

<file name="infrahub_sdk/schema/validate.py">

<violation number="1" location="infrahub_sdk/schema/validate.py:170">
P2: When a YAML schema contains a non-string mapping key, this loop can crash while reporting extras because Python cannot sort mixed `int`/`str` keys. Normalize keys for ordering and for the reported location, or report the malformed key as a validation error instead of letting the validator raise an unrelated `TypeError`.</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">
P2: This plan contradicts the code shipped in the same checkout. It says the SDK emits `X-Priority: high|normal|low` with a `Priority` member `NORMAL="normal"`, but the actual `Priority` enum (`infrahub_sdk/constants.py:12-31`) is `HIGH="high"`, `MEDIUM="medium"`, `LOW="low"` — no `normal` exists anywhere in the SDK, and the tests all use `MEDIUM`/`medium`. The middle wire value is `medium`, not `normal`. Align the plan with the implemented enum (and confirm with the server contract, which per `dev/specs/ihs-259-sdk-x-priority-header/spec.md:140` treats unknown values as `normal`) so a reader isn't told the SDK emits `normal` when it emits `medium`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

node = await client.get(kind="BuiltinTag", name__value="blue", priority=Priority.HIGH)

# Explicit NORMAL beats a LOW default for this call only:
await client.execute_graphql(query=MY_QUERY, priority=Priority.NORMAL) # -> X-Priority: normal

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The quickstart's examples reference Priority.NORMAL and expect an X-Priority: normal header, but the implemented enum infrahub_sdk/constants.py:12-23 has HIGH/MEDIUM/LOW with no NORMAL member (config.py and tests/unit/sdk/test_priority.py use medium). Following this guide raises AttributeError: type object 'Priority' has no attribute 'NORMAL' and the asserted wire value normal never matches. Update every NORMAL/normal reference here (and the matching spec docs) to MEDIUM/medium, or correct the enum if NORMAL is the intended design.

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/quickstart.md, line 45:

<comment>The quickstart's examples reference `Priority.NORMAL` and expect an `X-Priority: normal` header, but the implemented enum infrahub_sdk/constants.py:12-23 has `HIGH/MEDIUM/LOW` with no `NORMAL` member (config.py and tests/unit/sdk/test_priority.py use `medium`). Following this guide raises `AttributeError: type object 'Priority' has no attribute 'NORMAL'` and the asserted wire value `normal` never matches. Update every `NORMAL`/`normal` reference here (and the matching spec docs) to `MEDIUM`/`medium`, or correct the enum if NORMAL is the intended design.</comment>

<file context>
@@ -0,0 +1,93 @@
+node = await client.get(kind="BuiltinTag", name__value="blue", priority=Priority.HIGH)
+
+# Explicit NORMAL beats a LOW default for this call only:
+await client.execute_graphql(query=MY_QUERY, priority=Priority.NORMAL)  # -> X-Priority: normal
+```
+
</file context>

Comment thread infrahub_sdk/node/node.py
# priority rides the X-Priority header, not the mutation body — the server context input has no such field
if request_context:
return request_context.model_dump(exclude_none=True)
return request_context.model_dump(exclude_none=True, exclude={"priority"}) or None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When RequestContext(priority=...) is supplied directly to a node mutation, this removes the priority before building the body but never forwards it to the request header. The client default can therefore win, or no priority can be sent; resolve the effective priority from the explicit request context, with the priority keyword taking precedence.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/node/node.py, line 287:

<comment>When `RequestContext(priority=...)` is supplied directly to a node mutation, this removes the priority before building the body but never forwards it to the request header. The client default can therefore win, or no priority can be sent; resolve the effective priority from the explicit request context, with the `priority` keyword taking precedence.</comment>

<file context>
@@ -282,14 +282,15 @@ def __setattr__(self, name: str, value: Any) -> None:
+        # priority rides the X-Priority header, not the mutation body — the server context input has no such field
         if request_context:
-            return request_context.model_dump(exclude_none=True)
+            return request_context.model_dump(exclude_none=True, exclude={"priority"}) or None
 
         client: InfrahubClient | InfrahubClientSync | None = getattr(self, "_client", None)
</file context>

@@ -1,155 +1,155 @@
from __future__ import annotations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Existing callers importing NodeExtensionSchema now fail during module import. Retain a compatibility class or alias before removing this public model name.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/schema/main.py, line 49:

<comment>Existing callers importing `NodeExtensionSchema` now fail during module import. Retain a compatibility class or alias before removing this public model name.</comment>

<file context>
@@ -1,155 +1,155 @@
+# ``main.py`` keeps the public names stable by re-exporting the enums and by subclassing the
+# generated data models with the hand-written behavior below. The historical import paths
+# (``from infrahub_sdk.schema.main import AttributeKind, NodeSchema, ...``) keep working.
+__all__ = [
+    "AllowOverrideType",
+    "AttributeKind",
</file context>


class Priority(str, enum.Enum):
HIGH = "high"
NORMAL = "normal"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This contract document names the public enum member NORMAL = "normal" and says config accepts high|normal|low, but the actual shipped infrahub_sdk.constants.Priority defines MEDIUM = "medium" (constants.py:20) and Config.priority's description lists high|medium|low (config.py:63), matching the tests' Priority.MEDIUM. A consumer following this contract would reference Priority.NORMAL, which does not exist, so the contract misstates the public API surface. Align the doc with the implementation: use MEDIUM = "medium" and high|medium|low.

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/contracts/priority-api.md, line 12:

<comment>This contract document names the public enum member `NORMAL = "normal"` and says config accepts `high|normal|low`, but the actual shipped `infrahub_sdk.constants.Priority` defines `MEDIUM = "medium"` (constants.py:20) and `Config.priority`'s description lists `high|medium|low` (config.py:63), matching the tests' `Priority.MEDIUM`. A consumer following this contract would reference `Priority.NORMAL`, which does not exist, so the contract misstates the public API surface. Align the doc with the implementation: use `MEDIUM = "medium"` and `high|medium|low`.</comment>

<file context>
@@ -0,0 +1,74 @@
+
+class Priority(str, enum.Enum):
+    HIGH = "high"
+    NORMAL = "normal"
+    LOW = "low"
+```
</file context>
Suggested change
NORMAL = "normal"
MEDIUM = "medium"


### Functional Requirements

- **FR-001**: System MUST expose a closed set of priority values `high | normal | low` as a `Priority` enum, so callers express intent as a typed value rather than a raw header string.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The spec documents the middle priority as normal/NORMAL throughout (overview, acceptance 4, edge cases, FR-001/006/009, Key Entity, assumptions), but the SDK that ships this feature defines the closed set as high | medium | low: Priority in infrahub_sdk/constants.py:17-28 has HIGH, MEDIUM, LOW (values high/medium/low), config.py:59-66 documents "one of high|medium|low", and tests/unit/sdk/test_priority.py uses Priority.MEDIUM -> "medium".

Because the enum's _missing_ only matches high/medium/low, an operator who follows this spec and configures normal (or passes Priority.NORMAL) gets a validation error and never emits a header, so the documented behavior cannot be achieved as written. Align the spec with the implemented enum: replace normal/NORMAL with medium/MEDIUM (e.g. FR-001 "high | medium | low", FR-009 "(high, medium, low), Key Entity members HIGH, MEDIUM, LOW`) so the documented value matches the wire value the SDK actually sends.

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/spec.md, line 111:

<comment>The spec documents the middle priority as `normal`/`NORMAL` throughout (overview, acceptance 4, edge cases, FR-001/006/009, Key Entity, assumptions), but the SDK that ships this feature defines the closed set as `high | medium | low`: `Priority` in infrahub_sdk/constants.py:17-28 has `HIGH`, `MEDIUM`, `LOW` (values `high`/`medium`/`low`), config.py:59-66 documents "one of high|medium|low", and tests/unit/sdk/test_priority.py uses `Priority.MEDIUM -> "medium"`.

Because the enum's `_missing_` only matches `high`/`medium`/`low`, an operator who follows this spec and configures `normal` (or passes `Priority.NORMAL`) gets a validation error and never emits a header, so the documented behavior cannot be achieved as written. Align the spec with the implemented enum: replace `normal`/`NORMAL` with `medium`/`MEDIUM` (e.g. FR-001 "`high | medium | low`", FR-009 "(`high`, `medium`, `low`)`, Key Entity members `HIGH`, `MEDIUM`, `LOW`) so the documented value matches the wire value the SDK actually sends.</comment>

<file context>
@@ -0,0 +1,156 @@
+
+### Functional Requirements
+
+- **FR-001**: System MUST expose a closed set of priority values `high | normal | low` as a `Priority` enum, so callers express intent as a typed value rather than a raw header string.
+- **FR-002**: Users MUST be able to configure a client-wide default priority via configuration, accepting either a `Priority` enum value or a case-insensitive string (through environment or file configuration).
+- **FR-003**: When a default priority is configured, System MUST attach the `X-Priority` header to **every** outgoing request across all transports — GraphQL queries/mutations, multipart uploads, and raw blob `_get`/`_post`.
</file context>

- **FR-002**: Users MUST be able to configure a client-wide default priority via configuration, accepting either a `Priority` enum value or a case-insensitive string (through environment or file configuration).
- **FR-003**: When a default priority is configured, System MUST attach the `X-Priority` header to **every** outgoing request across all transports — GraphQL queries/mutations, multipart uploads, and raw blob `_get`/`_post`.
- **FR-004**: When no priority is configured and none is supplied per request, System MUST omit the `X-Priority` header entirely, producing outgoing requests byte-for-byte identical to current (pre-feature) behaviour.
- **FR-005**: Users MUST be able to override priority per request via a `priority` argument (accepting a `Priority` value or `None`, default `None`) on the covered public methods: `get`, `all`, `create`, `save`, the diff methods, `execute_graphql`, and its file variant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: FR-005's covered-method list conflicts with the rest of the spec docs in this batch. It lists create as a covered per-request-override method, but the contract (priority-api.md) and the data-model coverage table both state client.create is intentionally NOT extended (it builds an unsaved node and issues no HTTP request). FR-005 also omits filters, count, and node update/delete, which the contract and data-model do extend. Align FR-005 with the contract so implementers get one authoritative surface.

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/spec.md, line 115:

<comment>FR-005's covered-method list conflicts with the rest of the spec docs in this batch. It lists `create` as a covered per-request-override method, but the contract (priority-api.md) and the data-model coverage table both state `client.create` is intentionally NOT extended (it builds an unsaved node and issues no HTTP request). FR-005 also omits `filters`, `count`, and node `update`/`delete`, which the contract and data-model do extend. Align FR-005 with the contract so implementers get one authoritative surface.</comment>

<file context>
@@ -0,0 +1,156 @@
+- **FR-002**: Users MUST be able to configure a client-wide default priority via configuration, accepting either a `Priority` enum value or a case-insensitive string (through environment or file configuration).
+- **FR-003**: When a default priority is configured, System MUST attach the `X-Priority` header to **every** outgoing request across all transports — GraphQL queries/mutations, multipart uploads, and raw blob `_get`/`_post`.
+- **FR-004**: When no priority is configured and none is supplied per request, System MUST omit the `X-Priority` header entirely, producing outgoing requests byte-for-byte identical to current (pre-feature) behaviour.
+- **FR-005**: Users MUST be able to override priority per request via a `priority` argument (accepting a `Priority` value or `None`, default `None`) on the covered public methods: `get`, `all`, `create`, `save`, the diff methods, `execute_graphql`, and its file variant.
+- **FR-006**: Priority resolution MUST be `resolved = per_request if per_request is not None else client_default`. A resolved value of `None` MUST omit the header; a resolved explicit value MUST be sent, including an explicit `NORMAL`, which MUST send `X-Priority: normal`.
+- **FR-007**: System MUST reject an invalid or unknown configured priority value at configuration-load time (a validation/type error) rather than coercing it or silently sending it.
</file context>
Suggested change
- **FR-005**: Users MUST be able to override priority per request via a `priority` argument (accepting a `Priority` value or `None`, default `None`) on the covered public methods: `get`, `all`, `create`, `save`, the diff methods, `execute_graphql`, and its file variant.
- **FR-005**: Users MUST be able to override priority per request via a `priority` argument (accepting a `Priority` value or `None`, default `None`) on the covered public methods: `get`, `all`, `filters`, `count`, the diff methods, `execute_graphql`, its file variant, and the node-level `save`, `create`, `update`, and `delete` methods (which forward it). `client.create` is excluded because it builds an unsaved node and issues no HTTP request.

| Member | Wire value | Meaning |
|----------|-----------|----------------------------------------------------------------|
| `HIGH` | `high` | Prefer this request; server should protect it under load. |
| `NORMAL` | `normal` | Default server treatment; equivalent to absent header. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The data-model spec defines the middle Priority member as NORMAL with wire value normal, but the shipped implementation Priority enum (infrahub_sdk/constants.py) defines MEDIUM = "medium" — there is no NORMAL member and the wire value is medium, not normal. The resolution truth table and config example in this spec repeat the same NORMAL/normal naming. Update the spec to MEDIUM/medium so the documented contract matches the code, or the spec will mislead implementers of this feature.

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/data-model.md, line 14:

<comment>The data-model spec defines the middle Priority member as `NORMAL` with wire value `normal`, but the shipped implementation `Priority` enum (infrahub_sdk/constants.py) defines `MEDIUM = "medium"` — there is no `NORMAL` member and the wire value is `medium`, not `normal`. The resolution truth table and config example in this spec repeat the same `NORMAL`/`normal` naming. Update the spec to `MEDIUM`/`medium` so the documented contract matches the code, or the spec will mislead implementers of this feature.</comment>

<file context>
@@ -0,0 +1,102 @@
+| Member   | Wire value | Meaning                                                        |
+|----------|-----------|----------------------------------------------------------------|
+| `HIGH`   | `high`    | Prefer this request; server should protect it under load.      |
+| `NORMAL` | `normal`  | Default server treatment; equivalent to absent header.         |
+| `LOW`    | `low`     | Sheddable first; intended for background/bulk workloads.       |
+
</file context>

# infrahub_sdk/constants.py
class Priority(str, enum.Enum):
HIGH = "high"
NORMAL = "normal"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Decision 1 proposes the enum member NORMAL = "normal", but the implementation this doc claims to confirm already uses MEDIUM = "medium" (infrahub_sdk/constants.py:20), and the Config.priority description says "one of high|medium|low". The wire token the doc fixes ("normal") does not match what the SDK sends ("medium"), so the doc would mislead any implementer who follows it. Align the enum shown here with the implemented HIGH/MEDIUM/LOW.

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/research.md, line 19:

<comment>Decision 1 proposes the enum member `NORMAL = "normal"`, but the implementation this doc claims to confirm already uses `MEDIUM = "medium"` (`infrahub_sdk/constants.py:20`), and the `Config.priority` description says "one of high|medium|low". The wire token the doc fixes ("normal") does not match what the SDK sends ("medium"), so the doc would mislead any implementer who follows it. Align the enum shown here with the implemented HIGH/MEDIUM/LOW.</comment>

<file context>
@@ -0,0 +1,83 @@
+# infrahub_sdk/constants.py
+class Priority(str, enum.Enum):
+    HIGH = "high"
+    NORMAL = "normal"
+    LOW = "low"
+
</file context>

## Decision 1 — Where the `Priority` enum lives and its shape

- **Decision**: `class Priority(str, enum.Enum)` in `infrahub_sdk/constants.py`, members `HIGH = "high"`, `NORMAL = "normal"`, `LOW = "low"`, plus a case-insensitive `_missing_` classmethod.
- **Rationale**: `constants.py` already hosts `InfrahubClientMode(str, enum.Enum)` (`constants.py:4`) and is already imported by both `config.py` (`config.py:11`) and `client.py`. A `str`-valued enum means `Priority.LOW.value == "high"`-style access gives the exact wire token, and the member *is* a `str` so it slots straight into a headers dict. `_missing_` lets `Priority("LOW")` resolve case-insensitively, which pydantic uses when coercing env/file strings.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The rationale sentence says Priority.LOW.value == "high", but Priority.LOW.value is "low"; the exact wire token example is wrong and should reference Priority.HIGH.value == "high". This is a factual error in a doc that claims to confirm the wire contract against current code.

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/research.md, line 10:

<comment>The rationale sentence says `Priority.LOW.value == "high"`, but `Priority.LOW.value` is `"low"`; the exact wire token example is wrong and should reference `Priority.HIGH.value == "high"`. This is a factual error in a doc that claims to confirm the wire contract against current code.</comment>

<file context>
@@ -0,0 +1,83 @@
+## Decision 1 — Where the `Priority` enum lives and its shape
+
+- **Decision**: `class Priority(str, enum.Enum)` in `infrahub_sdk/constants.py`, members `HIGH = "high"`, `NORMAL = "normal"`, `LOW = "low"`, plus a case-insensitive `_missing_` classmethod.
+- **Rationale**: `constants.py` already hosts `InfrahubClientMode(str, enum.Enum)` (`constants.py:4`) and is already imported by both `config.py` (`config.py:11`) and `client.py`. A `str`-valued enum means `Priority.LOW.value == "high"`-style access gives the exact wire token, and the member *is* a `str` so it slots straight into a headers dict. `_missing_` lets `Priority("LOW")` resolve case-insensitively, which pydantic uses when coercing env/file strings.
+- **Alternatives considered**:
+  - `infrahub_sdk/enums.py` (`OrderDirection`, `enums.py:4`) — viable, but `constants.py` is the home for *client/config-consumed* enums (`InfrahubClientMode`), which is exactly this case.
</file context>
Suggested change
- **Rationale**: `constants.py` already hosts `InfrahubClientMode(str, enum.Enum)` (`constants.py:4`) and is already imported by both `config.py` (`config.py:11`) and `client.py`. A `str`-valued enum means `Priority.LOW.value == "high"`-style access gives the exact wire token, and the member *is* a `str` so it slots straight into a headers dict. `_missing_` lets `Priority("LOW")` resolve case-insensitively, which pydantic uses when coercing env/file strings.
A `str`-valued enum means `Priority.HIGH.value == "high"`-style access gives the exact wire token, and the member *is* a `str` so it slots straight into a headers dict.

- IP
- IP Fabric
- IPAM
- IPAddress

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The new IPAddress exception breaks the alphabetical ordering of the list. Case-insensitively, ipaddress sorts before ipam (the d in IPAddress precedes the m in IPAM), but it is inserted after IPAM. Move it above IPAM to match the list's ordering convention.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .vale/styles/Infrahub/sentence-case.yml, line 48:

<comment>The new `IPAddress` exception breaks the alphabetical ordering of the list. Case-insensitively, `ipaddress` sorts before `ipam` (the `d` in IPAddress precedes the `m` in IPAM), but it is inserted after `IPAM`. Move it above `IPAM` to match the list's ordering convention.</comment>

<file context>
@@ -45,6 +45,7 @@ exceptions:
   - IP
   - IP Fabric
   - IPAM
+  - IPAddress
   - IPHost
   - IPNetwork
</file context>

@ogenstad
ogenstad marked this pull request as ready for review August 20, 2026 08:59
@ogenstad
ogenstad requested a review from a team as a code owner August 20, 2026 08:59
@ogenstad
ogenstad merged commit a87df40 into develop Aug 20, 2026
21 checks passed
@ogenstad
ogenstad deleted the pog-stable-to-develop-20260820 branch August 20, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants