Merge pull request #19154 from kysrpex/file_sources_assign_paths

Let file sources choose a path for uploaded files

Author: davelopez

client/src/api/schema/schema.ts

@@ -8935,6 +8935,8 @@ export interface components {
             error?: string | null;
             /** Success */
             success: boolean;
+            /** Uri */
+            uri?: string | null;
         };
         /**
          * ExportObjectType

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() {

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(

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):