-
Notifications
You must be signed in to change notification settings - Fork 12
Merge 'stable' to 'develop' with resolved conflicts #1260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
16cd5bc
dbf6986
db6e0c7
8300e68
10a45a0
7fce489
9a85580
169f1cb
08fa009
645553a
97886fd
f821605
54d96aa
51cd2c5
1fe0ccd
26ec3ec
ef6bbc5
ce019bd
112a7c5
28a33ad
d417166
bd93619
ab49028
fdfbe23
c40ab56
d4125e1
f1d46e3
f828cb1
c401806
9c74bfa
fefd01c
6fe1bf1
07eb001
bb88b2d
62e6be2
72540fd
5a0176e
2506cb6
e9e85ca
40d7ae6
3a490da
fead632
43990f3
a6b7d1d
89e848a
f0338b8
e0b4725
02b4da5
f736cc6
d3c2493
3b964a8
7e99f52
f7eb7ea
ef694f5
cd5a2c3
ab60207
48b9fd6
2952d5a
01f7c6f
7ee6566
fbcb569
fd7f55b
3596555
816d834
ba6f582
b0b5bce
c9e8b5d
e281381
c121d45
632e542
37f5847
cc72f69
30ddc7f
0db7361
f728151
231a285
ea27bcf
44d882c
9161e24
ea11136
b951dc7
b1dcb35
12af188
1face70
165ea52
d70aaa1
48359e9
438f1cd
07bc7ba
7779004
72fc224
c59cf87
d0c3a32
75a5b2c
1ea829b
c0cac67
e6f65a3
45f367e
99e923a
8f4de3c
7d0c1b0
4ea2bbf
78060c6
5fabbec
9474ffe
a592f7c
22fc7ac
c7313d5
eabb609
99a380a
7cf2424
d0537e5
f210dde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,7 @@ exceptions: | |
| - IP | ||
| - IP Fabric | ||
| - IPAM | ||
| - IPAddress | ||
| - IPHost | ||
| - IPNetwork | ||
| - JavaScript | ||
|
|
||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # Spec / Ask Alignment Check: SDK `X-Priority` Request Header | ||
|
|
||
| **Date**: 2026-07-10 | ||
| **Feature dir**: `specs/ihs-259-sdk-x-priority-header/` | ||
|
|
||
| ## 1. Source | ||
|
|
||
| - **Source PRD**: Jira **IHS-259** — "feat: SDK X-Priority request header" (`https://opsmill.atlassian.net/browse/IHS-259`), fetched via the Atlassian integration. The Jira issue description *is* a full PRD (Problem Statement, Solution Overview, 7 User Stories, FR-001…008, Key Entities, Edge Cases, SC-001…005, Implementation/Testing Decisions, Out of Scope, Assumptions). | ||
| - Compared against: `spec.md` (current, post-critique). | ||
|
|
||
| ## 2. Verdict | ||
|
|
||
| ✅ **ALIGNED** | ||
|
|
||
| The spec faithfully carries every PRD requirement, user story, acceptance criterion, edge case, and out-of-scope boundary. The only differences are expansions of detail and testability clarifications that preserve — and in two cases make verifiable — the PRD's stated intent. No requirement is missing, changed in meaning, dropped, softened, or contradicted. | ||
|
|
||
| ## 3. Findings | ||
|
|
||
| | Severity | Category | PRD reference | Spec reference | Description | | ||
| |----------|----------|---------------|----------------|-------------| | ||
| | ℹ️ Info (no drift) | mapping | PRD User Stories 1–7 | spec US1–US5 | 7 PRD stories consolidated into 5. All intent preserved: PRD US1→US1, US2 (enum) folded into FR-001 + contracts, US3 (override)→US2, US4 (rides every transport)→US1/FR-003, US5 (zero change)→US3, US6 (invalid rejected)→US4, US7 (async=sync)→US5. Consolidation, not loss. | | ||
| | ℹ️ Info (expansion) | added | PRD Assumptions ("header is exactly `X-Priority`") | spec FR-009 | Spec adds FR-009 stating the header name is exactly `X-Priority` with lowercase value. This promotes a PRD assumption to a testable requirement — expansion of detail, within PRD scope. | | ||
| | ℹ️ Info (expansion) | added | PRD Edge Cases ("batch mode and raw blob transfers inherit the client default") | spec SC-006 | Spec adds SC-006 verifying batch/blob inherit the configured default. Makes an implicit PRD scope claim testable; does not add new scope (no per-request override for these, matching the PRD). Raised by the critique (P5/X1). | | ||
| | ⚠️ Minor (clarified, not softened) | changed-wording | PRD SC-002 ("emits no `X-Priority` header — asserted byte-for-byte against current behaviour") | spec SC-002 | Reworded to "no `X-Priority` emitted; no other SDK-set outgoing header changes (assert `X-Priority` absent; not a literal byte-for-byte comparison of transport-injected headers)". The requirement (no header, no behaviour change) is unchanged; only the assertion method is clarified because httpx injects its own headers, making a literal byte comparison neither stable nor meaningful. Raised by the critique (E6). | | ||
|
|
||
| All FR-001…008 map 1:1 to spec FR-001…008. All SC-001, SC-003, SC-004, SC-005 map 1:1. All PRD edge cases and all five Out-of-Scope items (429/#1124, server-side/INFP-636, classification guidance, per-batch/blob knobs, anti-escalation) are present in the spec. | ||
|
|
||
| ## 4. Action | ||
|
|
||
| **Proceed.** No remediation required. The spec is aligned with IHS-259; the two ⚠️/expansion items are testability clarifications that strengthen the spec without departing from the PRD. `tasks.md` (Phase 4) is ready for review and implementation. | ||
|
|
||
| - Remediation passes used: **0**. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # Specification Quality Checklist: SDK `X-Priority` Request Header | ||
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
| **Created**: 2026-07-10 | ||
| **Feature**: [spec.md](../spec.md) | ||
|
|
||
| ## Content Quality | ||
|
|
||
| - [x] No implementation details (languages, frameworks, APIs) | ||
| - [x] Focused on user value and business needs | ||
| - [x] Written for non-technical stakeholders | ||
| - [x] All mandatory sections completed | ||
|
|
||
| ## Requirement Completeness | ||
|
|
||
| - [x] No [NEEDS CLARIFICATION] markers remain | ||
| - [x] Requirements are testable and unambiguous | ||
| - [x] Success criteria are measurable | ||
| - [x] Success criteria are technology-agnostic (no implementation details) | ||
| - [x] All acceptance scenarios are defined | ||
| - [x] Edge cases are identified | ||
| - [x] Scope is clearly bounded | ||
| - [x] Dependencies and assumptions identified | ||
|
|
||
| ## Feature Readiness | ||
|
|
||
| - [x] All functional requirements have clear acceptance criteria | ||
| - [x] User scenarios cover primary flows | ||
| - [x] Feature meets measurable outcomes defined in Success Criteria | ||
| - [x] No implementation details leak into specification | ||
|
|
||
| ## Notes | ||
|
|
||
| - The spec unavoidably names the concrete wire contract (`X-Priority` header, `Priority` enum, `Config` field, method kwarg) because these ARE the requirement contract handed down from the PRD (IHS-259) and the server-side effort (INFP-636), not free implementation choices. Enum/config/kwarg names are treated as the externally observable API surface, not internal implementation detail. | ||
| - No [NEEDS CLARIFICATION] markers were needed: the source PRD is detailed and unambiguous, with resolution rules, transport coverage, and testing decisions all specified. | ||
| - All items pass. Spec is ready for `/speckit-plan`. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,74 @@ | ||||||
| # API Contract: Priority public surface | ||||||
|
|
||||||
| **Feature**: IHS-259 | **Scope**: SDK public Python API (async + sync). This is a public-API-signature change (governance-approved in IHS-259). | ||||||
|
|
||||||
| ## New public symbol: `Priority` | ||||||
|
|
||||||
| ```python | ||||||
| from infrahub_sdk.constants import Priority | ||||||
|
|
||||||
| class Priority(str, enum.Enum): | ||||||
| HIGH = "high" | ||||||
| NORMAL = "normal" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This contract document names the public enum member Prompt for AI agents
Suggested change
|
||||||
| LOW = "low" | ||||||
| ``` | ||||||
|
|
||||||
| - `str`-valued closed enum. `Priority("LOW") is Priority.LOW` (case-insensitive via `_missing_`). | ||||||
| - Unknown values raise `ValueError` (→ `pydantic.ValidationError` at config load). | ||||||
| - Imported from `infrahub_sdk.constants` (kept out of the top-level `infrahub_sdk` namespace so importing the enum does not pull in `Config` and the client classes). | ||||||
|
|
||||||
| ## Extended: `Config.priority` | ||||||
|
|
||||||
| ```python | ||||||
| class ConfigBase(BaseSettings): | ||||||
| ... | ||||||
| priority: Priority | None = Field( | ||||||
| default=None, | ||||||
| description="Default request priority emitted as the X-Priority header on every request. " | ||||||
| "One of high|normal|low (case-insensitive). When unset, no header is sent.", | ||||||
| ) | ||||||
| ``` | ||||||
|
|
||||||
| - Env var: `INFRAHUB_PRIORITY`. | ||||||
| - Accepts a `Priority` or a case-insensitive string; unknown → validation error at load. | ||||||
| - Default `None` → no client-wide default. | ||||||
|
|
||||||
| ## Extended method signatures (new `priority` keyword — both `InfrahubClient` and `InfrahubClientSync`) | ||||||
|
|
||||||
| Each covered method gains `priority: Priority | None = None` (default `None` preserves current behaviour). The argument is keyword-friendly and additive — existing positional/keyword calls are unaffected. | ||||||
|
|
||||||
| ```python | ||||||
| # Client | ||||||
| def get(self, kind, ..., priority: Priority | None = None) -> ... | ||||||
| def all(self, kind, ..., priority: Priority | None = None) -> ... # forwards to filters + count | ||||||
| def filters(self, kind, ..., priority: Priority | None = None) -> ... | ||||||
| def count(self, kind, ..., priority: Priority | None = None) -> int | ||||||
| def execute_graphql(self, query, ..., priority: Priority | None = None) -> dict | ||||||
| def _execute_graphql_with_file(self, ..., priority: Priority | None = None) -> ... # file variant | ||||||
| def create_diff(self, ..., priority: Priority | None = None) -> ... | ||||||
| def get_diff_summary(self, ..., priority: Priority | None = None) -> ... | ||||||
| def get_diff_tree(self, ..., priority: Priority | None = None) -> ... | ||||||
|
|
||||||
| # Node (InfrahubNode / InfrahubNodeSync) | ||||||
| def save(self, ..., priority: Priority | None = None) -> None | ||||||
| def create(self, ..., priority: Priority | None = None) -> None | ||||||
| def update(self, ..., priority: Priority | None = None) -> None | ||||||
| def delete(self, ..., priority: Priority | None = None) -> None | ||||||
| ``` | ||||||
|
|
||||||
| > **Note — `client.create` is intentionally NOT extended.** `InfrahubClient.create()` / `InfrahubClientSync.create()` only build an unsaved `InfrahubNode` in memory and issue no HTTP request, so a `priority=` kwarg there would be a no-op. Per-request priority for creating a node is carried by the node-level `save()` / `create()` (which issue the mutation). Within a node create/update, a resource-pool relationship's follow-up peer fetch inherits the same `priority`. | ||||||
|
|
||||||
| ### Behavioural contract per call | ||||||
|
|
||||||
| - `priority=None` (default): use the client-wide default (which may itself be `None` → no header). No client state is mutated. | ||||||
| - `priority=Priority.X`: this request carries `X-Priority: x`, overriding the client default for this call only. The next un-annotated call reverts to the client default. | ||||||
| - Resolution: `resolved = per_request if per_request is not None else client_default`. | ||||||
|
|
||||||
| ## Explicitly NOT extended (v1) | ||||||
|
|
||||||
| - `_get`, `_post`, `_get_streaming` (raw blob transfers) — inherit the client default only; no `priority` kwarg. | ||||||
| - Batch mode — inherits the client default only; no per-call override. | ||||||
|
|
||||||
| ## Backwards-compatibility guarantee | ||||||
|
|
||||||
| - Adding a keyword-only-friendly parameter with a `None` default and a new optional config field is additive. Any existing caller that sets nothing sees no change in outgoing requests (FR-004 / SC-002). | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # Wire Contract: `X-Priority` HTTP header | ||
|
|
||
| **Feature**: IHS-259 | **Consumer**: Infrahub API server (INFP-636) | ||
|
|
||
| ## Header | ||
|
|
||
| | Property | Value | | ||
| |----------|-------| | ||
| | Name | `X-Priority` (exact, case-insensitive on the server per HTTP header rules) | | ||
| | Values | `high`, `normal`, `low` (lowercase emitted by the SDK) | | ||
| | Cardinality | 0 or 1 per request | | ||
|
|
||
| ## Emission rules (SDK side) | ||
|
|
||
| 1. The SDK emits the header on a request **iff** the resolved priority for that request is non-`None`. | ||
| 2. When emitted, the value is exactly the lowercase token of the resolved `Priority` member. | ||
| 3. The header is emitted uniformly across every transport when a client-wide default is configured: GraphQL query/mutation, multipart file upload, and raw blob `_get`/`_post`. | ||
| 4. When no priority is configured and none is passed per request, the header is **absent** — the outgoing request is byte-for-byte identical to the pre-feature SDK. | ||
|
|
||
| ## Server semantics (assumed, per INFP-636 — not implemented here) | ||
|
|
||
| - The server treats the value case-insensitively. | ||
| - An **absent** header and an **unknown** value are both treated as `normal`. | ||
| - Consequently, "omit the header" and "send `normal`" are server-equivalent, which is what makes omitting-when-unconfigured a safe, non-breaking rollout. | ||
|
|
||
| ## Non-goals (this contract) | ||
|
|
||
| - No `Retry-After` / 429 semantics (GitHub #1124). | ||
| - No server-side admission control, routing, or throttling behaviour (INFP-636). | ||
| - The SDK does not read or react to any response header related to priority. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The new
IPAddressexception breaks the alphabetical ordering of the list. Case-insensitively,ipaddresssorts beforeipam(thedin IPAddress precedes themin IPAM), but it is inserted afterIPAM. Move it aboveIPAMto match the list's ordering convention.Prompt for AI agents