Skip to content

311 fix run deletion #431

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.

Sign up for GitHub

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

Open
Elena-kal wants to merge 2 commits into
base: dev
Choose a base branch
from
+37 −14
Open

311 fix run deletion #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3019288
fix run deletion bug
Elena-kal May 20, 2026
f1798d7
improve error messaging
Elena-kal May 20, 2026
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
18 changes: 11 additions & 7 deletions backend/main/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,21 +197,25 @@ def delete_run(request):
run_name = data.get("run_name")

try:
if run_name not in Run._instances:
delete_run_folder(run_name)
if run_name in Run._instances:
try:
run = Run(run_name)
run.delete_run()
except Exception:
Run._instances.pop(run_name, None)
delete_run_folder(run_name)
Comment on lines +201 to +206
Copy link
Copy Markdown
Collaborator

@AnnaPolensky AnnaPolensky May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look at the delete_run() method of runs. It looks like this:

def delete_run(self) -> None:
        delete_run_folder(self.run_name)
        self._instances.pop(
            self.run_name, None
        )  # remove instance from the class dictionary

So, the except is basically doing exactly the same as the try, apart from the different order? So, could you potentially just swap the order in delete_run or do something like:

def delete_run(self) -> None:
    try:
        delete_run_folder(self.run_name)
    finally:
        self._instances.pop(self.run_name, None)

Copy link
Copy Markdown
Collaborator Author

@Elena-kal Elena-kal May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference between what happens in try is the instantiation of the run which we then delete. This is our "normal" case. However, if the run is broken (because for example it contains a step from another branch that this branch does not know) the instatiation will fail and that is when we land in exception.
The exception essentially does the same thing only it does not need to create the run. I guess what we could do is to not use delete_run at all and simply do it by ourselves without the need to create it, but the Run._instances.pop(run_name, None) feels more like something we only want to do in an exception and not as the normal case. But that is simply my feeling idk what do you say?

Copy link
Copy Markdown
Collaborator Author

@Elena-kal Elena-kal May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what I was trying to say is that simply changing delete_run will not do the trick because this is not where the problem lies.

Copy link
Copy Markdown
Collaborator

@AnnaPolensky AnnaPolensky May 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation. It makes sense to me now, so I would keep your implementation as is.

else:
run = Run(run_name)
run.delete_run()
delete_run_folder(run_name)

return JsonResponse({"success": True, "message": "Deleted run"})
except Exception as e:
traceback.print_exc() # not sure if it still needs to be here
return JsonResponse(
{
"success": False,
"message": format_trace(traceback.format_exception(e)),
"message": str(e),
"traceback": format_trace(traceback.format_exception(e)),
},
status=404,
status=500,
)
else:
return JsonResponse(
Expand Down
33 changes: 26 additions & 7 deletions frontend/src/components/app/runs-table/runs-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RunEditMenu } from "@protzilla/app";
import { RunEditMenu, useNotification } from "@protzilla/app";
import {
DeleteModal,
Icon,
Expand Down Expand Up @@ -113,6 +113,7 @@ export const RunsTable: React.FC<RunsTableProps> = ({
}) => {
const navigate = useNavigate();
const theme = useTheme();
const notify = useNotification();

const { handlePointerEnter, handlePointerLeave, showTooltip, mouseAnchor } =
useTooltipScheduling(true);
Expand Down Expand Up @@ -162,11 +163,29 @@ export const RunsTable: React.FC<RunsTableProps> = ({
setRuns(updated);
};

const handleDeleteRun = (runName: string) => {
void callApiWithParameters("delete_run/", { run_name: runName });
const updated = runs.filter((run) => run.run_name !== runName);
setIsDeleteModalOpen(false);
setRuns(updated);
const handleDeleteRun = async (runName: string) => {
try {
const response = await callApiWithParameters("delete_run/", { run_name: runName });

if (response?.success) {
const updated = runs.filter((run) => run.run_name !== runName);
setIsDeleteModalOpen(false);
setRuns(updated);
} else {
notify({
title: "Error",
message: `Failed to delete run: ${String(response?.message ?? "Unknown error")}`,
type: "error",
});
}
} catch (error) {
console.error("Error deleting run:", error);
notify({
title: "Error",
message: "An unexpected error occurred while deleting the run.",
type: "error",
});
}
};

const handleContinueRun = (runName: string) => {
Expand Down Expand Up @@ -331,7 +350,7 @@ export const RunsTable: React.FC<RunsTableProps> = ({
title={`Delete run "${actionRunName}"?`}
isOpen={isDeleteModalOpen}
onConfirm={() => {
handleDeleteRun(actionRunName);
void handleDeleteRun(actionRunName);
}}
onClose={() => {
setIsDeleteModalOpen(false);
Expand Down
Loading