From 944b748cedbe570558c2d8948ae766acf5be3373 Mon Sep 17 00:00:00 2001 From: y0z Date: Mon, 15 Jan 2024 18:26:00 +0900 Subject: [PATCH 1/2] Enhance delete artifacts APIs and add tests for them --- optuna_dashboard/artifact/_backend.py | 13 +++++-- python_tests/artifact/test_backend.py | 49 +++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/optuna_dashboard/artifact/_backend.py b/optuna_dashboard/artifact/_backend.py index 81cd766d3..b8a44abd9 100644 --- a/optuna_dashboard/artifact/_backend.py +++ b/optuna_dashboard/artifact/_backend.py @@ -14,6 +14,7 @@ from bottle import request from bottle import response import optuna +from optuna.artifacts.exceptions import ArtifactNotFound from optuna.trial import FrozenTrial from .._bottle_util import json_api_view @@ -184,7 +185,11 @@ def delete_trial_artifact(study_id: int, trial_id: int, artifact_id: str) -> dic if artifact_store is None: response.status = 400 # Bad Request return {"reason": "Cannot access to the artifacts."} - artifact_store.remove(artifact_id) + try: + artifact_store.remove(artifact_id) + except ArtifactNotFound as e: + response.status = 404 + return {"reason": str(e)} # The artifact's metadata is stored in one of the following two locations: storage.set_study_system_attr( @@ -203,7 +208,11 @@ def delete_study_artifact(study_id: int, artifact_id: str) -> dict[str, Any]: if artifact_store is None: response.status = 400 # Bad Request return {"reason": "Cannot access to the artifacts."} - artifact_store.remove(artifact_id) + try: + artifact_store.remove(artifact_id) + except ArtifactNotFound as e: + response.status = 404 + return {"reason": str(e)} storage.set_study_system_attr( study_id, ARTIFACTS_ATTR_PREFIX + artifact_id, json.dumps(None) diff --git a/python_tests/artifact/test_backend.py b/python_tests/artifact/test_backend.py index 52fe60276..165c50299 100644 --- a/python_tests/artifact/test_backend.py +++ b/python_tests/artifact/test_backend.py @@ -249,3 +249,52 @@ def test_upload_artifact() -> None: with open(f"{tmpdir}/{res['artifact_id']}", "r") as f: data = f.read() assert data == "dummy_content" + + +def test_delete_study_artifact() -> None: + storage = optuna.storages.InMemoryStorage() + study = optuna.create_study(storage=storage) + with tempfile.TemporaryDirectory() as tmpdir: + artifact_store = FileSystemArtifactStore(tmpdir) + with tempfile.NamedTemporaryFile() as f: + f.write(b"dummy_content") + f.flush() + artifact_id = upload_artifact(study, f.name, artifact_store=artifact_store) + app = create_app(storage, artifact_store) + status, _, _ = send_request( + app, + f"/api/artifacts/{study._study_id}/{artifact_id}", + "DELETE", + ) + assert status == 204 + status, _, _ = send_request( + app, + f"/api/artifacts/{study._study_id}/{artifact_id}", + "DELETE", + ) + assert status == 404 + + +def test_delete_trial_artifact() -> None: + storage = optuna.storages.InMemoryStorage() + study = optuna.create_study(storage=storage) + trial = study.ask() + with tempfile.TemporaryDirectory() as tmpdir: + artifact_store = FileSystemArtifactStore(tmpdir) + with tempfile.NamedTemporaryFile() as f: + f.write(b"dummy_content") + f.flush() + artifact_id = upload_artifact(trial, f.name, artifact_store=artifact_store) + app = create_app(storage, artifact_store) + status, _, _ = send_request( + app, + f"/api/artifacts/{study._study_id}/{trial._trial_id}/{artifact_id}", + "DELETE", + ) + assert status == 204 + status, _, _ = send_request( + app, + f"/api/artifacts/{study._study_id}/{trial._trial_id}/{artifact_id}", + "DELETE", + ) + assert status == 404 From 99a54cb28e703325132959497c0e4c67a1606c97 Mon Sep 17 00:00:00 2001 From: y0z Date: Mon, 15 Jan 2024 19:36:17 +0900 Subject: [PATCH 2/2] Fix import for older versions of Optuna --- optuna_dashboard/artifact/_backend.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/optuna_dashboard/artifact/_backend.py b/optuna_dashboard/artifact/_backend.py index b8a44abd9..a76568e21 100644 --- a/optuna_dashboard/artifact/_backend.py +++ b/optuna_dashboard/artifact/_backend.py @@ -14,7 +14,6 @@ from bottle import request from bottle import response import optuna -from optuna.artifacts.exceptions import ArtifactNotFound from optuna.trial import FrozenTrial from .._bottle_util import json_api_view @@ -182,6 +181,8 @@ def upload_study_artifact_api(study_id: int) -> dict[str, Any]: @app.delete("/api/artifacts///") @json_api_view def delete_trial_artifact(study_id: int, trial_id: int, artifact_id: str) -> dict[str, Any]: + from optuna.artifacts.exceptions import ArtifactNotFound + if artifact_store is None: response.status = 400 # Bad Request return {"reason": "Cannot access to the artifacts."} @@ -205,6 +206,8 @@ def delete_trial_artifact(study_id: int, trial_id: int, artifact_id: str) -> dic @app.delete("/api/artifacts//") @json_api_view def delete_study_artifact(study_id: int, artifact_id: str) -> dict[str, Any]: + from optuna.artifacts.exceptions import ArtifactNotFound + if artifact_store is None: response.status = 400 # Bad Request return {"reason": "Cannot access to the artifacts."}