fix(auditor): add aggregated auditor artifacts endpoint - #1190
Merged
Conversation
parkanzky
force-pushed
the
nvbug6562262-agg-artifacts
branch
from
August 7, 2026 21:55
9663c74 to
44f1126
Compare
Contributor
|
Caution CodeRabbit couldn't update its existing comment. The review summary may be out of date. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@plugins/nemo-auditor/openapi/openapi.yaml`:
- Around line 337-347: Update the response definition near the existing 200
response to document the archive as application/gzip with a binary schema
instead of application/json, and add the endpoint’s 404 no-artifacts response
while preserving the existing 422 validation response. Regenerate the OpenAPI
specification using the project’s documented refresh command so the generated
file remains synchronized with the API definitions.
In `@plugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.py`:
- Around line 76-80: Move the blocking archive creation currently inside the
async artifact route into a worker thread, including tarfile.open and each
tar.add operation. Keep the existing tar_path and artifact_tmps inputs and
ensure the route awaits the offloaded operation without performing synchronous
disk I/O on the event loop.
🪄 Autofix
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: dd8fca0f-d799-44c1-8400-b9fde3f06322
📒 Files selected for processing (5)
plugins/nemo-auditor/openapi/openapi.yamlplugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.pyplugins/nemo-auditor/src/nemo_auditor/jobs/audit.pyplugins/nemo-auditor/src/nemo_auditor/service.pyplugins/nemo-auditor/tests/test_api_artifacts.py
💤 Files with no reviewable changes (1)
- plugins/nemo-auditor/src/nemo_auditor/jobs/audit.py
Contributor
|
Caution CodeRabbit couldn't update its existing comment. The review summary may be out of date. Error details |
Contributor
Author
|
/nvskills-ci |
Contributor
📝 WalkthroughWalkthroughThe PR adds a workspace-scoped endpoint that retrieves available audit report artifacts, packages them into a gzip-compressed tar archive, streams the archive, and removes temporary files. OpenAPI documentation and endpoint tests are included. ChangesAuditor artifact download
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuditorRouter
participant download_audit_artifacts
participant AsyncNeMoPlatform
participant ResultManager
Client->>AuditorRouter: Request workspace/job artifact archive
AuditorRouter->>download_audit_artifacts: Route request
download_audit_artifacts->>AsyncNeMoPlatform: Query configured audit results
AsyncNeMoPlatform-->>download_audit_artifacts: Result metadata
download_audit_artifacts->>ResultManager: Download available artifacts
ResultManager-->>download_audit_artifacts: Artifact files
download_audit_artifacts-->>Client: Stream artifacts.tar.gz or return 404
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
/nvskills-ci |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.py (1)
96-107: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftGuarantee temporary-directory cleanup on cancellation and failed sends.
except Exceptiondoes not catch cancellation on supported Python versions. Starlette 1.3.1 runs background tasks only afterFileResponsecompletes allsendcalls. A cancellation or failed send can therefore leave temporary directories allocated. Use one idempotent cleanup path in a response/streamfinallyblock, and add tests for cancellation and interrupted sends.🤖 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 `@plugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.py` around lines 96 - 107, Update the artifact response flow surrounding _cleanup and FileResponse so temporary directories are released from a single idempotent cleanup path in a response/stream finally block, including cancellation and failed sends that bypass except Exception. Preserve successful-response cleanup and ensure cleanup cannot run destructively more than once; add coverage for cancellation and interrupted sends.
🤖 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.
Outside diff comments:
In `@plugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.py`:
- Around line 96-107: Update the artifact response flow surrounding _cleanup and
FileResponse so temporary directories are released from a single idempotent
cleanup path in a response/stream finally block, including cancellation and
failed sends that bypass except Exception. Preserve successful-response cleanup
and ensure cleanup cannot run destructively more than once; add coverage for
cancellation and interrupted sends.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 454799c7-1234-41b6-850a-dbcc7bbfb94c
📒 Files selected for processing (1)
plugins/nemo-auditor/src/nemo_auditor/api/v2/artifacts.py
Contributor
|
Contributor
Author
|
/nvskills-ci |
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
parkanzky
force-pushed
the
nvbug6562262-agg-artifacts
branch
from
August 7, 2026 22:56
75a0974 to
ed601ea
Compare
parkanzky
enabled auto-merge
August 7, 2026 22:57
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
2 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
Connect the auditor artifacts download endpoint to tarball.
Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowSummary by CodeRabbit
New Features
.tar.gzarchive.404response when no artifacts are available.Documentation