-
Notifications
You must be signed in to change notification settings - Fork 20
fix(auditor): connect aggregated artifacts download #1141
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
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
57 changes: 57 additions & 0 deletions
57
plugins/nemo-auditor/src/nemo_auditor/jobs/artifacts_route.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """On-the-fly aggregate artifact download handler for audit jobs.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import shutil | ||
| import tarfile | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
||
| from fastapi import BackgroundTasks, Depends, HTTPException, Response | ||
| from fastapi.responses import FileResponse | ||
| from nemo_platform import AsyncNeMoPlatform | ||
| from nemo_platform_plugin.client.adapter import client_from_platform | ||
| from nemo_platform_plugin.dependencies import get_sdk_client | ||
| from nemo_platform_plugin.jobs.client import AsyncJobsClient | ||
| from nemo_platform_plugin.jobs.result_manager import download_from_result_info | ||
|
|
||
| from nemo_auditor.jobs.audit import GARAK_RESULT_NAMES | ||
|
|
||
|
|
||
| async def aggregate_artifacts_download( | ||
| workspace: str, | ||
| job: str, | ||
| background_tasks: BackgroundTasks, | ||
| sdk: AsyncNeMoPlatform = Depends(get_sdk_client), | ||
| ) -> Response: | ||
| """Download all available Garak report results and return them as a single tar.gz.""" | ||
| jobs_client = client_from_platform(sdk, AsyncJobsClient) | ||
| all_results = (await jobs_client.list_job_results(name=job, workspace=workspace)).data() | ||
| relevant = [r for r in all_results.data if r.name in GARAK_RESULT_NAMES] | ||
|
|
||
| if not relevant: | ||
| raise HTTPException(status_code=404, detail="No artifact results found for this audit job") | ||
|
|
||
| tmp_dirs = [] | ||
| for result in relevant: | ||
| filename, tmp_dir_path = await download_from_result_info( | ||
| result_name=result.name, | ||
| job_name=job, | ||
| workspace=workspace, | ||
| artifact_url=result.artifact_url, | ||
| files_sdk=sdk, | ||
| ) | ||
| tmp_dirs.append((filename, tmp_dir_path)) | ||
| background_tasks.add_task(tmp_dir_path.cleanup_tmp_dir) | ||
|
|
||
| agg_tmp = Path(tempfile.mkdtemp()) | ||
| tar_path = agg_tmp / "artifacts.tar.gz" | ||
| with tarfile.open(tar_path, "w:gz") as tar: | ||
| for filename, tmp_dir_path in tmp_dirs: | ||
| tar.add(tmp_dir_path.path, arcname=filename) | ||
| background_tasks.add_task(lambda: shutil.rmtree(agg_tmp, ignore_errors=True)) | ||
|
|
||
| return FileResponse(path=tar_path, media_type="application/gzip", filename="artifacts.tar.gz") | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Unit tests for the on-the-fly aggregate artifacts download handler.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import io | ||
| import tarfile | ||
| import tempfile | ||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
| from typing import Any | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| import pytest | ||
| from fastapi import HTTPException | ||
| from fastapi.responses import FileResponse | ||
| from nemo_auditor.jobs.artifacts_route import aggregate_artifacts_download | ||
| from nemo_platform_plugin.jobs.file_manager import TmpDirPath | ||
|
|
||
|
|
||
| def _make_result(name: str, artifact_url: str = "file:///fake") -> MagicMock: | ||
| r = MagicMock() | ||
| r.name = name | ||
| r.artifact_url = artifact_url | ||
| return r | ||
|
|
||
|
|
||
| def _make_results_page(*names: str) -> MagicMock: | ||
| page = MagicMock() | ||
| page.data = [_make_result(n) for n in names] | ||
| return page | ||
|
|
||
|
|
||
| def _make_tmp_file(tmp_path: Path, name: str, content: bytes = b"data") -> TmpDirPath: | ||
| tmp_dir = Path(tempfile.mkdtemp(dir=tmp_path)) | ||
| file_path = tmp_dir / name | ||
| file_path.write_bytes(content) | ||
| return TmpDirPath(path=file_path, tmp_dir=tmp_dir) | ||
|
|
||
|
|
||
| @dataclass | ||
| class _FakeSdk: | ||
| pass | ||
|
|
||
|
|
||
| def _make_jobs_client_mock(results_page: Any) -> MagicMock: | ||
| client = MagicMock() | ||
| list_resp = MagicMock() | ||
| list_resp.data.return_value = results_page | ||
| client.list_job_results = AsyncMock(return_value=list_resp) | ||
| return client | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| class TestAggregateArtifactsDownload: | ||
| async def test_happy_path_returns_tar_with_all_results(self, tmp_path: Path) -> None: | ||
| results = _make_results_page("report-html", "report-jsonl") | ||
| html_tmp = _make_tmp_file(tmp_path, "report-html", b"<html/>") | ||
| jsonl_tmp = _make_tmp_file(tmp_path, "report-jsonl", b'{"probe":"x"}') | ||
|
|
||
| download_side_effects = [ | ||
| ("report-html", html_tmp), | ||
| ("report-jsonl", jsonl_tmp), | ||
| ] | ||
|
|
||
| background_tasks = MagicMock() | ||
| sdk = _FakeSdk() | ||
|
|
||
| with ( | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.client_from_platform", | ||
| return_value=_make_jobs_client_mock(results), | ||
| ), | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.download_from_result_info", | ||
| new=AsyncMock(side_effect=download_side_effects), | ||
| ), | ||
| ): | ||
| response = await aggregate_artifacts_download( | ||
| workspace="default", | ||
| job="audit-job-123", | ||
| background_tasks=background_tasks, | ||
| sdk=sdk, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| assert isinstance(response, FileResponse) | ||
| tar_path = Path(response.path) | ||
| assert tar_path.exists() | ||
| with tarfile.open(tar_path, "r:gz") as tar: | ||
| members = {m.name for m in tar.getmembers()} | ||
| assert "report-html" in members | ||
| assert "report-jsonl" in members | ||
| # cleanup scheduled for both individual tmp dirs + aggregate dir | ||
| assert background_tasks.add_task.call_count == 3 | ||
|
|
||
| async def test_partial_results_skips_missing(self, tmp_path: Path) -> None: | ||
| # Only report-html present; report-jsonl and report-hitlog-jsonl absent | ||
| results = _make_results_page("report-html") | ||
| html_tmp = _make_tmp_file(tmp_path, "report-html", b"<html/>") | ||
|
|
||
| background_tasks = MagicMock() | ||
| sdk = _FakeSdk() | ||
|
|
||
| with ( | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.client_from_platform", | ||
| return_value=_make_jobs_client_mock(results), | ||
| ), | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.download_from_result_info", | ||
| new=AsyncMock(return_value=("report-html", html_tmp)), | ||
| ), | ||
| ): | ||
| response = await aggregate_artifacts_download( | ||
| workspace="default", | ||
| job="audit-job-456", | ||
| background_tasks=background_tasks, | ||
| sdk=sdk, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| assert isinstance(response, FileResponse) | ||
| with tarfile.open(Path(response.path), "r:gz") as tar: | ||
| members = {m.name for m in tar.getmembers()} | ||
| assert members == {"report-html"} | ||
|
|
||
| async def test_no_relevant_results_raises_404(self) -> None: | ||
| # Job has a result, but not one of the known Garak names | ||
| results = _make_results_page("some-other-result") | ||
|
|
||
| background_tasks = MagicMock() | ||
| sdk = _FakeSdk() | ||
|
|
||
| with ( | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.client_from_platform", | ||
| return_value=_make_jobs_client_mock(results), | ||
| ), | ||
| ): | ||
| with pytest.raises(HTTPException) as exc_info: | ||
| await aggregate_artifacts_download( | ||
| workspace="default", | ||
| job="audit-job-789", | ||
| background_tasks=background_tasks, | ||
| sdk=sdk, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| assert exc_info.value.status_code == 404 | ||
|
|
||
| async def test_empty_results_raises_404(self) -> None: | ||
| results = _make_results_page() | ||
|
|
||
| background_tasks = MagicMock() | ||
| sdk = _FakeSdk() | ||
|
|
||
| with ( | ||
| patch( | ||
| "nemo_auditor.jobs.artifacts_route.client_from_platform", | ||
| return_value=_make_jobs_client_mock(results), | ||
| ), | ||
| ): | ||
| with pytest.raises(HTTPException) as exc_info: | ||
| await aggregate_artifacts_download( | ||
| workspace="default", | ||
| job="audit-job-000", | ||
| background_tasks=background_tasks, | ||
| sdk=sdk, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| assert exc_info.value.status_code == 404 |
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
Oops, something went wrong.
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.
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.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Clean up temporary directories on failure.
If
download_from_result_infoortar.addraises, the endpoint returns noFileResponse. FastAPI does not run these scheduled background tasks. The successful downloads then leak temporary directories.Use
try/exceptto clean accumulated result directories andagg_tmpsynchronously before re-raising. Schedule background cleanup only after archive creation succeeds. Add a failed-download test.🤖 Prompt for AI Agents