diff --git a/backend/openapi.json b/backend/openapi.json index 7b6eb298..c75c271d 100644 --- a/backend/openapi.json +++ b/backend/openapi.json @@ -1,7 +1,7 @@ { "openapi": "3.1.0", "info": { - "title": "Rapid Evaluation Framework", + "title": "Climate Rapid Evaluation Framework", "description": "\nAPI for querying the results from the Climate Rapid Evaluation Framework (Climate REF).\nThis is used to back the [Climate REF Explorer](https://dashboard.climate-ref.org) web application.\n\nThis API is a work in progress and will likely change in the future without warning.\nWe are interested in your feedback on how you would like to use this API.\nThis will help us prioritize features, and be aware of any API consumers before we make any changes.\n\nContact [Jared Lewis](mailto:jared.lewis@climate-resource.com) or\n[file an issue on GitHub](https://github.com/Climate-REF/ref-app/issues),\nif you have any questions or feedback.\n", "contact": { "name": "Development Lead - Jared Lewis", @@ -4222,19 +4222,47 @@ "$ref": "#/components/schemas/ResultOutputType" }, "filename": { - "type": "string", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], "title": "Filename" }, "short_name": { - "type": "string", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], "title": "Short Name" }, "long_name": { - "type": "string", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], "title": "Long Name" }, "description": { - "type": "string", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], "title": "Description" }, "created_at": { 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/api/routes/results.py b/backend/src/ref_backend/api/routes/results.py index 19a5f31d..6fb40d42 100644 --- a/backend/src/ref_backend/api/routes/results.py +++ b/backend/src/ref_backend/api/routes/results.py @@ -24,6 +24,9 @@ def get_result(session: SessionDep, reader: ReaderDep, result_id: int) -> Stream # Release the connection now, so streaming the file does not hold it open session.close() + if filename is None: + raise HTTPException(status_code=404, detail="Result file not found") + file_path = resolve_artifact(reader.artifacts.output_file, output_fragment, filename) mime_type, _encoding = mimetypes.guess_type(file_path) diff --git a/backend/src/ref_backend/core/file_handling.py b/backend/src/ref_backend/core/file_handling.py index ba482b16..a2f755c7 100644 --- a/backend/src/ref_backend/core/file_handling.py +++ b/backend/src/ref_backend/core/file_handling.py @@ -4,11 +4,15 @@ import zlib from collections.abc import Callable, Generator from pathlib import Path +from typing import ParamSpec, TypeVar from fastapi import HTTPException +P = ParamSpec("P") +T = TypeVar("T", bound=Path | None) -def resolve_artifact(resolve: Callable[..., Path], *parts: str) -> Path: + +def resolve_artifact(resolve: Callable[P, T], *args: P.args, **kwargs: P.kwargs) -> T: """ Call an ``ArtifactsReader`` resolver and turn a containment failure into a 404. @@ -19,11 +23,11 @@ def resolve_artifact(resolve: Callable[..., Path], *parts: str) -> Path: ---------- resolve Bound ``ArtifactsReader`` method, such as ``artifacts.log_file`` - parts + args, kwargs Arguments to pass to the resolver """ try: - return resolve(*parts) + return resolve(*args, **kwargs) except ValueError: raise HTTPException(status_code=404, detail="Execution output not found") 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/backend/tests/test_api/test_routes/test_results.py b/backend/tests/test_api/test_routes/test_results.py index b3029bb4..4c6f3737 100644 --- a/backend/tests/test_api/test_routes/test_results.py +++ b/backend/tests/test_api/test_routes/test_results.py @@ -1,6 +1,8 @@ import pytest from fastapi.testclient import TestClient +from sqlalchemy import select +from climate_ref.database import Database from climate_ref.models import ExecutionOutput from ref_backend.api import deps from ref_backend.api.routes.results import get_result @@ -71,3 +73,22 @@ def test_result_releases_connection_before_streaming(): assert response.status_code == 200 assert pool.checkedout() == idle + + +def test_result_without_filename_is_not_found(writable_client: TestClient, settings): + """An output with no recorded filename is listed but has no file to serve.""" + with Database.from_config(writable_client.ref_config, run_migrations=False).session_scope() as session: # type: ignore[attr-defined] + output = session.scalars(select(ExecutionOutput).limit(1)).one() + output.filename = None # type: ignore[assignment] + result_id = output.id + group_id = output.execution.execution_group_id + session.commit() + + r = writable_client.get(f"{settings.API_V1_STR}/executions/{group_id}") + assert r.status_code == 200 + outputs = [o for e in r.json()["executions"] for o in e["outputs"]] + assert next(o for o in outputs if o["id"] == result_id)["filename"] is None + + r = writable_client.get(f"{settings.API_V1_STR}/results/{result_id}") + assert r.status_code == 404 + assert r.json()["detail"] == "Result file not found" diff --git a/changelog/173.fix.1.md b/changelog/173.fix.1.md new file mode 100644 index 00000000..a20ad4a7 --- /dev/null +++ b/changelog/173.fix.1.md @@ -0,0 +1,2 @@ +Execution outputs with no filename, name or description no longer fail to load, matching the fields `climate-ref` allows to be empty. +Downloading an output with no filename now returns a 404 instead of a server error. 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/changelog/173.trivial.md b/changelog/173.trivial.md new file mode 100644 index 00000000..160f0c16 --- /dev/null +++ b/changelog/173.trivial.md @@ -0,0 +1 @@ +The OpenAPI spec now uses a fixed project name, so a local `.env` no longer changes it. 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), + )), ); } diff --git a/scripts/generate-client-sdk.sh b/scripts/generate-client-sdk.sh index 054db9a3..dab2df29 100644 --- a/scripts/generate-client-sdk.sh +++ b/scripts/generate-client-sdk.sh @@ -7,7 +7,8 @@ set -x cd "$(dirname "$0")/.." pushd backend -uv run python -c "import ref_backend.main; import json; print(json.dumps(ref_backend.main.app.openapi(), indent=2))" > openapi.json +# Pinned so a local .env cannot change the committed spec. +PROJECT_NAME="Climate Rapid Evaluation Framework" uv run python -c "import ref_backend.main; import json; print(json.dumps(ref_backend.main.app.openapi(), indent=2))" > openapi.json popd # openapi-ts needs the TypeScript 5 compiler API, which TypeScript 7 does not ship.