mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Add discarded_data option to model store import API
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.
This commit is contained in:
@@ -80,6 +80,7 @@ export async function reimportHistoryFromRecord(record: ExportRecord) {
|
||||
body: {
|
||||
store_content_uri: record.importUri,
|
||||
model_store_format: record.modelStoreFormat,
|
||||
discarded_data: "forbid",
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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: {
|
||||
/**
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user