Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve API Error Handling, etc #467

Merged
merged 4 commits into from
Oct 24, 2024
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
2 changes: 1 addition & 1 deletion frontend/src/api/experiments/Experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export async function downloadExperimentConfigApi(
return response.data
}

export async function renameExperiment(
export async function renameExperimentApi(
workspaceId: number,
uid: string,
new_name: string,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { memo, useContext, useState } from "react"
import { useSelector, useDispatch } from "react-redux"

import { useSnackbar } from "notistack"

import DeleteOutlineIcon from "@mui/icons-material/DeleteOutline"
import IconButton from "@mui/material/IconButton"

Expand All @@ -26,12 +28,20 @@ export const DeleteButton = memo(function DeleteButton() {
})
const name = useSelector(selectExperimentName(uid))
const [open, setOpen] = useState(false)
const { enqueueSnackbar } = useSnackbar()

const openDialog = () => {
setOpen(true)
}
const handleDelete = () => {
dispatch(deleteExperimentByUid(uid))
.unwrap()
.then(() => {
// do nothing.
})
.catch(() => {
enqueueSnackbar("Failed to delete", { variant: "error" })
})
uid === currentPipelineUid && dispatch(clearCurrentPipeline())
}

Expand Down
23 changes: 21 additions & 2 deletions frontend/src/components/Workspace/Experiment/ExperimentTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
} from "react"
import { useSelector, useDispatch } from "react-redux"

import { useSnackbar } from "notistack"

import DeleteIcon from "@mui/icons-material/Delete"
import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown"
import KeyboardArrowUpIcon from "@mui/icons-material/KeyboardArrowUp"
Expand All @@ -34,7 +36,7 @@ import TableRow from "@mui/material/TableRow"
import TableSortLabel from "@mui/material/TableSortLabel"
import Typography from "@mui/material/Typography"

import { renameExperiment } from "api/experiments/Experiments"
import { renameExperimentApi } from "api/experiments/Experiments"
import { ConfirmDialog } from "components/common/ConfirmDialog"
import { useLocalStorage } from "components/utils/LocalStorageUtil"
import { DeleteButton } from "components/Workspace/Experiment/Button/DeleteButton"
Expand Down Expand Up @@ -122,6 +124,8 @@ const TableImple = memo(function TableImple() {
const isPending = selectPipelineIsStartedSuccess(state)
return checkedList.includes(currentUid as string) && isPending
})
const { enqueueSnackbar } = useSnackbar()

const onClickReload = () => {
dispatch(getExperiments())
}
Expand Down Expand Up @@ -157,6 +161,14 @@ const TableImple = memo(function TableImple() {
}
const onClickOk = () => {
dispatch(deleteExperimentByList(checkedList))
.unwrap()
.then(() => {
// do nothing.
})
.catch(() => {
enqueueSnackbar("Failed to delete", { variant: "error" })
})

checkedList.filter((v) => v === currentPipelineUid).length > 0 &&
dispatch(clearCurrentPipeline())
setCheckedList([])
Expand Down Expand Up @@ -424,6 +436,7 @@ const RowItem = memo(function RowItem({
const [errorEdit, setErrorEdit] = useState("")
const [valueEdit, setValueEdit] = useState(name)
const dispatch = useDispatch<AppDispatch>()
const { enqueueSnackbar } = useSnackbar()

const onBlurEdit = (event: FocusEvent) => {
event.preventDefault()
Expand Down Expand Up @@ -451,7 +464,13 @@ const RowItem = memo(function RowItem({

const onSaveNewName = async () => {
if (valueEdit === name || workspaceId === void 0) return
await renameExperiment(workspaceId, uid, valueEdit)

try {
await renameExperimentApi(workspaceId, uid, valueEdit)
} catch (e) {
enqueueSnackbar("Failed to rename", { variant: "error" })
}

dispatch(getExperiments())
}

Expand Down
11 changes: 7 additions & 4 deletions frontend/src/const/API.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
const HOST =
process.env.REACT_APP_SERVER_HOST ?? window.location.hostname ?? "localhost"
const PORT = process.env.REACT_APP_SERVER_PORT ?? window.location.port ?? 8000
process.env.REACT_APP_SERVER_HOST || window.location.hostname || "localhost"
const PROTO =
process.env.REACT_APP_SERVER_PROTO ??
window.location.protocol.replace(":", "") ??
process.env.REACT_APP_SERVER_PROTO ||
window.location.protocol.replace(":", "") ||
"http"
const PORT =
process.env.REACT_APP_SERVER_PORT ||
window.location.port ||
(PROTO == "https" ? 443 : 8000)

export const BASE_URL =
PORT == null ? `${PROTO}://${HOST}` : `${PROTO}://${HOST}:${PORT}`
15 changes: 12 additions & 3 deletions frontend/src/store/slice/Pipeline/PipelineHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,28 @@ export function useRunPipeline() {
const filePathIsUndefined = useSelector(selectFilePathIsUndefined)
const algorithmNodeNotExist = useSelector(selectAlgorithmNodeNotExist)
const runPostData = useSelector(selectRunPostData)
const { enqueueSnackbar } = useSnackbar()

const handleRunPipeline = useCallback(
(name: string) => {
dispatch(run({ runPostData: { name, ...runPostData, forceRunList: [] } }))
.unwrap()
.catch(() => {
enqueueSnackbar("Failed to Run workflow", { variant: "error" })
})
},
[dispatch, runPostData],
[dispatch, enqueueSnackbar, runPostData],
)
const handleClickVariant = (variant: VariantType, mess: string) => {
enqueueSnackbar(mess, { variant })
}
const handleRunPipelineByUid = useCallback(() => {
dispatch(runByCurrentUid({ runPostData }))
}, [dispatch, runPostData])
.unwrap()
.catch(() => {
enqueueSnackbar("Failed to Run workflow", { variant: "error" })
})
}, [dispatch, enqueueSnackbar, runPostData])
const handleCancelPipeline = useCallback(async () => {
if (uid != null) {
const data = await dispatch(cancelResult({ uid }))
Expand All @@ -123,7 +133,6 @@ export function useRunPipeline() {
}
}, [dispatch, uid, isCanceled, isStartedSuccess])
const status = useSelector(selectPipelineStatus)
const { enqueueSnackbar } = useSnackbar()
// タブ移動による再レンダリングするたびにスナックバーが実行されてしまう挙動を回避するために前回の値を保持
const [prevStatus, setPrevStatus] = useState(status)
useEffect(() => {
Expand Down
16 changes: 12 additions & 4 deletions studio/app/common/core/experiment/experiment_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def write(self) -> None:
else:
self.create_config()

self.function_from_nodeDict()
self.build_function_from_nodeDict()

ConfigWriter.write(
dirname=join_filepath(
Expand All @@ -72,12 +72,14 @@ def create_config(self) -> ExptConfig:

def add_run_info(self) -> ExptConfig:
return (
self.builder.set_started_at(datetime.now().strftime(DATE_FORMAT)) # 時間を更新
self.builder.set_started_at(
datetime.now().strftime(DATE_FORMAT)
) # Update time
.set_success("running")
.build()
)

def function_from_nodeDict(self) -> ExptConfig:
def build_function_from_nodeDict(self) -> ExptConfig:
func_dict: Dict[str, ExptFunction] = {}
node_dict = WorkflowConfigReader.read(
join_filepath(
Expand Down Expand Up @@ -113,10 +115,13 @@ def __init__(
self.unique_id = unique_id

def delete_data(self) -> bool:
result = True

shutil.rmtree(
join_filepath([DIRPATH.OUTPUT_DIR, self.workspace_id, self.unique_id])
)
return True

return result

def rename(self, new_name: str) -> ExptConfig:
filepath = join_filepath(
Expand All @@ -128,6 +133,9 @@ def rename(self, new_name: str) -> ExptConfig:
]
)

# validate params
new_name = "" if new_name is None else new_name # filter None

# Note: "r+" option is not used here because it requires file pointer control.
with open(filepath, "r") as f:
config = yaml.safe_load(f)
Expand Down
42 changes: 32 additions & 10 deletions studio/app/common/routers/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
from glob import glob
from typing import Dict

from fastapi import APIRouter, Depends, HTTPException
from fastapi import APIRouter, Depends, HTTPException, status
from fastapi.responses import FileResponse

from studio.app.common.core.experiment.experiment import ExptConfig
from studio.app.common.core.experiment.experiment_reader import ExptConfigReader
from studio.app.common.core.experiment.experiment_writer import ExptDataWriter
from studio.app.common.core.logger import AppLogger
from studio.app.common.core.utils.filepath_creater import join_filepath
from studio.app.common.core.workspace.workspace_dependencies import (
is_workspace_available,
Expand All @@ -18,6 +19,8 @@

router = APIRouter(prefix="/experiments", tags=["experiments"])

logger = AppLogger.get_logger()


@router.get(
"/{workspace_id}",
Expand All @@ -33,7 +36,8 @@ async def get_experiments(workspace_id: str):
try:
config = ExptConfigReader.read(path)
exp_config[config.unique_id] = config
except Exception:
except Exception as e:
logger.error(e, exc_info=True)
pass

return exp_config
Expand All @@ -49,10 +53,18 @@ async def rename_experiment(workspace_id: str, unique_id: str, item: RenameItem)
workspace_id,
unique_id,
).rename(item.new_name)
config.nodeDict = []
config.edgeDict = []
try:
config.nodeDict = []
config.edgeDict = []

return config

return config
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="rename experiment failed",
)


@router.delete(
Expand All @@ -67,8 +79,12 @@ async def delete_experiment(workspace_id: str, unique_id: str):
unique_id,
).delete_data()
return True
except Exception:
return False
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="can not delete record.",
)


@router.post(
Expand All @@ -84,8 +100,12 @@ async def delete_experiment_list(workspace_id: str, deleteItem: DeleteItem):
unique_id,
).delete_data()
return True
except Exception:
return False
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="can not delete record.",
)


@router.get(
Expand All @@ -99,4 +119,6 @@ async def download_config_experiment(workspace_id: str, unique_id: str):
if os.path.exists(config_filepath):
return FileResponse(config_filepath)
else:
raise HTTPException(status_code=404, detail="file not found")
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND, detail="file not found"
)
52 changes: 41 additions & 11 deletions studio/app/common/routers/run.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import uuid
from typing import Dict

from fastapi import APIRouter, BackgroundTasks, Depends
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status

from studio.app.common.core.logger import AppLogger
from studio.app.common.core.workflow.workflow import Message, NodeItem, RunItem
Expand All @@ -23,12 +23,20 @@
dependencies=[Depends(is_workspace_owner)],
)
async def run(workspace_id: str, runItem: RunItem, background_tasks: BackgroundTasks):
unique_id = str(uuid.uuid4())[:8]
WorkflowRunner(workspace_id, unique_id, runItem).run_workflow(background_tasks)
try:
unique_id = str(uuid.uuid4())[:8]
WorkflowRunner(workspace_id, unique_id, runItem).run_workflow(background_tasks)

logger.info("run snakemake")
logger.info("run snakemake")

return unique_id
return unique_id

except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Failed to run workflow.",
)


@router.post(
Expand All @@ -39,12 +47,20 @@ async def run(workspace_id: str, runItem: RunItem, background_tasks: BackgroundT
async def run_id(
workspace_id: str, uid: str, runItem: RunItem, background_tasks: BackgroundTasks
):
WorkflowRunner(workspace_id, uid, runItem).run_workflow(background_tasks)
try:
WorkflowRunner(workspace_id, uid, runItem).run_workflow(background_tasks)

logger.info("run snakemake")
logger.info("forcerun list: %s", runItem.forceRunList)

logger.info("run snakemake")
logger.info("forcerun list: %s", runItem.forceRunList)
return uid

return uid
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Failed to run workflow.",
)


@router.post(
Expand All @@ -53,7 +69,14 @@ async def run_id(
dependencies=[Depends(is_workspace_available)],
)
async def run_result(workspace_id: str, uid: str, nodeDict: NodeItem):
return WorkflowResult(workspace_id, uid).get(nodeDict.pendingNodeIdList)
try:
return WorkflowResult(workspace_id, uid).get(nodeDict.pendingNodeIdList)
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Failed to result workflow.",
)


@router.post(
Expand All @@ -62,4 +85,11 @@ async def run_result(workspace_id: str, uid: str, nodeDict: NodeItem):
dependencies=[Depends(is_workspace_owner)],
)
async def run_cancel(workspace_id: str, uid: str):
return WorkflowResult(workspace_id, uid).cancel()
try:
return WorkflowResult(workspace_id, uid).cancel()
except Exception as e:
logger.error(e, exc_info=True)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Failed to cencel workflow.",
)
Loading
Loading