feat(client): carry request priority on RequestContext#1205
Open
dgarros wants to merge 2 commits into
Open
Conversation
Add an optional `priority` field to RequestContext so a client-set context drives the X-Priority header. Resolution precedence is per-call `priority=` kwarg > request_context.priority > Config.priority default > no header. The priority is emitted as a header only and is excluded from the mutation body serialization (the server context input accepts only `account`). Part of INFP-636. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Align the SDK `Priority` enum with the backend `WorkflowPriority` (high/medium/low) so the X-Priority wire value and the enum members match. The case-insensitive resolution is unchanged. Part of INFP-636. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
the
type/documentation
Deploying infrahub-sdk-python with
|
| Latest commit: |
522c870
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bc8a3c42.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-feat-rate-limiting-api-z.infrahub-sdk-python.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 11 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="infrahub_sdk/node/node.py">
<violation number="1" location="infrahub_sdk/node/node.py:287">
P2: A per-call `RequestContext(priority=...)` is silently ignored: excluding it here removes the value from the mutation, but the subsequent transport call does not receive that per-call context and only checks `client.request_context`. Resolving the explicit context's priority into the effective `priority` argument would preserve the documented `priority=` > `request_context.priority` > `Config.priority` behavior.</violation>
</file>
<file name="infrahub_sdk/client.py">
<violation number="1" location="infrahub_sdk/client.py:258">
P2: File downloads, artifact-generation calls, and password-authentication requests still omit `RequestContext.priority` because they bypass `_request_headers`; the companion admission layer classifies a missing header as MEDIUM at opsmill/infrahub#9879/backend/infrahub/api/admission/middleware.py:87-92, so high/low work on these paths can be admitted or shed at the wrong tier. Centralizing the context-priority merge in the low-level transports would cover all request paths.</violation>
</file>
<file name="infrahub_sdk/constants.py">
<violation number="1" location="infrahub_sdk/constants.py:20">
P2: Existing SDK callers using `Priority.NORMAL` will fail at runtime after upgrading, making this a public API compatibility break. Keeping `NORMAL` as an alias of `MEDIUM` preserves those callers while emitting the new `medium` wire value.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # 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 |
Contributor
There was a problem hiding this comment.
P2: A per-call RequestContext(priority=...) is silently ignored: excluding it here removes the value from the mutation, but the subsequent transport call does not receive that per-call context and only checks client.request_context. Resolving the explicit context's priority into the effective priority argument would preserve the documented priority= > request_context.priority > Config.priority behavior.
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>A per-call `RequestContext(priority=...)` is silently ignored: excluding it here removes the value from the mutation, but the subsequent transport call does not receive that per-call context and only checks `client.request_context`. Resolving the explicit context's priority into the effective `priority` argument would preserve the documented `priority=` > `request_context.priority` > `Config.priority` behavior.</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>
| if priority is not None: | ||
| headers["X-Priority"] = priority.value | ||
| effective_priority = priority | ||
| if effective_priority is None and self._request_context is not None: |
Contributor
There was a problem hiding this comment.
P2: File downloads, artifact-generation calls, and password-authentication requests still omit RequestContext.priority because they bypass _request_headers; the companion admission layer classifies a missing header as MEDIUM at opsmill/infrahub#9879/backend/infrahub/api/admission/middleware.py:87-92, so high/low work on these paths can be admitted or shed at the wrong tier. Centralizing the context-priority merge in the low-level transports would cover all request paths.
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 258:
<comment>File downloads, artifact-generation calls, and password-authentication requests still omit `RequestContext.priority` because they bypass `_request_headers`; the companion admission layer classifies a missing header as MEDIUM at opsmill/infrahub#9879/backend/infrahub/api/admission/middleware.py:87-92, so high/low work on these paths can be admitted or shed at the wrong tier. Centralizing the context-priority merge in the low-level transports would cover all request paths.</comment>
<file context>
@@ -254,8 +254,11 @@ def _request_headers(self, tracker: str | None = None, priority: Priority | None
- if priority is not None:
- headers["X-Priority"] = priority.value
+ effective_priority = priority
+ if effective_priority is None and self._request_context is not None:
+ effective_priority = self._request_context.priority
+ if effective_priority is not None:
</file context>
|
|
||
| HIGH = "high" | ||
| NORMAL = "normal" | ||
| MEDIUM = "medium" |
Contributor
There was a problem hiding this comment.
P2: Existing SDK callers using Priority.NORMAL will fail at runtime after upgrading, making this a public API compatibility break. Keeping NORMAL as an alias of MEDIUM preserves those callers while emitting the new medium wire value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/constants.py, line 20:
<comment>Existing SDK callers using `Priority.NORMAL` will fail at runtime after upgrading, making this a public API compatibility break. Keeping `NORMAL` as an alias of `MEDIUM` preserves those callers while emitting the new `medium` wire value.</comment>
<file context>
@@ -17,7 +17,7 @@ class Priority(str, enum.Enum):
HIGH = "high"
- NORMAL = "normal"
+ MEDIUM = "medium"
LOW = "low"
</file context>
Suggested change
| MEDIUM = "medium" | |
| MEDIUM = "medium" | |
| NORMAL = MEDIUM |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #1205 +/- ##
====================================================
- Coverage 83.98% 83.26% -0.72%
====================================================
Files 139 139
Lines 14390 12442 -1948
Branches 2381 1853 -528
====================================================
- Hits 12085 10360 -1725
+ Misses 1654 1519 -135
+ Partials 651 563 -88
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 11 files with indirect coverage changes 🚀 New features to boost your workflow:
|
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
1 participant
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.
Companion SDK changes for the priority-aware server-side API backpressure feature in Infrahub.
RequestContextso a per-requestX-Prioritycan be propagated by callers.NORMAL→MEDIUM.Pairs with opsmill/infrahub#9879 (priority-aware server-side API backpressure). That PR bumps the
python_sdksubmodule pointer to this branch's tip, so this SDK PR must merge first (or alongside it).🤖 Generated with Claude Code
Summary by cubic
Adds
RequestContext.priorityto drive theX-Priorityheader on all client requests and renamesPriority.NORMALtoPriority.MEDIUMto align with backend. Supports INFP-636 (priority-aware API backpressure).New Features
RequestContext.priorityemitsX-PriorityonInfrahubClientandInfrahubClientSyncrequests.priority=>request_context.priority>Config.priority> none.Migration
Priority.NORMALwithPriority.MEDIUM; updateINFRAHUB_PRIORITY=normaltomedium.Written for commit 522c870. Summary will update on new commits.