Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions backend/openapi.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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": {
Expand Down
15 changes: 6 additions & 9 deletions backend/src/ref_backend/api/routes/executions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down Expand Up @@ -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")

Expand Down
3 changes: 3 additions & 0 deletions backend/src/ref_backend/api/routes/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 7 additions & 3 deletions backend/src/ref_backend/core/file_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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")

Expand Down
8 changes: 4 additions & 4 deletions backend/src/ref_backend/models/executions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
lewisjared marked this conversation as resolved.
Resolved
short_name: str | None
long_name: str | None
description: str | None
created_at: datetime
updated_at: datetime
url: str
Expand Down
49 changes: 49 additions & 0 deletions backend/tests/test_api/test_routes/test_executions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import io
import json
import tarfile
from types import SimpleNamespace

import pytest
from fastapi.testclient import TestClient
Expand Down Expand Up @@ -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
21 changes: 21 additions & 0 deletions backend/tests/test_api/test_routes/test_results.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"
2 changes: 2 additions & 0 deletions changelog/173.fix.1.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions changelog/173.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The metric bundle endpoint now reads the bundle path recorded for each execution instead of assuming `diagnostic.json`.
1 change: 1 addition & 0 deletions changelog/173.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`GET /executions/` now returns an error when an execution group cannot be loaded, instead of leaving it out of the page.
1 change: 1 addition & 0 deletions changelog/173.trivial.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The OpenAPI spec now uses a fixed project name, so a local `.env` no longer changes it.
8 changes: 4 additions & 4 deletions frontend/src/client/types.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
14 changes: 10 additions & 4 deletions frontend/src/components/diagnostics/figure.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,21 @@ export function Figure({
long_name,
size,
}: ExecutionOutput & VariantProps<typeof figureVariants>) {
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 (
<div className="flex flex-col items-center gap-2">
<img src={url} alt={long_name} className={cn(figureVariants({ size }))} />
<img
src={url}
alt={long_name ?? undefined}
Comment thread
lewisjared marked this conversation as resolved.
Resolved
className={cn(figureVariants({ size }))}
/>
{captionHtml ? (
<small
className="text-muted-foreground line-clamp-3"
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/components/diagnostics/figureGalleryData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export function filterFigures(
({ figure, executionGroup }) =>
matchesSelectorFilters(executionGroup.selectors, selectorFilters) &&
(!pattern ||
pattern.test(figure.description) ||
pattern.test(figure.filename)),
[figure.description, figure.filename].some(
(field) => field != null && pattern.test(field),
)),
);
}
3 changes: 2 additions & 1 deletion scripts/generate-client-sdk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading