From 5e5cd68025e90cc82f9f474ada67640f34f4b76e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Manuel=20Dom=C3=ADnguez?= Date: Mon, 13 Jan 2025 16:40:40 +0100 Subject: [PATCH] Add new field `uri` to `ExportObjectResultMetadata` model Write the actual URIs where files are saved to by file sources to a new `uri` field of the `ExportObjectResultMetadata` model. Restore behavior before commit 53012bf, namely setting the history export request metadata before attempting the export. --- Before applying the set of changes from 53012bf, the method `ModelStoreManager.set_history_export_request_metadata()` instantiates a `ExportObjectMetadata` Pydantic model and dumps it to the database in the form of JSON as the field `StoreExportAssociation.export_metadata`. After the export is complete, the method `set_history_export_result_metadata()` takes the same instance of `ExportObjectMetadata`, instantiates a `ExportObjectResultMetadata` Pydantic model, sets it as the `result_data` of the `ExportObjectMetadata` instance, and then saves the `ExportObjectMetadata` Pydantic model in the form of JSON to the database again. After applying the set of changes, the call to `ModelStoreManager.set_history_export_request_metadata()` is delayed until the file has already been saved to the file source, as the actual URI that the file source assigns to the file is otherwise unknown. The URI assigned by the file source overwrites the original target URI in the request. This involves a slight deviation from the previous behavior: if for example, power gets cut at the right time, `StoreExportAssociation.export_metadata` may not exist despite the history having been already saved to the file source, because database writes happen within the `finally:` block. Moreover, overwriting the original target URI from the request is formally wrong, because the actual URI assigned by the file source should be part of the export result metadata, as it becomes known when the export completes. This commit addresses these issues by adding a new field `uri` to the `ExportObjectResultMetadata` model, where the actual URI the file has been saved to by the file source is written. Backwards compatibility is ensured via `HistoriesService._get_export_record_data()`, which retrieves the new field when available and otherwise uses `ExportObjectRequestMetadata.payload.target_uri` (given that `ExportObjectRequestMetadata.payload` is of type `WriteStoreToPayload`). --- client/src/api/schema/schema.ts | 2 ++ .../Common/models/exportRecordModel.ts | 7 ++++- lib/galaxy/managers/model_stores.py | 30 ++++++++++++------- lib/galaxy/schema/schema.py | 27 +++++++++++++++++ .../webapps/galaxy/services/histories.py | 18 ++++++++--- 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 1e9ab944ad2e..857bd1aded9c 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -8951,6 +8951,8 @@ export interface components { error?: string | null; /** Success */ success: boolean; + /** URI */ + uri?: string | null; }; /** * ExportObjectType diff --git a/client/src/components/Common/models/exportRecordModel.ts b/client/src/components/Common/models/exportRecordModel.ts index 35ebd1abf4bc..cf052b5a104c 100644 --- a/client/src/components/Common/models/exportRecordModel.ts +++ b/client/src/components/Common/models/exportRecordModel.ts @@ -2,6 +2,7 @@ import { formatDistanceToNow, parseISO } from "date-fns"; import { type ExportObjectRequestMetadata, + type ExportObjectResultMetadata, type ModelStoreFormat, type ObjectExportTaskResponse, type StoreExportPayload, @@ -85,12 +86,14 @@ export class ExportRecordModel implements ExportRecord { private _data: ObjectExportTaskResponse; private _expirationDate?: Date; private _requestMetadata?: ExportObjectRequestMetadata; + private _resultMetadata?: ExportObjectResultMetadata | null; private _exportParameters?: ExportParamsModel; constructor(data: ObjectExportTaskResponse) { this._data = data; this._expirationDate = undefined; this._requestMetadata = data.export_metadata?.request_data; + this._resultMetadata = data.export_metadata?.result_data; this._exportParameters = this._requestMetadata?.payload ? new ExportParamsModel(this._requestMetadata?.payload) : undefined; @@ -130,7 +133,9 @@ export class ExportRecordModel implements ExportRecord { get importUri() { const payload = this._requestMetadata?.payload; - return payload && "target_uri" in payload ? payload.target_uri : undefined; + const requestUri = payload && "target_uri" in payload ? payload.target_uri : undefined; + const resultUri = this._resultMetadata?.uri; + return resultUri || requestUri; } get canReimport() { diff --git a/lib/galaxy/managers/model_stores.py b/lib/galaxy/managers/model_stores.py index b9761aa5ad2b..2524f5fac095 100644 --- a/lib/galaxy/managers/model_stores.py +++ b/lib/galaxy/managers/model_stores.py @@ -114,6 +114,8 @@ def prepare_history_download(self, request: GenerateHistoryDownload): include_hidden = request.include_hidden include_deleted = request.include_deleted export_metadata = self.set_history_export_request_metadata(request) + + exception_exporting_history: Optional[Exception] = None try: with storage_context( request.short_term_storage_request_id, self._short_term_storage_monitor @@ -122,12 +124,16 @@ def prepare_history_download(self, request: GenerateHistoryDownload): short_term_storage_target.path ) as export_store: export_store.export_history(history, include_hidden=include_hidden, include_deleted=include_deleted) - self.set_history_export_result_metadata(request.export_association_id, export_metadata, success=True) - except Exception as e: + except Exception as exception: + exception_exporting_history = exception + raise + finally: self.set_history_export_result_metadata( - request.export_association_id, export_metadata, success=False, error=str(e) + request.export_association_id, + export_metadata, + success=not bool(exception_exporting_history), + error=str(exception_exporting_history) if exception_exporting_history else None, ) - raise def prepare_history_content_download(self, request: GenerateHistoryContentDownload): model_store_format = request.model_store_format @@ -220,29 +226,30 @@ def write_history_content_to(self, request: WriteHistoryContentTo): def write_history_to(self, request: WriteHistoryTo): model_store_format = request.model_store_format export_files = "symlink" if request.include_files else None - target_uri = request.target_uri user_context = self._build_user_context(request.user.user_id) + export_metadata = self.set_history_export_request_metadata(request) exception_exporting_history: Optional[Exception] = None + uri: Optional[str] = None try: export_store = model.store.get_export_store_factory( self._app, model_store_format, export_files=export_files, user_context=user_context - )(target_uri) + )(request.target_uri) with export_store: history = self._history_manager.by_id(request.history_id) export_store.export_history( history, include_hidden=request.include_hidden, include_deleted=request.include_deleted ) - request.target_uri = str(export_store.file_source_uri) or request.target_uri - except Exception as e: - exception_exporting_history = e + uri = str(export_store.file_source_uri) if export_store.file_source_uri else request.target_uri + except Exception as exception: + exception_exporting_history = exception raise finally: - export_metadata = self.set_history_export_request_metadata(request) self.set_history_export_result_metadata( request.export_association_id, export_metadata, success=not bool(exception_exporting_history), + uri=uri, error=str(exception_exporting_history) if exception_exporting_history else None, ) @@ -273,10 +280,11 @@ def set_history_export_result_metadata( export_association_id: Optional[int], export_metadata: Optional[ExportObjectMetadata], success: bool, + uri: Optional[str] = None, error: Optional[str] = None, ): if export_association_id is not None and export_metadata is not None: - export_metadata.result_data = ExportObjectResultMetadata(success=success, error=error) + export_metadata.result_data = ExportObjectResultMetadata(success=success, uri=uri, error=error) self._export_tracker.set_export_association_metadata(export_association_id, export_metadata) def import_model_store(self, request: ImportModelStoreTaskRequest): diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 3febee546896..a494d26ca635 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -1833,8 +1833,35 @@ class ExportObjectRequestMetadata(Model): class ExportObjectResultMetadata(Model): success: bool + uri: Optional[str] = None error: Optional[str] = None + @model_validator(mode="after") + @classmethod + def validate_success(cls, model): + """ + Ensure successful exports do not have error text. + """ + if model.success and model.error is not None: + raise ValueError("successful exports cannot have error text") + + return model + + @model_validator(mode="after") + @classmethod + def validate_uri(cls, model): + """ + Ensure unsuccessful exports do not have a URI. + """ + # short therm exports need not have a URI + # if model.success and not model.uri: + # raise ValueError("successful exports must have a URI") + + if not model.success and model.uri: + raise ValueError("unsuccessful exports cannot have a URI") + + return model + class ExportObjectMetadata(Model): request_data: ExportObjectRequestMetadata diff --git a/lib/galaxy/webapps/galaxy/services/histories.py b/lib/galaxy/webapps/galaxy/services/histories.py index 32e8a9fa8a1c..1746130ffd70 100644 --- a/lib/galaxy/webapps/galaxy/services/histories.py +++ b/lib/galaxy/webapps/galaxy/services/histories.py @@ -61,6 +61,7 @@ CreateHistoryPayload, CustomBuildsMetadataResponse, ExportHistoryArchivePayload, + ExportRecordData, HistoryArchiveExportResult, HistoryImportArchiveSourceType, JobExportHistoryArchiveModel, @@ -810,15 +811,24 @@ def _serialize_archived_history( serialization_params = SerializationParams() archived_history = self._serialize_history(trans, history, serialization_params, default_view) export_record_data = self._get_export_record_data(history) - archived_history["export_record_data"] = export_record_data.model_dump() if export_record_data else None + archived_history["export_record_data"] = export_record_data return archived_history - def _get_export_record_data(self, history: model.History) -> Optional[WriteStoreToPayload]: + def _get_export_record_data(self, history: model.History) -> Optional[ExportRecordData]: if history.archive_export_id: export_record = self.history_export_manager.get_task_export_by_id(history.archive_export_id) export_metadata = self.history_export_manager.get_record_metadata(export_record) - if export_metadata and isinstance(export_metadata.request_data.payload, WriteStoreToPayload): - return export_metadata.request_data.payload + if export_metadata and isinstance( + request_data_payload := export_metadata.request_data.payload, WriteStoreToPayload + ): + request_uri = request_data_payload.target_uri + result_uri = export_metadata.result_data.uri if export_metadata.result_data else None + + export_record_data_dict = request_data_payload.model_dump() + export_record_data_dict.update({"target_uri": result_uri or request_uri}) + export_record_data = ExportRecordData(**export_record_data_dict) + + return export_record_data return None