diff --git a/optuna_dashboard/artifact/_backend.py b/optuna_dashboard/artifact/_backend.py index ee1b85a8..55dc844c 100644 --- a/optuna_dashboard/artifact/_backend.py +++ b/optuna_dashboard/artifact/_backend.py @@ -179,7 +179,7 @@ def register_artifact_route( @app.delete("/api/artifacts///") @json_api_view - def delete_artifact(study_id: int, trial_id: int, artifact_id: str) -> dict[str, Any]: + def delete_trial_artifact(study_id: int, trial_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."} @@ -187,7 +187,7 @@ def register_artifact_route( # The artifact's metadata is stored in one of the following two locations: storage.set_study_system_attr( - study_id, _dashboard_trial_artifact_prefix(trial_id) + artifact_id, json.dumps(None) + study_id, _dashboard_artifact_prefix(trial_id) + artifact_id, json.dumps(None) ) storage.set_trial_system_attr( trial_id, ARTIFACTS_ATTR_PREFIX + artifact_id, json.dumps(None) @@ -196,6 +196,25 @@ def register_artifact_route( response.status = 204 return {} + @app.delete("/api/artifacts//") + @json_api_view + 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) + + # The artifact's metadata is stored in one of the following two locations: + storage.set_study_system_attr( + study_id, _dashboard_artifact_prefix(study_id) + artifact_id, json.dumps(None) + ) + storage.set_study_system_attr( + study_id, ARTIFACTS_ATTR_PREFIX + artifact_id, json.dumps(None) + ) + + response.status = 204 + return {} + def upload_artifact( backend: ArtifactBackend, @@ -253,7 +272,7 @@ def upload_artifact( return artifact_id -def _dashboard_trial_artifact_prefix(trial_id: int) -> str: +def _dashboard_artifact_prefix(trial_id: int) -> str: return DASHBOARD_ARTIFACTS_ATTR_PREFIX + f"{trial_id}:" @@ -273,7 +292,7 @@ def get_trial_artifact_meta( ) -> Optional[ArtifactMeta]: # Search study_system_attrs due to backward compatibility. study_system_attrs = storage.get_study_system_attrs(study_id) - attr_key = _dashboard_trial_artifact_prefix(trial_id=trial_id) + artifact_id + attr_key = _dashboard_artifact_prefix(trial_id=trial_id) + artifact_id artifact_meta = study_system_attrs.get(attr_key) if artifact_meta is not None: return json.loads(artifact_meta) @@ -317,7 +336,7 @@ def list_trial_artifacts( dashboard_artifact_metas = [ json.loads(value) for key, value in study_system_attrs.items() - if key.startswith(_dashboard_trial_artifact_prefix(trial._trial_id)) + if key.startswith(_dashboard_artifact_prefix(trial._trial_id)) ] # Collect ArtifactMeta from trial_system_attrs. Note that artifacts uploaded via diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index cfcf4c24..e3be3d85 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -11,9 +11,11 @@ import { tellTrialAPI, saveTrialUserAttrsAPI, renameStudyAPI, - uploadArtifactAPI, + uploadTrialArtifactAPI, + uploadStudyArtifactAPI, getMetaInfoAPI, - deleteArtifactAPI, + deleteTrialArtifactAPI, + deleteStudyArtifactAPI, reportPreferenceAPI, skipPreferentialTrialAPI, removePreferentialHistoryAPI, @@ -106,7 +108,7 @@ export const actionCreator = () => { setStudyDetailState(studyId, newStudy) } - const deleteTrialArtifact = ( + const deleteTrialArtifactState = ( studyId: number, trialId: number, artifact_id: string @@ -128,6 +130,18 @@ export const actionCreator = () => { setTrialArtifacts(studyId, index, newArtifacts) } + const deleteStudyArtifactState = (studyId: number, artifact_id: string) => { + const artifacts = studyDetails[studyId].artifacts + const artifactIndex = artifacts.findIndex( + (a) => a.artifact_id === artifact_id + ) + const newArtifacts = [ + ...artifacts.slice(0, artifactIndex), + ...artifacts.slice(artifactIndex + 1, artifacts.length), + ] + setStudyArtifacts(studyId, newArtifacts) + } + const setTrialStateValues = ( studyId: number, index: number, @@ -445,7 +459,7 @@ export const actionCreator = () => { setUploading(true) reader.readAsDataURL(file) reader.onload = (upload: ProgressEvent) => { - uploadArtifactAPI( + uploadTrialArtifactAPI( studyId, trialId, file.name, @@ -473,17 +487,13 @@ export const actionCreator = () => { } } - const uploadStudyArtifact = ( - studyId: number, - file: File - ): void => { + const uploadStudyArtifact = (studyId: number, file: File): void => { const reader = new FileReader() setUploading(true) reader.readAsDataURL(file) reader.onload = (upload: ProgressEvent) => { - uploadArtifactAPI( + uploadStudyArtifactAPI( studyId, - null, file.name, upload.target?.result as string ) @@ -503,14 +513,30 @@ export const actionCreator = () => { } } - const deleteArtifact = ( + const deleteTrialArtifact = ( studyId: number, trialId: number, artifactId: string ): void => { - deleteArtifactAPI(studyId, trialId, artifactId) + deleteTrialArtifactAPI(studyId, trialId, artifactId) .then(() => { - deleteTrialArtifact(studyId, trialId, artifactId) + deleteTrialArtifactState(studyId, trialId, artifactId) + enqueueSnackbar(`Success to delete an artifact.`, { + variant: "success", + }) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to delete ${reason}.`, { + variant: "error", + }) + }) + } + + const deleteStudyArtifact = (studyId: number, artifactId: string): void => { + deleteStudyArtifactAPI(studyId, artifactId) + .then(() => { + deleteStudyArtifactState(studyId, artifactId) enqueueSnackbar(`Success to delete an artifact.`, { variant: "success", }) @@ -731,7 +757,8 @@ export const actionCreator = () => { saveTrialNote, uploadTrialArtifact, uploadStudyArtifact, - deleteArtifact, + deleteTrialArtifact, + deleteStudyArtifact, makeTrialComplete, makeTrialFail, saveTrialUserAttrs, diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index c1dedd14..f42d20de 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -280,17 +280,14 @@ type UploadArtifactAPIResponse = { artifacts: Artifact[] } -export const uploadArtifactAPI = ( +export const uploadTrialArtifactAPI = ( studyId: number, - trialId: number | null, + trialId: number, fileName: string, dataUrl: string ): Promise => { - const APIurl = `/api/artifacts/${studyId}${ - trialId != null ? `/${trialId}` : "" - }` return axiosInstance - .post(APIurl, { + .post(`/api/artifacts/${studyId}/${trialId}`, { file: dataUrl, filename: fileName, }) @@ -299,7 +296,22 @@ export const uploadArtifactAPI = ( }) } -export const deleteArtifactAPI = ( +export const uploadStudyArtifactAPI = ( + studyId: number, + fileName: string, + dataUrl: string +): Promise => { + return axiosInstance + .post(`/api/artifacts/${studyId}`, { + file: dataUrl, + filename: fileName, + }) + .then((res) => { + return res.data + }) +} + +export const deleteTrialArtifactAPI = ( studyId: number, trialId: number, artifactId: string @@ -311,6 +323,17 @@ export const deleteArtifactAPI = ( }) } +export const deleteStudyArtifactAPI = ( + studyId: number, + artifactId: string +): Promise => { + return axiosInstance + .delete(`/api/artifacts/${studyId}/${artifactId}`) + .then(() => { + return + }) +} + export const tellTrialAPI = ( trialId: number, state: TrialStateFinished, diff --git a/optuna_dashboard/ts/components/DeleteArtifactDialog.tsx b/optuna_dashboard/ts/components/DeleteArtifactDialog.tsx index 2c9c229e..4bf5e95d 100644 --- a/optuna_dashboard/ts/components/DeleteArtifactDialog.tsx +++ b/optuna_dashboard/ts/components/DeleteArtifactDialog.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode, useState } from "react" +import React, { ReactNode, useState, FC } from "react" import { Dialog, DialogTitle, @@ -9,9 +9,9 @@ import { } from "@mui/material" import { actionCreator } from "../action" -export const useDeleteArtifactDialog = (): [ +export const useDeleteTrialArtifactDialog = (): [ (studyId: number, trialId: number, artifact: Artifact) => void, - () => ReactNode + () => ReactNode, ] => { const action = actionCreator() @@ -33,7 +33,7 @@ export const useDeleteArtifactDialog = (): [ if (artifact === null) { return } - action.deleteArtifact(studyId, trialId, artifact.artifact_id) + action.deleteTrialArtifact(studyId, trialId, artifact.artifact_id) setOpenDeleteArtifactDialog(false) setTarget([-1, -1, null]) } @@ -45,32 +45,96 @@ export const useDeleteArtifactDialog = (): [ const renderDeleteArtifactDialog = () => { return ( - { - handleCloseDeleteArtifactDialog() - }} - aria-labelledby="delete-artifact-dialog-title" - > - - Delete artifact - - - - Are you sure you want to delete an artifact (" - {target[2]?.filename}")? - - - - - - - + ) } return [openDialog, renderDeleteArtifactDialog] } + +export const useDeleteStudyArtifactDialog = (): [ + (studyId: number, artifact: Artifact) => void, + () => ReactNode, +] => { + const action = actionCreator() + + const [openDeleteArtifactDialog, setOpenDeleteArtifactDialog] = + useState(false) + const [target, setTarget] = useState<[number, Artifact | null]>([-1, null]) + + const handleCloseDeleteArtifactDialog = () => { + setOpenDeleteArtifactDialog(false) + setTarget([-1, null]) + } + + const handleDeleteArtifact = () => { + const [studyId, artifact] = target + if (artifact === null) { + return + } + action.deleteStudyArtifact(studyId, artifact.artifact_id) + setOpenDeleteArtifactDialog(false) + setTarget([-1, null]) + } + + const openDialog = (studyId: number, artifact: Artifact) => { + setTarget([studyId, artifact]) + setOpenDeleteArtifactDialog(true) + } + + const renderDeleteArtifactDialog = () => { + return ( + + ) + } + return [openDialog, renderDeleteArtifactDialog] +} + +const DeleteDialog: FC<{ + openDeleteArtifactDialog: boolean + handleCloseDeleteArtifactDialog: () => void + filename: string | undefined + handleDeleteArtifact: () => void +}> = ({ + openDeleteArtifactDialog, + handleCloseDeleteArtifactDialog, + filename, + handleDeleteArtifact, +}) => { + return ( + { + handleCloseDeleteArtifactDialog() + }} + aria-labelledby="delete-artifact-dialog-title" + > + + Delete artifact + + + + Are you sure you want to delete an artifact (" + {filename}")? + + + + + + + + ) +} diff --git a/optuna_dashboard/ts/components/StudyArtifactCards.tsx b/optuna_dashboard/ts/components/StudyArtifactCards.tsx index 9d9f35f9..71143ed4 100644 --- a/optuna_dashboard/ts/components/StudyArtifactCards.tsx +++ b/optuna_dashboard/ts/components/StudyArtifactCards.tsx @@ -21,7 +21,7 @@ import DeleteIcon from "@mui/icons-material/Delete" import FullscreenIcon from "@mui/icons-material/Fullscreen" import { actionCreator } from "../action" -import { useDeleteArtifactDialog } from "./DeleteArtifactDialog" +import { useDeleteStudyArtifactDialog } from "./DeleteArtifactDialog" import { useThreejsArtifactModal, isThreejsArtifact, @@ -31,7 +31,7 @@ import { ArtifactCardMedia } from "./ArtifactCardMedia" export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => { const theme = useTheme() const [openDeleteArtifactDialog, renderDeleteArtifactDialog] = - useDeleteArtifactDialog() + useDeleteStudyArtifactDialog() const [openThreejsArtifactModal, renderThreejsArtifactModal] = useThreejsArtifactModal() @@ -104,11 +104,7 @@ export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => { color="inherit" sx={{ margin: "auto 0" }} onClick={() => { - openDeleteArtifactDialog( - trial.study_id, - trial.trial_id, - artifact - ) + openDeleteArtifactDialog(study.id, artifact) }} > @@ -129,6 +125,7 @@ export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => { })} + {renderDeleteArtifactDialog()} {renderThreejsArtifactModal()} ) diff --git a/optuna_dashboard/ts/components/TrialArtifactCards.tsx b/optuna_dashboard/ts/components/TrialArtifactCards.tsx index edbbce65..1a078f29 100644 --- a/optuna_dashboard/ts/components/TrialArtifactCards.tsx +++ b/optuna_dashboard/ts/components/TrialArtifactCards.tsx @@ -21,7 +21,7 @@ import DeleteIcon from "@mui/icons-material/Delete" import FullscreenIcon from "@mui/icons-material/Fullscreen" import { actionCreator } from "../action" -import { useDeleteArtifactDialog } from "./DeleteArtifactDialog" +import { useDeleteTrialArtifactDialog } from "./DeleteArtifactDialog" import { useThreejsArtifactModal, isThreejsArtifact, @@ -31,7 +31,7 @@ import { ArtifactCardMedia } from "./ArtifactCardMedia" export const TrialArtifactCards: FC<{ trial: Trial }> = ({ trial }) => { const theme = useTheme() const [openDeleteArtifactDialog, renderDeleteArtifactDialog] = - useDeleteArtifactDialog() + useDeleteTrialArtifactDialog() const [openThreejsArtifactModal, renderThreejsArtifactModal] = useThreejsArtifactModal() diff --git a/python_tests/artifact/test_backend.py b/python_tests/artifact/test_backend.py index 74698a09..22544e6f 100644 --- a/python_tests/artifact/test_backend.py +++ b/python_tests/artifact/test_backend.py @@ -12,7 +12,7 @@ def test_get_artifact_path() -> None: def test_artifact_prefix() -> None: - actual = _backend._dashboard_trial_artifact_prefix(trial_id=0) + actual = _backend._dashboard_artifact_prefix(trial_id=0) assert actual == "dashboard:artifacts:0:"