From b8d6eba1185d9fab88ff52afe1bec5d813e6049e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 14 Jan 2026 15:44:16 +0100 Subject: [PATCH] Add discarded_data option to model store import API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit and change default from `FORCE` to `ALLOW`. When importing an invocation via POST /api/invocations/from_store with a model store that includes files (include_files=True), the datasets would end up in 'discarded' state with size 0 instead of having their actual content imported. The root cause was that create_objects_from_store() used ImportDiscardedDataType.FORCE which forces all datasets to be discarded regardless of whether file data is available in the store. This was originally added for the DEFERRED dataset feature but import_model_store was later updated to use the default FORBID mode. This change aligns create_objects_from_store with import_model_store behavior. You can now choose from these options: - ALLOW: datasets without data → discarded but not deleted (new default, mirrors source structure) - FORBID: datasets without data → discarded AND deleted - FORCE: all datasets → discarded regardless of data availability (useful if only metadata is needed) Add test to verify reimported invocation datasets have 'ok' state. --- client/src/api/histories.export.ts | 1 + client/src/api/schema/schema.ts | 30 ++++++++++++ .../History/useHistoryCardActions.ts | 1 + lib/galaxy/managers/model_stores.py | 12 +++-- lib/galaxy/schema/schema.py | 18 +++++++ lib/galaxy_test/api/test_workflows.py | 48 +++++++++++++++++++ lib/galaxy_test/base/populators.py | 10 +++- .../selenium/test_history_dataset_state.py | 1 + 8 files changed, 116 insertions(+), 5 deletions(-) diff --git a/client/src/api/histories.export.ts b/client/src/api/histories.export.ts index f1ab9386605..72b4fec3aea 100644 --- a/client/src/api/histories.export.ts +++ b/client/src/api/histories.export.ts @@ -80,6 +80,7 @@ export async function reimportHistoryFromRecord(record: ExportRecord) { body: { store_content_uri: record.importUri, model_store_format: record.modelStoreFormat, + discarded_data: "forbid", }, }); diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index c234ecfc638..278007e6cb3 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -8671,6 +8671,12 @@ export interface components { }; /** CreateHistoryContentFromStore */ CreateHistoryContentFromStore: { + /** + * Discarded Data + * @description How to handle datasets with unavailable data. 'forbid': mark as deleted, 'allow': import as discarded but not deleted, 'force': import all datasets as discarded regardless of whether file data is available (useful for importing metadata only). + * @default allow + */ + discarded_data: components["schemas"]["DiscardedDataType"]; model_store_format?: components["schemas"]["ModelStoreFormat"] | null; /** Store Content Uri */ store_content_uri?: string | null; @@ -8772,6 +8778,12 @@ export interface components { }; /** CreateHistoryFromStore */ CreateHistoryFromStore: { + /** + * Discarded Data + * @description How to handle datasets with unavailable data. 'forbid': mark as deleted, 'allow': import as discarded but not deleted, 'force': import all datasets as discarded regardless of whether file data is available (useful for importing metadata only). + * @default allow + */ + discarded_data: components["schemas"]["DiscardedDataType"]; model_store_format?: components["schemas"]["ModelStoreFormat"] | null; /** Store Content Uri */ store_content_uri?: string | null; @@ -8803,6 +8815,12 @@ export interface components { }; /** CreateInvocationsFromStorePayload */ CreateInvocationsFromStorePayload: { + /** + * Discarded Data + * @description How to handle datasets with unavailable data. 'forbid': mark as deleted, 'allow': import as discarded but not deleted, 'force': import all datasets as discarded regardless of whether file data is available (useful for importing metadata only). + * @default allow + */ + discarded_data: components["schemas"]["DiscardedDataType"]; /** * History ID * @description The ID of the history associated with the invocations. @@ -8841,6 +8859,12 @@ export interface components { }; /** CreateLibrariesFromStore */ CreateLibrariesFromStore: { + /** + * Discarded Data + * @description How to handle datasets with unavailable data. 'forbid': mark as deleted, 'allow': import as discarded but not deleted, 'force': import all datasets as discarded regardless of whether file data is available (useful for importing metadata only). + * @default allow + */ + discarded_data: components["schemas"]["DiscardedDataType"]; model_store_format?: components["schemas"]["ModelStoreFormat"] | null; /** Store Content Uri */ store_content_uri?: string | null; @@ -11114,6 +11138,12 @@ export interface components { | components["schemas"]["EmptyFieldParameterValidatorModel"] )[]; }; + /** + * DiscardedDataType + * @description Options for handling discarded datasets on import. + * @enum {string} + */ + DiscardedDataType: "forbid" | "allow" | "force"; /** DisconnectAction */ DisconnectAction: { /** diff --git a/client/src/components/History/useHistoryCardActions.ts b/client/src/components/History/useHistoryCardActions.ts index dbbcb3fa417..a98f9c9a317 100644 --- a/client/src/components/History/useHistoryCardActions.ts +++ b/client/src/components/History/useHistoryCardActions.ts @@ -130,6 +130,7 @@ export function useHistoryCardActions( body: { model_store_format: hti.export_record_data?.model_store_format, store_content_uri: hti.export_record_data?.target_uri, + discarded_data: "forbid", }, }); diff --git a/lib/galaxy/managers/model_stores.py b/lib/galaxy/managers/model_stores.py index 1f397a5c16e..a5d04ffdf0d 100644 --- a/lib/galaxy/managers/model_stores.py +++ b/lib/galaxy/managers/model_stores.py @@ -25,6 +25,7 @@ from galaxy.schema.schema import ( ExportObjectType, HistoryContentType, ShortTermStoreExportPayload, + StoreContentSource, WriteStoreToPayload, ) from galaxy.schema.tasks import ( @@ -325,17 +326,22 @@ class ModelStoreManager: def create_objects_from_store( app: MinimalManagerApp, galaxy_user: Optional[model.User], - payload, + payload: StoreContentSource, history: Optional[model.History] = None, for_library: bool = False, ) -> ObjectImportTracker: + # Note: Galaxy's base Model uses use_enum_values=True, so enum fields + # are stored as their string values after pydantic validation. import_options = ImportOptions( - discarded_data=ImportDiscardedDataType.FORCE, + discarded_data=ImportDiscardedDataType(payload.discarded_data), allow_library_creation=for_library, ) user_context = ModelStoreUserContext(app, galaxy_user) if galaxy_user is not None else None + source = payload.store_content_uri or payload.store_dict + if source is None: + raise RequestParameterInvalidException("Must provide store_content_uri or store_dict") model_import_store = source_to_import_store( - payload.store_content_uri or payload.store_dict, + source, app=app, import_options=import_options, model_store_format=payload.model_store_format, diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index cd2feced23b..97f767c2a9e 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -1861,10 +1861,28 @@ class ModelStoreFormat(str, Enum): return value in [cls.BAG_DOT_TAR, cls.BAG_DOT_TGZ, cls.BAG_DOT_ZIP] +class DiscardedDataType(str, Enum): + """Options for handling discarded datasets on import.""" + + # Don't allow discarded 'okay' datasets on import, datasets will be marked deleted. + FORBID = "forbid" + # Allow datasets to be imported as DISCARDED datasets that are not deleted if file data is unavailable. + ALLOW = "allow" + # Import all datasets as discarded regardless of whether file data is available in the store. + FORCE = "force" + + class StoreContentSource(Model): store_content_uri: Optional[str] = None store_dict: Optional[dict[str, Any]] = None model_store_format: Optional["ModelStoreFormat"] = None + discarded_data: DiscardedDataType = Field( + default=DiscardedDataType.ALLOW, + title="Discarded Data", + description="How to handle datasets with unavailable data. 'forbid': mark as deleted, " + "'allow': import as discarded but not deleted, 'force': import all datasets as discarded " + "regardless of whether file data is available (useful for importing metadata only).", + ) class CreateHistoryFromStore(StoreContentSource): diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 4abe6e23f8b..f64e2a4a5a9 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -3632,6 +3632,54 @@ input_1: workflow = crate.mainEntity assert workflow + @skip_without_tool("cat1") + def test_reimport_invocation_with_files(self): + """Test that reimporting an invocation with include_files=True preserves dataset state and content.""" + with self.dataset_populator.test_history() as history_id: + # Run a simple workflow + summary = self._run_workflow(WORKFLOW_SIMPLE, test_data={"input1": "hello world"}, history_id=history_id) + invocation_id = summary.invocation_id + self.workflow_populator.wait_for_invocation_and_jobs( + history_id=history_id, workflow_id=summary.workflow_id, invocation_id=invocation_id + ) + + # Export the invocation with files included + store_path = self.workflow_populator.download_invocation_to_store( + invocation_id, include_files=True, extension="tgz" + ) + + # Create a new history and import the invocation + with self.dataset_populator.test_history() as new_history_id: + imported_invocations = self.workflow_populator.create_invocation_from_store( + history_id=new_history_id, store_path=store_path + ) + assert len(imported_invocations) == 1 + imported_invocation_id = imported_invocations[0]["id"] + + # Get the full invocation details including outputs + imported_invocation = self.workflow_populator.get_invocation(imported_invocation_id) + + # Verify the imported invocation has output datasets + assert "outputs" in imported_invocation + assert "wf_output_1" in imported_invocation["outputs"] + output_id = imported_invocation["outputs"]["wf_output_1"]["id"] + + # Get the imported dataset and verify it has state 'ok' + dataset_details = self.dataset_populator.get_history_dataset_details( + new_history_id, dataset_id=output_id + ) + assert ( + dataset_details["state"] == "ok" + ), f"Expected dataset state 'ok', got '{dataset_details['state']}'" + + # Verify the content is correct + output_content = self.dataset_populator.get_history_dataset_content( + new_history_id, dataset_id=output_id + ) + assert ( + output_content.strip() == "hello world" + ), f"Expected content 'hello world', got '{output_content.strip()}'" + @skip_without_tool("__MERGE_COLLECTION__") def test_merge_collection_scheduling(self, history_id): summary = self._run_workflow( diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 54dddd241a4..8e5cdde38c2 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -681,13 +681,19 @@ class BaseDatasetPopulator(BasePopulator): return create_response def create_contents_from_store( - self, history_id: str, store_dict: Optional[dict[str, Any]] = None, store_path: Optional[str] = None + self, + history_id: str, + store_dict: Optional[dict[str, Any]] = None, + store_path: Optional[str] = None, + discarded_data: Optional[str] = None, ) -> list[dict[str, Any]]: if store_dict is not None: assert isinstance(store_dict, dict) if store_path is not None: assert isinstance(store_path, str) payload = _store_payload(store_dict=store_dict, store_path=store_path) + if discarded_data is not None: + payload["discarded_data"] = discarded_data create_response = self.create_contents_from_store_raw(history_id, payload) create_response.raise_for_status() return create_response.json() @@ -2253,7 +2259,7 @@ class BaseWorkflowPopulator(BasePopulator): store_dict: Optional[dict[str, Any]] = None, store_path: Optional[str] = None, model_store_format: Optional[str] = None, - ) -> Response: + ) -> list[dict[str, Any]]: create_response = self.create_invocation_from_store_raw( history_id, store_dict=store_dict, store_path=store_path, model_store_format=model_store_format ) diff --git a/lib/galaxy_test/selenium/test_history_dataset_state.py b/lib/galaxy_test/selenium/test_history_dataset_state.py index a3470e32ee3..bb8ab05183c 100644 --- a/lib/galaxy_test/selenium/test_history_dataset_state.py +++ b/lib/galaxy_test/selenium/test_history_dataset_state.py @@ -62,6 +62,7 @@ class TestHistoryDatasetState(SeleniumTestCase, UsesHistoryItemAssertions): self.dataset_populator.create_contents_from_store( history_id, store_dict=one_hda_model_store_dict(include_source=False), + discarded_data="force", ) # regression after 3/24/2022 - explicit refresh now required. self.home()