diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 1e9ab944ad2e..5eec29d24cd6 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/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py index fd1e84670173..c00160cc4102 100644 --- a/lib/galaxy/files/sources/__init__.py +++ b/lib/galaxy/files/sources/__init__.py @@ -220,7 +220,7 @@ def write_from( native_path: str, user_context: "OptionalUserContext" = None, opts: Optional[FilesSourceOptions] = None, - ): + ) -> str: """Write file at native path to target_path (relative to uri root). :param target_path: url of the target file to write to within the filesource. e.g. `gxfiles://myftp1/myfile.txt` @@ -231,6 +231,9 @@ def write_from( :type user_context: _type_, optional :param opts: A set of options to exercise additional control over the write_from method. Filesource specific, defaults to None :type opts: Optional[FilesSourceOptions], optional + :return: Actual url of the written file, fixed by the service backing the FileSource. May differ from the target + path. + :rtype: str """ @abc.abstractmethod @@ -511,10 +514,10 @@ def write_from( native_path: str, user_context: "OptionalUserContext" = None, opts: Optional[FilesSourceOptions] = None, - ): + ) -> str: self._ensure_writeable() self._check_user_access(user_context) - self._write_from(target_path, native_path, user_context=user_context, opts=opts) + return self._write_from(target_path, native_path, user_context=user_context, opts=opts) or target_path @abc.abstractmethod def _write_from( @@ -523,7 +526,7 @@ def _write_from( native_path: str, user_context: "OptionalUserContext" = None, opts: Optional[FilesSourceOptions] = None, - ): + ) -> Optional[str]: pass def realize_to( diff --git a/lib/galaxy/managers/model_stores.py b/lib/galaxy/managers/model_stores.py index 1e4a82c89363..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 @@ -140,11 +146,13 @@ def prepare_history_content_download(self, request: GenerateHistoryContentDownlo ) as export_store: if request.content_type == HistoryContentType.dataset: hda = self._sa_session.get(model.HistoryDatasetAssociation, request.content_id) - export_store.add_dataset(hda) + export_store.add_dataset(hda) # type: ignore[arg-type] else: hdca = self._sa_session.get(model.HistoryDatasetCollectionAssociation, request.content_id) export_store.export_collection( - hdca, include_hidden=request.include_hidden, include_deleted=request.include_deleted + hdca, # type: ignore[arg-type] + include_hidden=request.include_hidden, + include_deleted=request.include_deleted, ) def prepare_invocation_download(self, request: GenerateInvocationDownload): @@ -161,7 +169,9 @@ def prepare_invocation_download(self, request: GenerateInvocationDownload): )(short_term_storage_target.path) as export_store: invocation = self._sa_session.get(model.WorkflowInvocation, request.invocation_id) export_store.export_workflow_invocation( - invocation, include_hidden=request.include_hidden, include_deleted=request.include_deleted + invocation, # type: ignore[arg-type] + include_hidden=request.include_hidden, + include_deleted=request.include_deleted, ) def write_invocation_to(self, request: WriteInvocationTo): @@ -178,7 +188,9 @@ def write_invocation_to(self, request: WriteInvocationTo): )(target_uri) as export_store: invocation = self._sa_session.get(model.WorkflowInvocation, request.invocation_id) export_store.export_workflow_invocation( - invocation, include_hidden=request.include_hidden, include_deleted=request.include_deleted + invocation, # type: ignore[arg-type] + include_hidden=request.include_hidden, + include_deleted=request.include_deleted, ) def _bco_export_options(self, request: BcoGenerationTaskParametersMixin): @@ -202,33 +214,44 @@ def write_history_content_to(self, request: WriteHistoryContentTo): )(target_uri) as export_store: if request.content_type == HistoryContentType.dataset: hda = self._sa_session.get(model.HistoryDatasetAssociation, request.content_id) - export_store.add_dataset(hda) + export_store.add_dataset(hda) # type: ignore[arg-type] else: hdca = self._sa_session.get(model.HistoryDatasetCollectionAssociation, request.content_id) export_store.export_collection( - hdca, include_hidden=request.include_hidden, include_deleted=request.include_deleted + hdca, # type: ignore[arg-type] + include_hidden=request.include_hidden, + include_deleted=request.include_deleted, ) 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: - with model.store.get_export_store_factory( + export_store = model.store.get_export_store_factory( self._app, model_store_format, export_files=export_files, user_context=user_context - )(target_uri) as export_store: + )(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 ) - self.set_history_export_result_metadata(request.export_association_id, export_metadata, success=True) - except Exception as 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: 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), + uri=uri, + error=str(exception_exporting_history) if exception_exporting_history else None, ) - raise def set_history_export_request_metadata( self, request: Union[WriteHistoryTo, GenerateHistoryDownload] @@ -257,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/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index 3ddafe11ad93..5a49732c0861 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -31,6 +31,7 @@ TYPE_CHECKING, Union, ) +from urllib.parse import urlparse from bdbag import bdbag_api as bdb from boltons.iterutils import remap @@ -2615,8 +2616,18 @@ class BcoExportOptions: override_xref: Optional[List[XrefItem]] = None -class BcoModelExportStore(WorkflowInvocationOnlyExportStore): - def __init__(self, uri, export_options: BcoExportOptions, **kwds): +class FileSourceModelExportStore(abc.ABC, DirectoryModelExportStore): + """ + Export to file sources, from where data can be retrieved later on using a URI. + """ + + file_source_uri: Optional[StrPath] = None + # data can be retrieved later using this URI + + out_file: StrPath + # the output file is written to this path, from which it is written to the file source + + def __init__(self, uri, **kwds): temp_output_dir = tempfile.mkdtemp() self.temp_output_dir = temp_output_dir if "://" in str(uri): @@ -2627,18 +2638,46 @@ def __init__(self, uri, export_options: BcoExportOptions, **kwds): self.out_file = uri self.file_source_uri = None export_directory = temp_output_dir - self.export_options = export_options super().__init__(export_directory, **kwds) + @abc.abstractmethod + def _generate_output_file(self) -> None: + """ + Generate the output file that will be uploaded to the file source. + + Produce an output file and save it to `self.out_file`. A common pattern for this method to create an archive out + of `self.export_directory`. + + This method runs after `DirectoryModelExportStore._finalize()`. Therefore, `self.export_directory` will already + have been populated when it runs. + """ + def _finalize(self): - super()._finalize() - core_biocompute_object, object_id = self._core_biocompute_object_and_object_id() - write_to_file(object_id, core_biocompute_object, self.out_file) + super()._finalize() # populate `self.export_directory` + self._generate_output_file() # generate the output file `self.out_file` if self.file_source_uri: + # upload output file to file source + if not self.file_sources: + raise Exception(f"Need self.file_sources but {type(self)} is missing it: {self.file_sources}.") + file_source_uri = urlparse(str(self.file_source_uri)) file_source_path = self.file_sources.get_file_source_path(self.file_source_uri) file_source = file_source_path.file_source assert os.path.exists(self.out_file) - file_source.write_from(file_source_path.path, self.out_file, user_context=self.user_context) + self.file_source_uri = f"{file_source_uri.scheme}://{file_source_uri.netloc}" + file_source.write_from( + file_source_path.path, self.out_file, user_context=self.user_context + ) + shutil.rmtree(self.temp_output_dir) + + +class BcoModelExportStore(FileSourceModelExportStore, WorkflowInvocationOnlyExportStore): + + def __init__(self, uri, export_options: BcoExportOptions, **kwds): + self.export_options = export_options + super().__init__(uri, **kwds) + + def _generate_output_file(self): + core_biocompute_object, object_id = self._core_biocompute_object_and_object_id() + write_to_file(object_id, core_biocompute_object, self.out_file) def _core_biocompute_object_and_object_id(self) -> Tuple[BioComputeObjectCore, str]: assert self.app # need app.security to do anything... @@ -2828,25 +2867,9 @@ def _finalize(self) -> None: ro_crate.write(self.crate_directory) -class ROCrateArchiveModelExportStore(DirectoryModelExportStore, WriteCrates): - file_source_uri: Optional[StrPath] - out_file: StrPath - - def __init__(self, uri: StrPath, **kwds) -> None: - temp_output_dir = tempfile.mkdtemp() - self.temp_output_dir = temp_output_dir - if "://" in str(uri): - self.out_file = os.path.join(temp_output_dir, "out") - self.file_source_uri = uri - export_directory = os.path.join(temp_output_dir, "export") - else: - self.out_file = uri - self.file_source_uri = None - export_directory = temp_output_dir - super().__init__(export_directory, **kwds) +class ROCrateArchiveModelExportStore(FileSourceModelExportStore, WriteCrates): - def _finalize(self) -> None: - super()._finalize() + def _generate_output_file(self): ro_crate = self._init_crate() ro_crate.write(self.export_directory) out_file_name = str(self.out_file) @@ -2854,48 +2877,18 @@ def _finalize(self) -> None: out_file = out_file_name[: -len(".zip")] else: out_file = out_file_name - rval = shutil.make_archive(out_file, "fastzip", self.export_directory) - if not self.file_source_uri: - shutil.move(rval, self.out_file) - else: - if not self.file_sources: - raise Exception(f"Need self.file_sources but {type(self)} is missing it: {self.file_sources}.") - file_source_path = self.file_sources.get_file_source_path(self.file_source_uri) - file_source = file_source_path.file_source - assert os.path.exists(rval), rval - file_source.write_from(file_source_path.path, rval, user_context=self.user_context) - shutil.rmtree(self.temp_output_dir) + archive = shutil.make_archive(out_file, "fastzip", self.export_directory) + shutil.move(archive, self.out_file) -class TarModelExportStore(DirectoryModelExportStore): - file_source_uri: Optional[StrPath] - out_file: StrPath +class TarModelExportStore(FileSourceModelExportStore): def __init__(self, uri: StrPath, gzip: bool = True, **kwds) -> None: self.gzip = gzip - temp_output_dir = tempfile.mkdtemp() - self.temp_output_dir = temp_output_dir - if "://" in str(uri): - self.out_file = os.path.join(temp_output_dir, "out") - self.file_source_uri = uri - export_directory = os.path.join(temp_output_dir, "export") - else: - self.out_file = uri - self.file_source_uri = None - export_directory = temp_output_dir - super().__init__(export_directory, **kwds) + super().__init__(uri, **kwds) - def _finalize(self) -> None: - super()._finalize() + def _generate_output_file(self): tar_export_directory(self.export_directory, self.out_file, self.gzip) - if self.file_source_uri: - if not self.file_sources: - raise Exception(f"Need self.file_sources but {type(self)} is missing it: {self.file_sources}.") - file_source_path = self.file_sources.get_file_source_path(self.file_source_uri) - file_source = file_source_path.file_source - assert os.path.exists(self.out_file) - file_source.write_from(file_source_path.path, self.out_file, user_context=self.user_context) - shutil.rmtree(self.temp_output_dir) class BagDirectoryModelExportStore(DirectoryModelExportStore): @@ -2908,37 +2901,16 @@ def _finalize(self) -> None: bdb.make_bag(self.out_directory) -class BagArchiveModelExportStore(BagDirectoryModelExportStore): - file_source_uri: Optional[StrPath] +class BagArchiveModelExportStore(FileSourceModelExportStore, BagDirectoryModelExportStore): def __init__(self, uri: StrPath, bag_archiver: str = "tgz", **kwds) -> None: # bag_archiver in tgz, zip, tar self.bag_archiver = bag_archiver - temp_output_dir = tempfile.mkdtemp() - self.temp_output_dir = temp_output_dir - if "://" in str(uri): - # self.out_file = os.path.join(temp_output_dir, "out") - self.file_source_uri = uri - export_directory = os.path.join(temp_output_dir, "export") - else: - self.out_file = uri - self.file_source_uri = None - export_directory = temp_output_dir - super().__init__(export_directory, **kwds) + super().__init__(uri, **kwds) - def _finalize(self) -> None: - super()._finalize() - rval = bdb.archive_bag(self.export_directory, self.bag_archiver) - if not self.file_source_uri: - shutil.move(rval, self.out_file) - else: - if not self.file_sources: - raise Exception(f"Need self.file_sources but {type(self)} is missing it: {self.file_sources}.") - file_source_path = self.file_sources.get_file_source_path(self.file_source_uri) - file_source = file_source_path.file_source - assert os.path.exists(rval) - file_source.write_from(file_source_path.path, rval, user_context=self.user_context) - shutil.rmtree(self.temp_output_dir) + def _generate_output_file(self): + archive = bdb.archive_bag(self.export_directory, self.bag_archiver) + shutil.move(archive, self.out_file) def get_export_store_factory( @@ -2947,13 +2919,8 @@ def get_export_store_factory( export_files=None, bco_export_options: Optional[BcoExportOptions] = None, user_context=None, -) -> Callable[[StrPath], ModelExportStore]: - export_store_class: Union[ - Type[TarModelExportStore], - Type[BagArchiveModelExportStore], - Type[ROCrateArchiveModelExportStore], - Type[BcoModelExportStore], - ] +) -> Callable[[StrPath], FileSourceModelExportStore]: + export_store_class: Type[FileSourceModelExportStore] export_store_class_kwds = { "app": app, "export_files": export_files, diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 3febee546896..57145769b092 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -1833,8 +1833,32 @@ 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. + """ + + 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/tools/imp_exp/export_history.py b/lib/galaxy/tools/imp_exp/export_history.py index 340c19a203d4..c16c4768db56 100644 --- a/lib/galaxy/tools/imp_exp/export_history.py +++ b/lib/galaxy/tools/imp_exp/export_history.py @@ -56,16 +56,18 @@ def main(argv=None): # Create archive. exit = create_archive(temp_directory, out_file, gzip=gzip) if destination_uri is not None and exit == 0: - _write_to_destination(options.file_sources, os.path.abspath(out_file), destination_uri) + actual_uri = _write_to_destination(options.file_sources, os.path.abspath(out_file), destination_uri) + if destination_uri != actual_uri: + print(f"Saved history archive to {actual_uri}.") return exit -def _write_to_destination(file_sources_path: str, out_file: str, destination_uri: str): +def _write_to_destination(file_sources_path: str, out_file: str, destination_uri: str) -> str: file_sources = get_file_sources(file_sources_path) file_source_path = file_sources.get_file_source_path(destination_uri) file_source = file_source_path.file_source assert os.path.exists(out_file) - file_source.write_from(file_source_path.path, out_file) + return file_source.write_from(file_source_path.path, out_file) def get_file_sources(file_sources_path: str) -> ConfiguredFileSources: 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 diff --git a/test/unit/app/managers/test_user_file_sources.py b/test/unit/app/managers/test_user_file_sources.py index bb30feed8184..72ca2c358edd 100644 --- a/test/unit/app/managers/test_user_file_sources.py +++ b/test/unit/app/managers/test_user_file_sources.py @@ -365,9 +365,10 @@ def test_io(self, tmp_path): temp_file = tmp_path / "tmp_file" temp_file.write_text("Moo Cow", "utf-8") - file_source.write_from("/moo", str(temp_file)) + actual_path = file_source.write_from("/moo", str(temp_file)) target = tmp_path / "round_trip" file_source.realize_to("/moo", target) + assert "/moo" == actual_path assert target.read_text("utf-8") == "Moo Cow" def test_to_dict_filters_hidden(self, tmp_path): @@ -421,7 +422,8 @@ def test_environment_injection(self, tmp_path): temp_file = tmp_path / "tmp_file" temp_file.write_text("Moo Cow", "utf-8") - file_source.write_from("/moo", str(temp_file)) + actual_path = file_source.write_from("/moo", str(temp_file)) + assert "/moo" == actual_path assert expected_target.exists() assert (expected_target / "moo").exists() @@ -439,7 +441,8 @@ def test_environment_defaults(self, tmp_path): temp_file = tmp_path / "tmp_file" temp_file.write_text("Moo Cow", "utf-8") - file_source.write_from("/moo", str(temp_file)) + actual_path = file_source.write_from("/moo", str(temp_file)) + assert "/moo" == actual_path assert expected_target.exists() assert (expected_target / "moo").exists() diff --git a/test/unit/files/_util.py b/test/unit/files/_util.py index 2bf1274fcee5..0a4a77ee14ff 100644 --- a/test/unit/files/_util.py +++ b/test/unit/files/_util.py @@ -149,12 +149,12 @@ def write_from( uri: str, content: str, user_context: OptionalUserContext = None, -): +) -> str: file_source_path = file_sources.get_file_source_path(uri) with tempfile.NamedTemporaryFile(mode="w") as f: f.write(content) f.flush() - file_source_path.file_source.write_from(file_source_path.path, f.name, user_context=user_context) + return file_source_path.file_source.write_from(file_source_path.path, f.name, user_context=user_context) def configured_file_sources(conf_file, file_sources_config: Optional[FileSourcePluginsConfig] = None): @@ -174,14 +174,14 @@ def assert_can_write_and_read_to_conf(conf: dict): file_source_id = conf["id"] file_sources = configured_file_sources([conf]) test_uri = f"gxfiles://{file_source_id}/{test_filename}" - write_from( + actual_uri = write_from( file_sources, test_uri, test_contents, ) assert_realizes_contains( file_sources, - test_uri, + actual_uri, test_contents, )