From ef23f06a558774335059bb1e2b7b65944370044c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 19 Aug 2026 11:13:38 -0400 Subject: [PATCH] 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) --- lib/galaxy/job_execution/actions/post.py | 8 ++++-- lib/galaxy_test/api/test_workflows.py | 36 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/job_execution/actions/post.py b/lib/galaxy/job_execution/actions/post.py index 79112ae538e..08616ea070b 100644 --- a/lib/galaxy/job_execution/actions/post.py +++ b/lib/galaxy/job_execution/actions/post.py @@ -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) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index a74e69b29eb..eb5a8a07df0 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -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: