Skip to content

Commit

Permalink
[Project] Fail project deletion if doesn't exist in leader [1.6.x] (m…
Browse files Browse the repository at this point in the history
  • Loading branch information
alonmr committed Mar 7, 2024
1 parent cba3c08 commit 55b8072
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 73 deletions.
44 changes: 10 additions & 34 deletions server/api/api/endpoints/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,38 +227,16 @@ async def delete_project(
background_tasks.add_task(task)
return fastapi.Response(status_code=http.HTTPStatus.ACCEPTED.value)

is_running_in_background = False
force_delete = False
try:
is_running_in_background = await run_in_threadpool(
get_project_member().delete_project,
db_session,
name,
deletion_strategy,
auth_info.projects_role,
auth_info,
wait_for_completion=wait_for_completion,
)
except mlrun.errors.MLRunNotFoundError as exc:
if not server.api.utils.helpers.is_request_from_leader(auth_info.projects_role):
logger.debug(
"Project not found in leader, ensuring project deleted in mlrun",
err=mlrun.errors.err_to_str(exc),
)
force_delete = True

if force_delete:
# In this case the wrapper delete project request is the one deleting the project because it
# doesn't exist in the leader.
await run_in_threadpool(
server.api.crud.Projects().delete_project,
db_session,
name,
deletion_strategy,
auth_info,
)

elif is_running_in_background:
is_running_in_background = await run_in_threadpool(
get_project_member().delete_project,
db_session,
name,
deletion_strategy,
auth_info.projects_role,
auth_info,
wait_for_completion=wait_for_completion,
)
if is_running_in_background:
return fastapi.Response(status_code=http.HTTPStatus.ACCEPTED.value)

else:
Expand All @@ -269,8 +247,6 @@ async def delete_project(
)

await get_project_member().post_delete_project(name)
if force_delete:
return fastapi.Response(status_code=http.HTTPStatus.ACCEPTED.value)
return fastapi.Response(status_code=http.HTTPStatus.NO_CONTENT.value)


Expand Down
43 changes: 11 additions & 32 deletions server/api/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1149,39 +1149,18 @@ async def _delete_project(
wait_for_project_deletion: bool,
background_task_name: str,
):
force_deleted = False
try:
await run_in_threadpool(
get_project_member().delete_project,
db_session,
project_name,
deletion_strategy,
auth_info.projects_role,
auth_info,
wait_for_completion=True,
background_task_name=background_task_name,
)
except mlrun.errors.MLRunNotFoundError as exc:
if not server.api.utils.helpers.is_request_from_leader(auth_info.projects_role):
logger.warning(
"Project not found in leader, ensuring project is deleted in mlrun",
project_name=project_name,
exc=err_to_str(exc),
)
force_deleted = True

if force_deleted:
# In this case the wrapper delete project job is the one deleting the project because it
# doesn't exist in the leader.
await run_in_threadpool(
server.api.crud.Projects().delete_project,
db_session,
project_name,
deletion_strategy,
auth_info,
)
await run_in_threadpool(
get_project_member().delete_project,
db_session,
project_name,
deletion_strategy,
auth_info.projects_role,
auth_info,
wait_for_completion=True,
background_task_name=background_task_name,
)

elif wait_for_project_deletion:
if wait_for_project_deletion:
await run_in_threadpool(
verify_project_is_deleted,
project_name,
Expand Down
21 changes: 14 additions & 7 deletions tests/api/api/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -980,17 +980,24 @@ def test_delete_project_not_found_in_leader(
with unittest.mock.patch.object(
mock_project_follower_iguazio_client,
"delete_project",
side_effect=mlrun.errors.MLRunNotFoundError("Project not found"),
side_effect=mlrun.errors.MLRunNotFoundError("Project not found in Iguazio"),
):
response = unversioned_client.delete(
f"{delete_api_version}/projects/{project.metadata.name}",
)
assert response.status_code == HTTPStatus.ACCEPTED.value

response = unversioned_client.get(
f"v1/projects/{project.metadata.name}",
)
assert response.status_code == HTTPStatus.NOT_FOUND.value
if delete_api_version == "v1":
assert response.status_code == HTTPStatus.NOT_FOUND.value
assert "Project not found in Iguazio" in response.json()["detail"]
else:
background_task = mlrun.common.schemas.BackgroundTask(**response.json())
background_task = server.api.utils.background_tasks.InternalBackgroundTasksHandler().get_background_task(
background_task.metadata.name
)
assert (
background_task.status.state
== mlrun.common.schemas.BackgroundTaskState.failed
)
assert "Project not found in Iguazio" in background_task.status.error


# Test should not run more than a few seconds because we test that if the background task fails,
Expand Down

0 comments on commit 55b8072

Please sign in to comment.