feat: support overriding authentication per request#115
Conversation
Add an optional keyword-only `auth` parameter to every public client method (AuthClient, CoreClient, StorageClient) to override authentication for a single request, leaving the client's bound authentication in place for all others. `auth` accepts an httpx.Auth instance, a (username, password) tuple, or a string sent verbatim as the Authorization header. Following httpx conventions, omitting it keeps the client's bound auth and `auth=None` disables authentication for that request.
|
Warning Review limit reached
More reviews will be available in 21 minutes and 54 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR introduces per-request authentication overrides across the entire hub-python-client library. A ChangesPer-request authentication override
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
flame_hub/_base_client.py (1)
9-10: 📐 Maintainability & Code Quality | 🔵 TrivialUse public
httpx.USE_CLIENT_DEFAULTinstead of importing from privatehttpx._client.
httpx._clientis unsupported private API (per httpx maintainers). Replacefrom httpx._client import USE_CLIENT_DEFAULT, UseClientDefaultwithfrom httpx import USE_CLIENT_DEFAULT. For theUseClientDefaulttype, usetype(httpx.USE_CLIENT_DEFAULT)or define a local type alias to avoid depending on unstable internal exports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@flame_hub/_base_client.py` around lines 9 - 10, Replace the import statement that imports USE_CLIENT_DEFAULT and UseClientDefault from the private httpx._client module with an import of USE_CLIENT_DEFAULT from the public httpx module. For the UseClientDefault type annotation, use type(httpx.USE_CLIENT_DEFAULT) to get the type dynamically, or alternatively define a local type alias to avoid depending on the unstable private API. Update all references throughout the file to use the public imports instead of the private ones.tests/test_base_client.py (1)
277-283: 📐 Maintainability & Code Quality | 🔵 TrivialAdd
httpx.BasicAuthand tuple auth override cases to_AUTH_OVERRIDE_CASES.The accepted contract includes
(username, password)tuples andhttpx.BasicAuthoverrides, but the integration matrix currently covers only sentinel/string/_StaticAuthorization/None. Extending this matrix would exercise these cases end-to-end across all six parametrized test functions that use_AUTH_OVERRIDE_CASES, ensuring the request-helper and public-method forwarding paths properly handle basic auth.Proposed matrix expansion
+import base64 import httpx ... +_BASIC_EXPECTED = "Basic " + base64.b64encode(b"user:pass").decode() _AUTH_OVERRIDE_CASES = [ (httpx.USE_CLIENT_DEFAULT, "Bearer DEFAULT"), # default sentinel keeps the client's bound auth ("Bearer OVERRIDE", "Bearer OVERRIDE"), # raw header value (_StaticAuthorization("Bearer VIA_AUTH"), "Bearer VIA_AUTH"), # httpx.Auth instance + (httpx.BasicAuth("user", "pass"), _BASIC_EXPECTED), # httpx.Auth basic override + (("user", "pass"), _BASIC_EXPECTED), # tuple basic override (None, None), # disable auth for this request (no Authorization header) ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_base_client.py` around lines 277 - 283, The `_AUTH_OVERRIDE_CASES` list is missing test coverage for two supported authentication forms. Extend the `_AUTH_OVERRIDE_CASES` tuple list to include test cases for httpx.BasicAuth instances and (username, password) tuple overrides. For the httpx.BasicAuth case, create an instance with test credentials and include the expected Base64-encoded Authorization header value. For the tuple case, provide a (username, password) tuple and include its expected Basic auth header representation. This will ensure the parametrized test functions that use `_AUTH_OVERRIDE_CASES` exercise these authentication methods end-to-end across the request-helper and public-method forwarding paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flame_hub/_core_client.py`:
- Around line 878-884: The delete request paths at lines 878 and 1202 use
absolute paths starting with "/" (e.g., "/analysis-node-logs"), which is
inconsistent with other HTTP calls in the file that use relative paths (e.g.,
"master-images/command"). Remove the leading "/" from both delete request path
strings in the _client.delete calls to make them consistent with the relative
path style used throughout the rest of the file.
In `@flame_hub/_storage_client.py`:
- Around line 119-123: The stream response handler in the context manager does
not check the response status code before yielding bytes, allowing error
responses (401, 403, etc.) to be returned as file content instead of raising an
error. Add a status code check immediately after the stream context manager is
entered (after the with statement for `self._client.stream()`) to verify the
response has a successful status code (2xx range), and raise an appropriate
exception if the status indicates an error. This same fix needs to be applied to
both occurrences of this pattern, including the one at lines 181-185.
---
Nitpick comments:
In `@flame_hub/_base_client.py`:
- Around line 9-10: Replace the import statement that imports USE_CLIENT_DEFAULT
and UseClientDefault from the private httpx._client module with an import of
USE_CLIENT_DEFAULT from the public httpx module. For the UseClientDefault type
annotation, use type(httpx.USE_CLIENT_DEFAULT) to get the type dynamically, or
alternatively define a local type alias to avoid depending on the unstable
private API. Update all references throughout the file to use the public imports
instead of the private ones.
In `@tests/test_base_client.py`:
- Around line 277-283: The `_AUTH_OVERRIDE_CASES` list is missing test coverage
for two supported authentication forms. Extend the `_AUTH_OVERRIDE_CASES` tuple
list to include test cases for httpx.BasicAuth instances and (username,
password) tuple overrides. For the httpx.BasicAuth case, create an instance with
test credentials and include the expected Base64-encoded Authorization header
value. For the tuple case, provide a (username, password) tuple and include its
expected Basic auth header representation. This will ensure the parametrized
test functions that use `_AUTH_OVERRIDE_CASES` exercise these authentication
methods end-to-end across the request-helper and public-method forwarding paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3637f1ab-429a-4c86-9103-830d151d08aa
📒 Files selected for processing (7)
README.mdflame_hub/_auth_client.pyflame_hub/_base_client.pyflame_hub/_core_client.pyflame_hub/_storage_client.pyflame_hub/types.pytests/test_base_client.py
Stream methods now check the response status and raise HubAPIError before yielding bytes, so an error response (e.g. from auth=None or an invalid per-request override) is no longer returned as file content. Also drop the leading slash from the two direct delete request paths for consistency with the relative paths used elsewhere in the core client. Addresses review feedback on PR #115.
Summary
Adds an optional keyword-only
authparameter to every public client method (AuthClient,CoreClient,StorageClient) so theAuthorizationheader can be overridden for a single request, while all other requests keep the client's bound authentication.authaccepts:httpx.Authinstance (e.g. anotherPasswordAuth/ClientAuth),(username, password)tuple for HTTP basic auth, orAuthorizationheader (e.g."Bearer <token>").Following
httpxconventions:auth(default) → use the client's bound authentication,auth=None→ send the request without any authentication.Implementation notes
flame_hub.types:RequestAuth(override forms) andRequestAuthArg(parameter type incl. theUSE_CLIENT_DEFAULT/Nonesentinels)._StaticAuthorizationhttpx auth flow wraps a raw header string;resolve_request_auth()maps the argument to an httpx-native value (string → wrapped, everything else passed through).authis threaded through theBaseClientrequest helpers and every public method. It is not part ofGetKwargs/FindAllKwargs(it's a transport concern, not a query option).Compatibility
Non-breaking — purely additive. All
authparameters default toUSE_CLIENT_DEFAULT, preserving existing behavior.Testing
httpx.MockTransport, no containers) coveringresolve_request_authand the default / string-override /httpx.Auth-override / disable paths across read, find, create and delete — including the public client surface.ruff format+ruff checkclean;88 passed(non-integration suite).Summary by CodeRabbit
New Features
authparameter to override the client's default authentication for individual requests.Documentation
authparameter, accepted formats, and usage examples.