Skip to content

Commit f621afe

Browse files
committed
fixup! fixup! fixup! fixup! chore: Use SAMIA run artifact file endpoint to download artifacts
1 parent 341d0e9 commit f621afe

File tree

2 files changed

+78
-115
lines changed

2 files changed

+78
-115
lines changed

src/aignostics/platform/resources/runs.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,10 +368,14 @@ def get_artifact_download_url(self, artifact_id: str) -> str:
368368
ServiceException: On 5xx server errors (retried automatically).
369369
RuntimeError: If the redirect Location header is missing.
370370
"""
371-
endpoint_url = f"{self._api.api_client.configuration.host}/v1/runs/{self.run_id}/artifacts/{artifact_id}/file"
371+
endpoint_url = (
372+
f"{self._api.api_client.configuration.host.rstrip('/')}"
373+
f"/v1/runs/{self.run_id}/artifacts/{artifact_id}/file"
374+
)
375+
token_provider = getattr(self._api.api_client.configuration, "token_provider", None) or get_token
372376

373377
def _resolve() -> str:
374-
token = get_token()
378+
token = token_provider()
375379
response = requests.get(
376380
endpoint_url,
377381
headers={"Authorization": f"Bearer {token}", "User-Agent": user_agent()},

tests/aignostics/application/download_test.py

Lines changed: 72 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
"""Tests for download utility functions in the application module."""
22

3+
from collections.abc import Generator
34
from pathlib import Path
4-
from unittest.mock import Mock, patch
5+
from unittest.mock import MagicMock, Mock, patch
56

67
import pytest
78
import requests
89

9-
from aignostics.application._download import download_url_to_file_with_progress, extract_filename_from_url
10+
from aignostics.application._download import (
11+
download_available_items,
12+
download_item_artifact,
13+
download_url_to_file_with_progress,
14+
extract_filename_from_url,
15+
)
1016
from aignostics.application._models import DownloadProgress, DownloadProgressState
1117

1218

@@ -399,168 +405,121 @@ def progress_callback(p: DownloadProgress) -> None:
399405
mock_get.assert_called_once_with(https_url, stream=True, timeout=60)
400406

401407

402-
@pytest.mark.unit
403-
def test_download_available_items_calls_url_resolver(tmp_path: Path) -> None:
404-
"""Test that download_available_items uses the get_artifact_download_url callback.
408+
@pytest.fixture
409+
def patched_item_and_run() -> Generator[tuple[MagicMock, MagicMock], None, None]:
410+
"""Fixture providing patched ItemState/ItemOutput enums and a mock run with one configurable artifact.
405411
406-
Verifies that when a callback is provided, it is called with (run_id, artifact_id)
407-
and the resolved URL is passed to download_item_artifact.
412+
Yields:
413+
tuple[MagicMock, MagicMock]: The mock run and its single mock artifact.
414+
Tests configure artifact attributes (output_artifact_id, download_url, metadata) as needed.
408415
"""
409-
from unittest.mock import MagicMock
410-
411-
from aignostics.application._download import download_available_items
412-
413-
# Build mock artifact
414416
mock_artifact = MagicMock()
415-
mock_artifact.output_artifact_id = "artifact-xyz"
416-
mock_artifact.name = "result"
417-
mock_artifact.metadata = {"checksum_base64_crc32c": "AAAA", "checksum_crc32c": ""}
418-
419-
# Build mock item
420417
mock_item = MagicMock()
421418
mock_item.external_id = "slide-1"
422419
mock_item.state = "TERMINATED"
423420
mock_item.output = "FULL"
424421
mock_item.output_artifacts = [mock_artifact]
425422

426-
# Patch enums for comparison
427423
with (
428424
patch("aignostics.application._download.ItemState") as mock_item_state,
429425
patch("aignostics.application._download.ItemOutput") as mock_item_output,
430426
):
431427
mock_item_state.TERMINATED = "TERMINATED"
432428
mock_item_output.FULL = "FULL"
433429

434-
# Build mock run
435430
mock_run = MagicMock()
436431
mock_run.run_id = "run-123"
437432
mock_run.results.return_value = [mock_item]
438433

439-
progress = DownloadProgress()
440-
downloaded_items: set[str] = set()
441-
resolved_url = "https://storage.googleapis.com/presigned"
442-
mock_url_resolver = MagicMock(return_value=resolved_url)
443-
444-
with patch("aignostics.application._download.download_item_artifact") as mock_download:
445-
download_available_items(
446-
progress=progress,
447-
application_run=mock_run,
448-
destination_directory=tmp_path,
449-
downloaded_items=downloaded_items,
450-
get_artifact_download_url=mock_url_resolver,
451-
)
434+
yield mock_run, mock_artifact
435+
436+
437+
@pytest.mark.unit
438+
def test_download_available_items_calls_url_resolver(
439+
tmp_path: Path, patched_item_and_run: tuple[MagicMock, MagicMock]
440+
) -> None:
441+
"""Test that download_available_items uses the get_artifact_download_url callback.
442+
443+
Verifies that when a callback is provided, it is called with (run_id, artifact_id)
444+
and the resolved URL is passed to download_item_artifact.
445+
"""
446+
mock_run, mock_artifact = patched_item_and_run
447+
mock_artifact.output_artifact_id = "artifact-xyz"
448+
mock_artifact.name = "result"
449+
mock_artifact.metadata = {"checksum_base64_crc32c": "AAAA", "checksum_crc32c": ""}
450+
451+
resolved_url = "https://storage.googleapis.com/presigned"
452+
mock_url_resolver = MagicMock(return_value=resolved_url)
453+
454+
with patch("aignostics.application._download.download_item_artifact") as mock_download:
455+
download_available_items(
456+
progress=DownloadProgress(),
457+
application_run=mock_run,
458+
destination_directory=tmp_path,
459+
downloaded_items=set(),
460+
get_artifact_download_url=mock_url_resolver,
461+
)
452462

453-
mock_url_resolver.assert_called_once_with("run-123", "artifact-xyz")
454-
mock_download.assert_called_once()
455-
# artifact_download_url is the 3rd positional arg
456-
assert mock_download.call_args[0][2] == resolved_url
463+
mock_url_resolver.assert_called_once_with("run-123", "artifact-xyz")
464+
mock_download.assert_called_once()
465+
assert mock_download.call_args[0][2] == resolved_url # artifact_download_url is 3rd positional arg
457466

458467

459468
@pytest.mark.unit
460-
def test_download_available_items_falls_back_to_download_url(tmp_path: Path) -> None:
469+
def test_download_available_items_falls_back_to_download_url(
470+
tmp_path: Path, patched_item_and_run: tuple[MagicMock, MagicMock]
471+
) -> None:
461472
"""Test that download_available_items falls back to artifact.download_url when no callback.
462473
463474
Verifies the deprecated fallback path when get_artifact_download_url is None.
464475
"""
465-
from unittest.mock import MagicMock
466-
467-
from aignostics.application._download import download_available_items
468-
469-
mock_artifact = MagicMock()
476+
mock_run, mock_artifact = patched_item_and_run
470477
mock_artifact.output_artifact_id = "artifact-xyz"
471478
mock_artifact.name = "result"
472479
mock_artifact.download_url = "https://old-signed-url.com/file"
473480
mock_artifact.metadata = {"checksum_base64_crc32c": "AAAA"}
474481

475-
mock_item = MagicMock()
476-
mock_item.external_id = "slide-1"
477-
mock_item.state = "TERMINATED"
478-
mock_item.output = "FULL"
479-
mock_item.output_artifacts = [mock_artifact]
480-
481-
with (
482-
patch("aignostics.application._download.ItemState") as mock_item_state,
483-
patch("aignostics.application._download.ItemOutput") as mock_item_output,
484-
):
485-
mock_item_state.TERMINATED = "TERMINATED"
486-
mock_item_output.FULL = "FULL"
487-
488-
mock_run = MagicMock()
489-
mock_run.run_id = "run-123"
490-
mock_run.results.return_value = [mock_item]
491-
492-
progress = DownloadProgress()
493-
downloaded_items: set[str] = set()
494-
495-
with patch("aignostics.application._download.download_item_artifact") as mock_download:
496-
download_available_items(
497-
progress=progress,
498-
application_run=mock_run,
499-
destination_directory=tmp_path,
500-
downloaded_items=downloaded_items,
501-
get_artifact_download_url=None,
502-
)
482+
with patch("aignostics.application._download.download_item_artifact") as mock_download:
483+
download_available_items(
484+
progress=DownloadProgress(),
485+
application_run=mock_run,
486+
destination_directory=tmp_path,
487+
downloaded_items=set(),
488+
get_artifact_download_url=None,
489+
)
503490

504-
mock_download.assert_called_once()
505-
assert mock_download.call_args[0][2] == "https://old-signed-url.com/file"
491+
mock_download.assert_called_once()
492+
assert mock_download.call_args[0][2] == "https://old-signed-url.com/file"
506493

507494

508495
@pytest.mark.unit
509-
def test_download_available_items_skips_when_no_url_available(tmp_path: Path) -> None:
496+
def test_download_available_items_skips_when_no_url_available(
497+
tmp_path: Path, patched_item_and_run: tuple[MagicMock, MagicMock]
498+
) -> None:
510499
"""Test that download_available_items skips artifacts with no URL available.
511500
512501
When get_artifact_download_url is None and artifact.download_url is also None,
513502
the artifact should be skipped.
514503
"""
515-
from unittest.mock import MagicMock
516-
517-
from aignostics.application._download import download_available_items
518-
519-
mock_artifact = MagicMock()
504+
mock_run, mock_artifact = patched_item_and_run
520505
mock_artifact.output_artifact_id = None
521-
mock_artifact.name = "result"
522506
mock_artifact.download_url = None
523-
mock_artifact.metadata = {"checksum_base64_crc32c": "AAAA"}
524-
525-
mock_item = MagicMock()
526-
mock_item.external_id = "slide-1"
527-
mock_item.state = "TERMINATED"
528-
mock_item.output = "FULL"
529-
mock_item.output_artifacts = [mock_artifact]
530-
531-
with (
532-
patch("aignostics.application._download.ItemState") as mock_item_state,
533-
patch("aignostics.application._download.ItemOutput") as mock_item_output,
534-
):
535-
mock_item_state.TERMINATED = "TERMINATED"
536-
mock_item_output.FULL = "FULL"
537507

538-
mock_run = MagicMock()
539-
mock_run.run_id = "run-123"
540-
mock_run.results.return_value = [mock_item]
541-
542-
progress = DownloadProgress()
543-
downloaded_items: set[str] = set()
544-
545-
with patch("aignostics.application._download.download_item_artifact") as mock_download:
546-
download_available_items(
547-
progress=progress,
548-
application_run=mock_run,
549-
destination_directory=tmp_path,
550-
downloaded_items=downloaded_items,
551-
get_artifact_download_url=None,
552-
)
508+
with patch("aignostics.application._download.download_item_artifact") as mock_download:
509+
download_available_items(
510+
progress=DownloadProgress(),
511+
application_run=mock_run,
512+
destination_directory=tmp_path,
513+
downloaded_items=set(),
514+
get_artifact_download_url=None,
515+
)
553516

554-
mock_download.assert_not_called()
517+
mock_download.assert_not_called()
555518

556519

557520
@pytest.mark.unit
558521
def test_download_item_artifact_uses_provided_url(tmp_path: Path) -> None:
559522
"""Test that download_item_artifact uses the explicitly provided artifact_download_url."""
560-
from unittest.mock import MagicMock
561-
562-
from aignostics.application._download import download_item_artifact
563-
564523
mock_artifact = MagicMock()
565524
mock_artifact.name = "cell_classification"
566525
mock_artifact.metadata = {"checksum_base64_crc32c": "AAAA"}

0 commit comments

Comments
 (0)