Repository navigation
chore: add hf retries to files - #322
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three ChangesHuggingFace Transient Retry Handling
Possibly Related PRs
🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/core/files/src/nmp/core/files/app/backends/huggingface.py (1)
178-192: 📐 Maintainability & Code Quality | 💤 Low valueLine 192 is unreachable.
The loop always either returns on line 181, raises on line 183, or raises on line 190. This
raisecan never execute. Harmless defensive code, but could be removed or replaced with an assertion to clarify intent.🤖 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 `@services/core/files/src/nmp/core/files/app/backends/huggingface.py` around lines 178 - 192, The final raise in _run_hf_request is unreachable because the loop always returns or raises; remove the trailing "raise HuggingfaceBackendError(...)" and replace it with an explicit assertion to document intent (e.g., assert False or raise AssertionError("unreachable in _run_hf_request")). Update surrounding references to _HF_TRANSIENT_RETRY_ATTEMPTS, EntryNotFoundError, HfHubHTTPError, _map_hf_http_error, HuggingfaceUnavailableError, and _sleep_before_retry as needed to ensure control flow is still clear.
🤖 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.
Nitpick comments:
In `@services/core/files/src/nmp/core/files/app/backends/huggingface.py`:
- Around line 178-192: The final raise in _run_hf_request is unreachable because
the loop always returns or raises; remove the trailing "raise
HuggingfaceBackendError(...)" and replace it with an explicit assertion to
document intent (e.g., assert False or raise AssertionError("unreachable in
_run_hf_request")). Update surrounding references to
_HF_TRANSIENT_RETRY_ATTEMPTS, EntryNotFoundError, HfHubHTTPError,
_map_hf_http_error, HuggingfaceUnavailableError, and _sleep_before_retry as
needed to ensure control flow is still clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 78b3df66-4bfd-4a24-ba49-ef418aa09363
📒 Files selected for processing (2)
services/core/files/src/nmp/core/files/app/backends/huggingface.pyservices/core/files/tests/test_huggingface_backend.py
Contributor
|
mckornfield
force-pushed
the
hf-retry-attempts/mck
branch
from
June 16, 2026 13:52
d919c31 to
488ae5b
Compare
Contributor
|
🌿 Preview your docs: https://nvidia-preview-hf-retry-attempts-mck.docs.buildwithfern.com/nemo-platform |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 53-58: The entrypoint field is currently defined as a free-form
string type with only examples provided, which does not enforce the valid values
and weakens client generation and validation. Replace the type declaration for
the entrypoint field with an enum constraint that explicitly lists the three
allowed values: allow, has_permissions, and has_role. This ensures the schema
enforces the contract at the definition level rather than relying only on
documentation examples.
In `@openapi/ga/openapi.yaml`:
- Around line 49-59: The entrypoint parameter in the OpenAPI schema currently
accepts any string value, but the documentation and examples show only three
valid values: allow, has_permissions, and has_role. Add an enum constraint to
the schema field of the entrypoint parameter to restrict it to these three
documented values, ensuring that generated clients will properly validate the
input according to the API contract.
In `@openapi/openapi.yaml`:
- Around line 52-58: The `entrypoint` parameter is currently defined as a string
type with examples, but examples in OpenAPI do not constrain valid values -
generated clients will still permit any string. Replace the `examples` field in
the entrypoint schema with an `enum` field containing the allowed values (allow,
has_permissions, has_role) to properly enforce the contract that only these
specific values are valid.
- Around line 67-87: The OpenAPI documentation for the authz endpoint is missing
documented responses for 401 (Unauthorized) and 403 (Forbidden) status codes
that the middleware returns for non-service principals, and it does not account
for the fact that FastAPI HTTPException wraps error response bodies under a
`detail` field. Update the responses block in the authz endpoint to include 401
and 403 response definitions, and modify the schemas for the 400 and 500
responses to reflect that their bodies are wrapped in the `detail` envelope as
produced by the actual FastAPI exception handling behavior observed in the
middleware and endpoint implementations.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 775814b6-c871-4eff-9ae6-fff68d0ab4d0
⛔ Files ignored due to path filters (1)
sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (10)
.github/workflows/ci.yamldocs/set-up/config-reference.mdxopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/files/src/nmp/core/files/app/backends/huggingface.pyservices/core/files/src/nmp/core/files/config.pyservices/core/files/tests/test_files_config.pyservices/core/files/tests/test_huggingface_backend.py
💤 Files with no reviewable changes (5)
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- services/core/files/src/nmp/core/files/config.py
- services/core/files/tests/test_files_config.py
- services/core/files/tests/test_huggingface_backend.py
- services/core/files/src/nmp/core/files/app/backends/huggingface.py
mckornfield
force-pushed
the
hf-retry-attempts/mck
branch
from
June 16, 2026 19:57
488ae5b to
fbac457
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@services/core/auth/src/nmp/core/auth/assets/static-authz.yaml`:
- Line 324: The permission key `auth.authz.create` is being granted in the
`Editor` role but is not declared in the `authz.permissions` registry section of
the YAML file. Add a declaration for the `auth.authz.create` permission in the
`authz.permissions` section to ensure consistency with the policy's registry
contract and prevent evaluation-time issues. The new permission entry should
follow the same format and structure as existing permission declarations in that
section.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e045c8f7-a131-4ecd-b839-e09670bbcb9a
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/files/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/files/filesets.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/files/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/files/fileset.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/files/fileset_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/files/fileset_metadata_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/fileset_metadata.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared_params/fileset_metadata_param.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (7)
.github/workflows/ci.yamldocs/set-up/config-reference.mdxservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/files/src/nmp/core/files/app/backends/huggingface.pyservices/core/files/src/nmp/core/files/config.pyservices/core/files/tests/test_files_config.pyservices/core/files/tests/test_huggingface_backend.py
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/set-up/config-reference.mdx
- services/core/files/tests/test_files_config.py
- .github/workflows/ci.yaml
- services/core/files/tests/test_huggingface_backend.py
mckornfield
force-pushed
the
hf-retry-attempts/mck
branch
3 times, most recently
from
June 26, 2026 21:53
64982a7 to
7be2172
Compare
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
mckornfield
force-pushed
the
hf-retry-attempts/mck
branch
from
June 29, 2026 21:18
7be2172 to
28a5be6
Compare
matthewgrossman
approved these changes
Jun 30, 2026
matthewgrossman
left a comment
matthewgrossman
left a comment
Member
There was a problem hiding this comment.
couple comments you could throw at the agent and see what it spits out ¯_(ツ)_/¯
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
svvarom
self-requested a review
June 30, 2026 20:40
a2bondar
approved these changes
Jun 30, 2026
svvarom
reviewed
Jun 30, 2026
svvarom
approved these changes
Jun 30, 2026
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
mckornfield
enabled auto-merge
July 1, 2026 16:43
arpitsardhana
pushed a commit
that referenced
this pull request
Jul 9, 2026
* chore: add hf retries to files Signed-off-by: Matt Kornfield <mkornfield@nvidia.com> * chore: flaky test patch Signed-off-by: Matt Kornfield <mkornfield@nvidia.com> * chore: improve flow of huggingface retries Signed-off-by: Matt Kornfield <mkornfield@nvidia.com> * chore: attempt to use retry headers Signed-off-by: Matt Kornfield <mkornfield@nvidia.com> --------- Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
hf_retry_attempts,hf_retry_initial_delay_seconds, andhf_retry_max_delay_seconds.Improvements
Documentation