Skip to content

Commit

Permalink
Don't serialize view of item in delete/purge request
Browse files Browse the repository at this point in the history 
This is a breaking change for the API, it is motivated by
https://sentry.galaxyproject.org/share/issue/3970e49a695f4600bcaeae55021555a9/:
```
Message
Unexpected error
Stack Trace

Newest

FileNotFoundError: [Errno 2] No such file or directory: ''
  File "galaxy/datatypes/interval.py", line 1326, in get_estimated_display_viewport
    with open(dataset.get_file_name()) as fh:
```
This is a purge request, which does return the entire detailed response,
including available display applications. We already check that the
dataset isn't purged, but ther celery task often executes between the
check and the deletion.

I don't think it makes sense to return any data at all here,
and there are other endpoints where we simply return 202 or 204,
like purging user file source instanes.
  • Loading branch information
@mvdbeek
mvdbeek committed Nov 15, 2024
1 parent edac0ff commit fdd125e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 105 deletions.
75 changes: 27 additions & 48 deletions client/src/api/schema/schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8308,30 +8308,6 @@ export interface components {
*/
stop_job: boolean;
};
/**
* DeleteHistoryContentResult
* @description Contains minimum information about the deletion state of a history item.
*
* Can also contain any other properties of the item.
*/
DeleteHistoryContentResult: {
/**
* Deleted
* @description True if the item was successfully deleted.
*/
deleted: boolean;
/**
* ID
* @description The encoded ID of the history item.
* @example 0123456789ABCDEF
*/
id: string;
/**
* Purged
* @description True if the item was successfully removed from disk.
*/
purged?: boolean | null;
};
/** DeleteHistoryPayload */
DeleteHistoryPayload: {
/**
Expand Down Expand Up @@ -19556,10 +19532,6 @@ export interface operations {
* @description Whether to stop the creating job if all outputs of the job have been deleted.
*/
stop_job?: boolean | null;
/** @description View to be passed to the serializer */
view?: string | null;
/** @description Comma-separated list of keys to be passed to the serializer */
keys?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
Expand All @@ -19577,23 +19549,28 @@ export interface operations {
};
};
responses: {
/** @description Request has been executed. */
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
"application/json": unknown;
};
};
/** @description Request accepted, processing will finish later. */
202: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
content?: never;
};
/** @description Request has been executed. */
204: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
Expand Down Expand Up @@ -25155,10 +25132,6 @@ export interface operations {
* @description Whether to stop the creating job if all outputs of the job have been deleted.
*/
stop_job?: boolean | null;
/** @description View to be passed to the serializer */
view?: string | null;
/** @description Comma-separated list of keys to be passed to the serializer */
keys?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
Expand All @@ -25178,23 +25151,28 @@ export interface operations {
};
};
responses: {
/** @description Request has been executed. */
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
"application/json": unknown;
};
};
/** @description Request accepted, processing will finish later. */
202: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
content?: never;
};
/** @description Request has been executed. */
204: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
Expand Down Expand Up @@ -25570,10 +25548,6 @@ export interface operations {
* @description Whether to stop the creating job if all outputs of the job have been deleted.
*/
stop_job?: boolean | null;
/** @description View to be passed to the serializer */
view?: string | null;
/** @description Comma-separated list of keys to be passed to the serializer */
keys?: string | null;
};
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
Expand All @@ -25595,23 +25569,28 @@ export interface operations {
};
};
responses: {
/** @description Request has been executed. */
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
"application/json": unknown;
};
};
/** @description Request accepted, processing will finish later. */
202: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DeleteHistoryContentResult"];
content?: never;
};
/** @description Request has been executed. */
204: {
headers: {
[name: string]: unknown;
};
content?: never;
};
/** @description Request Error */
"4XX": {
Expand Down
22 changes: 0 additions & 22 deletions lib/galaxy/schema/schema.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3413,28 +3413,6 @@ class DeleteHistoryContentPayload(Model):
)


class DeleteHistoryContentResult(Model):
"""Contains minimum information about the deletion state of a history item.
Can also contain any other properties of the item."""

id: DecodedDatabaseIdField = Field(
...,
title="ID",
description="The encoded ID of the history item.",
)
deleted: bool = Field(
...,
title="Deleted",
description="True if the item was successfully deleted.",
)
purged: Optional[bool] = Field(
default=None,
title="Purged",
description="True if the item was successfully removed from disk.",
)


class HistoryContentsArchiveDryRunResult(RootModel):
"""
Contains a collection of filepath/filename entries that represent
Expand Down
27 changes: 7 additions & 20 deletions lib/galaxy/webapps/galaxy/api/history_contents.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
DatasetAssociationRoles,
DatasetSourceType,
DeleteHistoryContentPayload,
DeleteHistoryContentResult,
HistoryContentBulkOperationPayload,
HistoryContentBulkOperationResult,
HistoryContentsArchiveDryRunResult,
Expand Down Expand Up @@ -148,13 +147,11 @@ def ContentTypeQueryParam(default: Optional[HistoryContentType]):


CONTENT_DELETE_RESPONSES = {
200: {
"description": "Request has been executed.",
"model": DeleteHistoryContentResult,
},
202: {
"description": "Request accepted, processing will finish later.",
"model": DeleteHistoryContentResult,
},
204: {
"description": "Request has been executed.",
},
}

Expand Down Expand Up @@ -874,7 +871,6 @@ def delete_typed(
id: HistoryItemIDPathParam,
trans: ProvidesHistoryContext = DependsOnTrans,
type: HistoryContentType = ContentTypePathParam,
serialization_params: SerializationParams = Depends(query_serialization_params),
purge: Optional[bool] = PurgeQueryParam,
recursive: Optional[bool] = RecursiveQueryParam,
stop_job: Optional[bool] = StopJobQueryParam,
Expand All @@ -890,7 +886,6 @@ def delete_typed(
trans,
id,
type,
serialization_params,
purge,
recursive,
stop_job,
Expand All @@ -910,7 +905,6 @@ def delete_legacy(
id: HistoryItemIDPathParam,
trans: ProvidesHistoryContext = DependsOnTrans,
type: HistoryContentType = ContentTypeQueryParam(default=HistoryContentType.dataset),
serialization_params: SerializationParams = Depends(query_serialization_params),
purge: Optional[bool] = PurgeQueryParam,
recursive: Optional[bool] = RecursiveQueryParam,
stop_job: Optional[bool] = StopJobQueryParam,
Expand All @@ -926,7 +920,6 @@ def delete_legacy(
trans,
id,
type,
serialization_params,
purge,
recursive,
stop_job,
Expand All @@ -944,7 +937,6 @@ def delete_dataset(
response: Response,
dataset_id: HistoryItemIDPathParam,
trans: ProvidesHistoryContext = DependsOnTrans,
serialization_params: SerializationParams = Depends(query_serialization_params),
purge: Optional[bool] = PurgeQueryParam,
recursive: Optional[bool] = RecursiveQueryParam,
stop_job: Optional[bool] = StopJobQueryParam,
Expand All @@ -960,7 +952,6 @@ def delete_dataset(
trans,
dataset_id,
HistoryContentType.dataset,
serialization_params,
purge,
recursive,
stop_job,
Expand All @@ -973,7 +964,6 @@ def _delete(
trans: ProvidesHistoryContext,
id: DecodedDatabaseIdField,
type: HistoryContentType,
serialization_params: SerializationParams,
purge: Optional[bool],
recursive: Optional[bool],
stop_job: Optional[bool],
Expand All @@ -985,17 +975,14 @@ def _delete(
payload.purge = payload.purge or purge is True
payload.recursive = payload.recursive or recursive is True
payload.stop_job = payload.stop_job or stop_job is True
rval = self.service.delete(
if self.service.delete(
trans,
id=id,
serialization_params=serialization_params,
contents_type=type,
payload=payload,
)
async_result = rval.pop("async_result", None)
if async_result:
response.status_code = status.HTTP_202_ACCEPTED
return rval
):
return Response(status_code=status.HTTP_202_ACCEPTED)
return Response(status_code=status.HTTP_204_NO_CONTENT)

@router.get(
"/api/histories/{history_id}/contents/archive/{filename}.{format}",
Expand Down
20 changes: 5 additions & 15 deletions lib/galaxy/webapps/galaxy/services/history_contents.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,20 +738,18 @@ def delete(
self,
trans,
id: DecodedDatabaseIdField,
serialization_params: SerializationParams,
contents_type: HistoryContentType,
payload: DeleteHistoryContentPayload,
):
"""
Delete the history content with the given ``id`` and specified type (defaults to dataset)
"""
if contents_type == HistoryContentType.dataset:
return self.__delete_dataset(trans, id, payload.purge, payload.stop_job, serialization_params)
return self.__delete_dataset(trans, id, payload.purge, payload.stop_job)
elif contents_type == HistoryContentType.dataset_collection:
async_result = self.dataset_collection_manager.delete(
return self.dataset_collection_manager.delete(
trans, "history", id, recursive=payload.recursive, purge=payload.purge
)
return {"id": self.encode_id(id), "deleted": True, "async_result": async_result is not None}
else:
raise exceptions.UnknownContentsType(f"Unknown contents type: {contents_type}")

Expand Down Expand Up @@ -865,24 +863,16 @@ def build_archive_files_and_paths(content, *parents):
archive.write(file_path, archive_path)
return archive

def __delete_dataset(
self, trans, id: DecodedDatabaseIdField, purge: bool, stop_job: bool, serialization_params: SerializationParams
):
def __delete_dataset(self, trans, id: DecodedDatabaseIdField, purge: bool, stop_job: bool):
hda = self.hda_manager.get_owned(id, trans.user, current_history=trans.history)
self.history_manager.error_unless_mutable(hda.history)
self.hda_manager.error_if_uploading(hda)

async_result = None
if purge:
async_result = self.hda_manager.purge(hda, user=trans.user)
return self.hda_manager.purge(hda, user=trans.user)
else:
self.hda_manager.delete(hda, stop_job=stop_job)
serialization_params.default_view = "detailed"
rval = self.hda_serializer.serialize_to_view(
hda, user=trans.user, trans=trans, encode_id=False, **serialization_params.model_dump()
)
rval["async_result"] = async_result is not None
return rval
return None

def __update_dataset_collection(self, trans, id: DecodedDatabaseIdField, payload: Dict[str, Any]):
return self.dataset_collection_manager.update(trans, "history", id, payload)
Expand Down

0 comments on commit fdd125e

Please sign in to comment.