diff --git a/backend/src/ref_backend/api/routes/executions.py b/backend/src/ref_backend/api/routes/executions.py index df6d197c..ac37505c 100644 --- a/backend/src/ref_backend/api/routes/executions.py +++ b/backend/src/ref_backend/api/routes/executions.py @@ -135,13 +135,10 @@ def list_recent_execution_groups( # noqa: PLR0913, PLR0917 limit=limit, include_outputs=include_outputs, ) - data = [] - for eg in groups: - try: - data.append(ExecutionGroup.build(eg, app_context, include_outputs=include_outputs)) - except Exception as e: - logger.error(f"Error building execution group ID {eg.id}: {e}") - return Collection(total_count=total_count, data=data) + return Collection( + total_count=total_count, + data=[ExecutionGroup.build(eg, app_context, include_outputs=include_outputs) for eg in groups], + ) @router.get("/{group_id}") @@ -258,10 +255,10 @@ def metric_bundle( execution = _get_execution(group_id, execution_id, app_context) file_path = resolve_artifact( - app_context.reader.artifacts.output_file, execution.output_fragment, "diagnostic.json" + app_context.reader.artifacts.bundle, execution.output_fragment, execution.path ) - if not file_path.exists(): + if file_path is None or not file_path.exists(): logger.warning(f"Metric bundle not found: {file_path}") raise HTTPException(status_code=404, detail="Metrics bundle not found") diff --git a/backend/src/ref_backend/core/file_handling.py b/backend/src/ref_backend/core/file_handling.py index ba482b16..3bcf9079 100644 --- a/backend/src/ref_backend/core/file_handling.py +++ b/backend/src/ref_backend/core/file_handling.py @@ -4,11 +4,14 @@ import zlib from collections.abc import Callable, Generator from pathlib import Path +from typing import TypeVar from fastapi import HTTPException +T = TypeVar("T", bound=Path | None) -def resolve_artifact(resolve: Callable[..., Path], *parts: str) -> Path: + +def resolve_artifact(resolve: Callable[..., T], *parts: str | None) -> T: """ Call an ``ArtifactsReader`` resolver and turn a containment failure into a 404. diff --git a/backend/src/ref_backend/models/executions.py b/backend/src/ref_backend/models/executions.py index 4cfdbc97..39d2fc16 100644 --- a/backend/src/ref_backend/models/executions.py +++ b/backend/src/ref_backend/models/executions.py @@ -19,10 +19,10 @@ class ExecutionOutput(BaseModel): id: int execution_id: int output_type: ResultOutputType - filename: str - short_name: str - long_name: str - description: str + filename: str | None + short_name: str | None + long_name: str | None + description: str | None created_at: datetime updated_at: datetime url: str diff --git a/backend/tests/test_api/test_routes/test_executions.py b/backend/tests/test_api/test_routes/test_executions.py index 20bd1309..b2d62fa3 100644 --- a/backend/tests/test_api/test_routes/test_executions.py +++ b/backend/tests/test_api/test_routes/test_executions.py @@ -1,5 +1,7 @@ import io +import json import tarfile +from types import SimpleNamespace import pytest from fastapi.testclient import TestClient @@ -683,3 +685,50 @@ def test_execution_archive(client: TestClient, settings, monkeypatch, tmp_path) with tarfile.open(fileobj=io.BytesIO(r.content), mode="r:gz") as tar: assert sorted(tar.getnames()) == ["diagnostic.json", "plots/map.png"] assert tar.extractfile("plots/map.png").read() == b"png" * 1000 + + +def test_metric_bundle(client: TestClient, settings) -> None: + group_id = get_execution_group_id(client, settings) + r = client.get(f"{settings.API_V1_STR}/executions/{group_id}/metric_bundle") + assert r.status_code == 200 + assert "RESULTS" in r.json() + + +def test_metric_bundle_reads_recorded_path(client: TestClient, settings, monkeypatch, tmp_path) -> None: + group_id = get_execution_group_id(client, settings) + bundle = client.get(f"{settings.API_V1_STR}/executions/{group_id}/metric_bundle").json() + custom = tmp_path / "custom.json" + custom.write_text(json.dumps(bundle)) + execution = SimpleNamespace(id=1, output_fragment="some/fragment", path="custom.json") + monkeypatch.setattr(executions_routes, "_get_execution", lambda *_: execution) + calls = [] + + def fake_resolve(resolve, *parts): + calls.append((resolve.__name__, parts)) + return custom + + monkeypatch.setattr(executions_routes, "resolve_artifact", fake_resolve) + + r = client.get(f"{settings.API_V1_STR}/executions/{group_id}/metric_bundle") + + assert r.status_code == 200 + assert r.json() == bundle + assert calls == [("bundle", ("some/fragment", "custom.json"))] + + +def test_metric_bundle_404_without_recorded_bundle(client: TestClient, settings) -> None: + r = client.get(f"{settings.API_V1_STR}/executions/37/metric_bundle?execution_id=20") + assert r.status_code == 404 + assert r.json()["detail"] == "Metrics bundle not found" + + +def test_execution_list_fails_when_a_group_cannot_be_built(app, settings, monkeypatch) -> None: + def fail_build(*args, **kwargs): + raise RuntimeError("build failed") + + monkeypatch.setattr(executions_routes.ExecutionGroup, "build", fail_build) + + with TestClient(app, raise_server_exceptions=False) as client: + r = client.get(f"{settings.API_V1_STR}/executions/") + + assert r.status_code == 500 diff --git a/changelog/173.fix.1.md b/changelog/173.fix.1.md new file mode 100644 index 00000000..751c66d2 --- /dev/null +++ b/changelog/173.fix.1.md @@ -0,0 +1 @@ +Execution outputs with no filename, name or description no longer fail to load, matching the fields `climate-ref` allows to be empty. diff --git a/changelog/173.fix.md b/changelog/173.fix.md new file mode 100644 index 00000000..6a36e7fc --- /dev/null +++ b/changelog/173.fix.md @@ -0,0 +1 @@ +The metric bundle endpoint now reads the bundle path recorded for each execution instead of assuming `diagnostic.json`. diff --git a/changelog/173.improvement.md b/changelog/173.improvement.md new file mode 100644 index 00000000..585e1c1e --- /dev/null +++ b/changelog/173.improvement.md @@ -0,0 +1 @@ +`GET /executions/` now returns an error when an execution group cannot be loaded, instead of leaving it out of the page. diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index d1074d13..5091af15 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -1013,19 +1013,19 @@ export type ExecutionOutput = { /** * Filename */ - filename: string; + filename: string | null; /** * Short Name */ - short_name: string; + short_name: string | null; /** * Long Name */ - long_name: string; + long_name: string | null; /** * Description */ - description: string; + description: string | null; /** * Created At */ diff --git a/frontend/src/components/diagnostics/figure.tsx b/frontend/src/components/diagnostics/figure.tsx index 9e0b13a5..d41fe0be 100644 --- a/frontend/src/components/diagnostics/figure.tsx +++ b/frontend/src/components/diagnostics/figure.tsx @@ -21,15 +21,21 @@ export function Figure({ long_name, size, }: ExecutionOutput & VariantProps) { - const hasLatex = long_name ? containsLatex(long_name) : false; const captionHtml = useMemo( - () => (hasLatex ? renderLatexToHtml(long_name) : null), - [long_name, hasLatex], + () => + long_name && containsLatex(long_name) + ? renderLatexToHtml(long_name) + : null, + [long_name], ); return (
- {long_name} + {long_name {captionHtml ? ( matchesSelectorFilters(executionGroup.selectors, selectorFilters) && (!pattern || - pattern.test(figure.description) || - pattern.test(figure.filename)), + [figure.description, figure.filename].some( + (field) => field != null && pattern.test(field), + )), ); }