Only defer ChangeDatatypeAction for collections that are still empty

The loop over a collection's dataset_instances ends in an else with no break
anywhere, so the else runs on every pass rather than only when the collection is
empty. Every job with an output collection therefore gets its element datatypes
changed and a redundant PostJobActionAssociation persisted.

Check whether the collection has elements instead, which is what the else comment
describes.

The deferral itself is load-bearing and stays. Job.get_change_datatype_actions
filters on action_type only, not immediate_actions, so the association reaches
discovery as ext_override - that is how a dynamic collection's elements get the
new extension. Those collections are precreated with ELEMENTS_UNINITIALIZED and so
still defer. Only the redundant association for an already populated collection
goes away.

That association is inert and invisible to the API: both job finish paths skip
immediate_actions, ext_override is keyed by output name so a structured
collection's entry never matches a discovered output, and no API exposes a job's
actions. So nothing here goes red on this change.

test_change_datatype_static_collection_output passes either side of it. It fills a
neighbouring gap - it covers the branch this restructures, a collection structured
up front whose elements already exist, which nothing exercised before. The
deferral half already had test_change_datatype_discovered_outputs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
John Chilton
2026-08-19 11:13:38 -04:00
parent 8bd272a050
commit ef23f06a55
2 changed files with 41 additions and 3 deletions
+5 -3
View File
@@ -133,9 +133,11 @@ class ChangeDatatypeAction(DefaultJobAction):
return
for dataset_collection_assoc in job.output_dataset_collection_instances:
if action.output_name == "" or dataset_collection_assoc.name == action.output_name:
for dataset_instance in dataset_collection_assoc.dataset_collection_instance.dataset_instances:
if dataset_instance:
app.datatypes_registry.change_datatype(dataset_instance, action.action_arguments["newtype"])
dataset_instances = dataset_collection_assoc.dataset_collection_instance.dataset_instances
if dataset_instances:
for dataset_instance in dataset_instances:
if dataset_instance:
app.datatypes_registry.change_datatype(dataset_instance, action.action_arguments["newtype"])
else:
# dynamic collection, add as PJA
pjaa = PostJobActionAssociation(action, job)
+36
View File
@@ -5908,6 +5908,42 @@ test_data:
)
assert details["elements"][0]["object"]["file_ext"] == "csv"
@skip_without_tool("collection_creates_pair")
def test_change_datatype_static_collection_output(self):
# The collection here is structured up front, so its elements already exist when the
# action runs - the counterpart to test_change_datatype_discovered_outputs, where they
# do not and the action has to be deferred to discovery instead.
with self.dataset_populator.test_history() as history_id:
jobs_summary = self._run_workflow(
"""
class: GalaxyWorkflow
inputs:
input: data
steps:
split:
tool_id: collection_creates_pair
in:
input1: input
out:
paired_output:
change_datatype: csv
outputs:
output:
outputSource: split/paired_output
test_data:
input: "1\n2\n3\n4"
""",
history_id=history_id,
)
inv = self.workflow_populator.get_invocation(jobs_summary.invocation_id, step_details=True)
details = self.dataset_populator.get_history_collection_details(
history_id=history_id, content_id=inv["output_collections"]["output"]["id"]
)
# Both, so that changing only the first element would still fail.
forward, reverse = details["elements"]
assert forward["object"]["file_ext"] == "csv"
assert reverse["object"]["file_ext"] == "csv"
@skip_without_tool("collection_type_source_map_over")
def test_mapping_and_subcollection_mapping(self):
with self.dataset_populator.test_history() as history_id: