fixup! fixup! fixup! fixup! chore: Use SAMIA run artifact file endpoint to download artifacts

Author: nahua-aignx

src/aignostics/application/_download.py

@@ -146,6 +146,44 @@ def update_progress(
         download_progress_queue.put_nowait(progress)
 
 
+def _resolve_artifact_url(
+    artifact: Any,  # noqa: ANN401
+    run_id: str,
+    get_artifact_download_url: Callable[[str, str], str] | None,
+) -> str | None:
+    """Resolve the download URL for an artifact.
+
+    Tries the new file endpoint first (via get_artifact_download_url), then falls back to
+    the deprecated download_url field on the artifact if the endpoint fails.
+
+    Args:
+        artifact: The artifact object with optional output_artifact_id and download_url.
+        run_id: The run ID, passed to get_artifact_download_url.
+        get_artifact_download_url: Callable for the new endpoint, or None to skip.
+
+    Returns:
+        str | None: The resolved download URL, or None if unavailable.
+
+    Raises:
+        Exception: Re-raises if the new endpoint fails and no fallback download_url exists.
+    """
+    if get_artifact_download_url and artifact.output_artifact_id:
+        try:
+            return get_artifact_download_url(run_id, artifact.output_artifact_id)
+        except Exception as e:
+            fallback_url: str | None = getattr(artifact, "download_url", None)
+            if fallback_url:
+                logger.warning(
+                    "Failed to resolve download URL via file endpoint for artifact {} ({}). "
+                    "Falling back to deprecated download_url field.",
+                    artifact.output_artifact_id,
+                    e,
+                )
+                return fallback_url
+            raise
+    return getattr(artifact, "download_url", None)
+
+
 def download_available_items(  # noqa: PLR0913, PLR0917
     progress: DownloadProgress,
     application_run: Run,
@@ -205,12 +243,9 @@ def download_available_items(  # noqa: PLR0913, PLR0917
                 progress.artifact = artifact
                 update_progress(progress, download_progress_callable, download_progress_queue)
 
-                # Resolve artifact download URL via the new file endpoint
-                if get_artifact_download_url and artifact.output_artifact_id:
-                    artifact_url = get_artifact_download_url(application_run.run_id, artifact.output_artifact_id)
-                elif download_url := getattr(artifact, "download_url", None):
-                    artifact_url = download_url  # Deprecated fallback
-                else:
+                # Resolve artifact download URL via the new file endpoint, with fallback
+                artifact_url = _resolve_artifact_url(artifact, application_run.run_id, get_artifact_download_url)
+                if not artifact_url:
                     logger.warning("No download URL available for artifact {}", artifact.output_artifact_id)
                     continue
 

src/aignostics/application/_gui/_page_application_run_describe.py

@@ -339,9 +339,9 @@ async def start_download() -> None:
 
             # Activate the timer now that download is starting
             progress_timer.activate()
+            download_button.disable()
+            download_button.props(add="loading")
             try:
-                download_button.disable()
-                download_button.props(add="loading")
                 results_folder = await nicegui_run.cpu_bound(
                     Service.application_run_download_static,
                     run_id=run.run_id,
@@ -365,19 +365,24 @@ async def start_download() -> None:
                 else:
                     ui.notify("Download completed.", type="positive")
                 show_in_file_manager(str(results_folder))
-            except ValueError as e:
+            except Exception as e:
+                logger.exception(
+                    "Download failed for run {} (qupath_project={}, marimo={}, folder={})",
+                    run.run_id,
+                    current_qupath_project,
+                    current_marimo,
+                    current_folder,
+                )
                 ui.notify(f"Download failed: {e}", type="negative", multi_line=True)
+            finally:
                 progress_timer.deactivate()
                 progress_state["queue"] = None
-                return
-            progress_timer.deactivate()
-            progress_state["queue"] = None
-            download_button.props(remove="loading")
-            download_button.enable()
-            download_item_status.set_visibility(False)
-            download_item_progress.set_visibility(False)
-            download_artifact_status.set_visibility(False)
-            download_artifact_progress.set_visibility(False)
+                download_button.props(remove="loading")
+                download_button.enable()
+                download_item_status.set_visibility(False)
+                download_item_progress.set_visibility(False)
+                download_artifact_status.set_visibility(False)
+                download_artifact_progress.set_visibility(False)
 
         ui.separator()
         with ui.row(align_items="end").classes("w-full justify-end"):
@@ -840,7 +845,7 @@ async def _preview_csv(
 
                                             preview_button.on_click(_preview_csv)
 
-                                        download_button = ui.button(
+                                        artifact_dl_button = ui.button(
                                             text="Download",
                                             icon="cloud_download",
                                         )
@@ -849,7 +854,7 @@ async def _download_artifact(
                                             _: ClickEventArguments,
                                             aid: str = artifact_id,
                                             _run: Run = run,
-                                            _btn: ui.button = download_button,
+                                            _btn: ui.button = artifact_dl_button,
                                         ) -> None:
                                             try:
                                                 _btn.props(add="loading")
@@ -860,7 +865,7 @@ async def _download_artifact(
                                             finally:
                                                 _btn.props(remove="loading")
 
-                                        download_button.on_click(_download_artifact)
+                                        artifact_dl_button.on_click(_download_artifact)
                                         if metadata:
                                             ui.button(
                                                 text="Schema",

src/aignostics/platform/resources/runs.py

@@ -15,7 +15,7 @@
 
 import requests
 from aignx.codegen.api.public_api import PublicApi
-from aignx.codegen.exceptions import NotFoundException, ServiceException
+from aignx.codegen.exceptions import ApiException, NotFoundException, ServiceException
 from aignx.codegen.models import (
     CustomMetadataUpdateRequest,
     ItemCreationRequest,
@@ -348,10 +348,72 @@ def download_to_folder(  # noqa: C901
             msg = f"Download operation failed unexpectedly for run {self.run_id}: {e}"
             raise RuntimeError(msg) from e
 
+    @staticmethod
+    def _fetch_artifact_redirect_url(
+        endpoint_url: str,
+        token: str,
+        proxy: str | None,
+        ssl_verify: bool | str,
+        artifact_id: str,
+    ) -> str:
+        """Execute the HTTP request to the artifact file endpoint and return the redirect URL.
+
+        Args:
+            endpoint_url (str): Full URL of the artifact file endpoint.
+            token (str): Bearer token for authorization.
+            proxy (str | None): Optional proxy URL.
+            ssl_verify (bool | str): SSL verification setting (True/False or CA bundle path).
+            artifact_id (str): Artifact ID used in error messages.
+
+        Returns:
+            str: The presigned URL from the redirect Location header.
+
+        Raises:
+            NotFoundException: If the artifact is not found (404).
+            ServiceException: On network errors or 5xx responses.
+            ApiException: On 4xx client errors.
+            RuntimeError: If the redirect has no Location header or returns an unexpected status.
+        """
+        try:
+            response = requests.get(
+                endpoint_url,
+                headers={"Authorization": f"Bearer {token}", "User-Agent": user_agent()},
+                allow_redirects=False,
+                timeout=settings().run_timeout,
+                proxies={"http": proxy, "https": proxy} if proxy else None,
+                verify=ssl_verify,
+            )
+        except requests.Timeout as e:
+            raise ServiceException(status=HTTPStatus.SERVICE_UNAVAILABLE, reason="Request timed out") from e
+        except requests.ConnectionError as e:
+            raise ServiceException(status=HTTPStatus.SERVICE_UNAVAILABLE, reason="Connection failed") from e
+        except requests.RequestException as e:
+            raise ServiceException(status=HTTPStatus.SERVICE_UNAVAILABLE, reason=f"Request failed: {e}") from e
+
+        if response.status_code in {
+            HTTPStatus.MOVED_PERMANENTLY,
+            HTTPStatus.FOUND,
+            HTTPStatus.TEMPORARY_REDIRECT,
+            HTTPStatus.PERMANENT_REDIRECT,
+        }:
+            location = response.headers.get("Location")
+            if not location:
+                msg = f"Redirect response {response.status_code} missing Location header for artifact {artifact_id}"
+                raise RuntimeError(msg)
+            return location
+        if response.status_code == HTTPStatus.NOT_FOUND:
+            raise NotFoundException(status=HTTPStatus.NOT_FOUND, reason="Artifact not found")
+        if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR:
+            raise ServiceException(status=response.status_code, reason=response.reason)
+        if response.status_code >= HTTPStatus.BAD_REQUEST:
+            raise ApiException(status=response.status_code, reason=response.reason)
+        msg = f"Unexpected status {response.status_code} from artifact file endpoint for artifact {artifact_id}"
+        raise RuntimeError(msg)
+
     def get_artifact_download_url(self, artifact_id: str) -> str:
         """Get a presigned download URL for an artifact via the file endpoint.
 
-        Calls GET /v1/runs/{run_id}/artifacts/{artifact_id}/file with
+        Calls GET /api/v1/runs/{run_id}/artifacts/{artifact_id}/file with
         allow_redirects=False and extracts the presigned URL from the Location
         header of the 307 redirect response. Using the codegen method is not
         viable here because urllib3 follows the redirect automatically, fetching
@@ -365,45 +427,28 @@ def get_artifact_download_url(self, artifact_id: str) -> str:
 
         Raises:
             NotFoundException: If the artifact is not found (404).
-            ServiceException: On 5xx server errors (retried automatically).
+            ServiceException: On 5xx or other unexpected HTTP errors (retried automatically where transient).
             RuntimeError: If the redirect Location header is missing.
         """
-        endpoint_url = f"{self._api.api_client.configuration.host}/v1/runs/{self.run_id}/artifacts/{artifact_id}/file"
-
-        def _resolve() -> str:
-            token = get_token()
-            response = requests.get(
-                endpoint_url,
-                headers={"Authorization": f"Bearer {token}", "User-Agent": user_agent()},
-                allow_redirects=False,
-                timeout=settings().run_timeout,
-            )
-            if response.status_code in {
-                HTTPStatus.MOVED_PERMANENTLY,
-                HTTPStatus.FOUND,
-                HTTPStatus.TEMPORARY_REDIRECT,
-                HTTPStatus.PERMANENT_REDIRECT,
-            }:
-                location = response.headers.get("Location")
-                if not location:
-                    msg = f"Redirect response {response.status_code} missing Location header for artifact {artifact_id}"
-                    raise RuntimeError(msg)
-                return location
-            if response.status_code == HTTPStatus.NOT_FOUND:
-                raise NotFoundException(status=HTTPStatus.NOT_FOUND, reason="Artifact not found")
-            if response.status_code >= HTTPStatus.INTERNAL_SERVER_ERROR:
-                raise ServiceException(status=response.status_code, reason=response.reason)
-            response.raise_for_status()
-            msg = f"Unexpected status {response.status_code} from artifact file endpoint for artifact {artifact_id}"
-            raise RuntimeError(msg)
-
-        return Retrying(
+        configuration = self._api.api_client.configuration
+        host = configuration.host.rstrip("/")
+        endpoint_url = f"{host}/api/v1/runs/{self.run_id}/artifacts/{artifact_id}/file"
+        token_provider = getattr(configuration, "token_provider", None) or get_token
+        proxy = getattr(configuration, "proxy", None)
+        ssl_ca_cert = getattr(configuration, "ssl_ca_cert", None)
+        verify_ssl = getattr(configuration, "verify_ssl", True)
+        ssl_verify: bool | str = ssl_ca_cert or verify_ssl
+        logger.trace("Resolving download URL for artifact {} via {}", artifact_id, endpoint_url)
+
+        url = Retrying(
             retry=retry_if_exception_type(exception_types=RETRYABLE_EXCEPTIONS),
             stop=stop_after_attempt(settings().run_retry_attempts),
             wait=wait_exponential_jitter(initial=settings().run_retry_wait_min, max=settings().run_retry_wait_max),
             before_sleep=_log_retry_attempt,
             reraise=True,
-        )(_resolve)
+        )(lambda: self._fetch_artifact_redirect_url(endpoint_url, token_provider(), proxy, ssl_verify, artifact_id))
+        logger.trace("Resolved download URL for artifact {}", artifact_id)
+        return url
 
     def ensure_artifacts_downloaded(
         self,
@@ -431,6 +476,13 @@ def ensure_artifacts_downloaded(
         downloaded_at_least_one_artifact = False
         for artifact in item.output_artifacts:
             if not artifact.output_artifact_id:
+                logger.warning(
+                    "Skipping artifact {} for item {}: missing output_artifact_id", artifact.name, item.external_id
+                )
+                if print_status:
+                    print(
+                        f"> Skipping artifact {artifact.name} for item {item.external_id}: missing output_artifact_id"
+                    )
                 continue
             item_dir.mkdir(exist_ok=True, parents=True)
             file_ending = mime_type_to_file_ending(get_mime_type_for_artifact(artifact))