Skip to content

Commit

Permalink
Add new field uri to ExportObjectResultMetadata model
Browse files Browse the repository at this point in the history
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`).
  • Loading branch information
kysrpex committed Jan 14, 2025
1 parent 6fe9264 commit 27e9a14
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 16 deletions.
2 changes: 2 additions & 0 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8951,6 +8951,8 @@ export interface components {
error?: string | null;
/** Success */
success: boolean;
/** URI */
uri?: string | null;
};
/**
* ExportObjectType
Expand Down
7 changes: 6 additions & 1 deletion client/src/components/Common/models/exportRecordModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { formatDistanceToNow, parseISO } from "date-fns";

import {
type ExportObjectRequestMetadata,
type ExportObjectResultMetadata,
type ModelStoreFormat,
type ObjectExportTaskResponse,
type StoreExportPayload,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
30 changes: 19 additions & 11 deletions lib/galaxy/managers/model_stores.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) or 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,
)

Expand Down Expand Up @@ -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):
Expand Down
27 changes: 27 additions & 0 deletions lib/galaxy/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 14 additions & 4 deletions lib/galaxy/webapps/galaxy/services/histories.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
CreateHistoryPayload,
CustomBuildsMetadataResponse,
ExportHistoryArchivePayload,
ExportRecordData,
HistoryArchiveExportResult,
HistoryImportArchiveSourceType,
JobExportHistoryArchiveModel,
Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 27e9a14

Please sign in to comment.