Merge branch 'release_24.0' into dev

lib/galaxy/celery/tasks.py

@@ -291,6 +291,7 @@ def abort_when_job_stops(function: Callable, session: galaxy_scoped_session, job
                 return future.result(timeout=1)
             except TimeoutError:
                 if is_aborted(session, job_id):
+                    future.cancel()
                     return
 
 

lib/galaxy/exceptions/__init__.py

@@ -16,6 +16,8 @@
 and messages.
 """
 
+from typing import Optional
+
 from .error_codes import (
     error_codes_by_name,
     ErrorCode,
@@ -30,7 +32,7 @@ class MessageException(Exception):
     # Error code information embedded into API json responses.
     err_code: ErrorCode = error_codes_by_name["UNKNOWN"]
 
-    def __init__(self, err_msg=None, type="info", **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", **extra_error_info):
         self.err_msg = err_msg or self.err_code.default_error_message
         self.type = type
         self.extra_error_info = extra_error_info
@@ -64,7 +66,7 @@ class AcceptedRetryLater(MessageException):
     err_code = error_codes_by_name["ACCEPTED_RETRY_LATER"]
     retry_after: int
 
-    def __init__(self, msg, retry_after=60):
+    def __init__(self, msg: Optional[str] = None, retry_after=60):
         super().__init__(msg)
         self.retry_after = retry_after
 
@@ -136,7 +138,7 @@ class ToolMissingException(MessageException):
     status_code = 400
     err_code = error_codes_by_name["USER_TOOL_MISSING_PROBLEM"]
 
-    def __init__(self, err_msg=None, type="info", tool_id=None, **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", tool_id=None, **extra_error_info):
         super().__init__(err_msg, type, **extra_error_info)
         self.tool_id = tool_id
 
@@ -152,7 +154,7 @@ class ToolInputsNotReadyException(MessageException):
 
 
 class ToolInputsNotOKException(MessageException):
-    def __init__(self, err_msg=None, type="info", *, src: str, id: int, **extra_error_info):
+    def __init__(self, err_msg: Optional[str] = None, type="info", *, src: str, id: int, **extra_error_info):
         super().__init__(err_msg, type, **extra_error_info)
         self.src = src
         self.id = id

lib/galaxy/tools/__init__.py

@@ -3356,9 +3356,15 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history
         if how == "first":
             extracted_element = collection.first_dataset_element
         elif how == "by_identifier":
-            extracted_element = collection[incoming["which"]["identifier"]]
+            try:
+                extracted_element = collection[incoming["which"]["identifier"]]
+            except KeyError as e:
+                raise exceptions.MessageException(e.args[0])
         elif how == "by_index":
-            extracted_element = collection[int(incoming["which"]["index"])]
+            try:
+                extracted_element = collection[int(incoming["which"]["index"])]
+            except KeyError as e:
+                raise exceptions.MessageException(e.args[0])
         else:
             raise exceptions.MessageException("Invalid tool parameters.")
         extracted = extracted_element.element_object

lib/galaxy/tools/extract_dataset.xml

@@ -19,7 +19,7 @@
       </param>
       <when value="first" />
       <when value="by_identifier">
-        <param name="identifier" label="Element identifier:" type="text">
+        <param name="identifier" label="Element identifier:" type="text" optional="false">
           <sanitizer invalid_char="">
               <valid initial="string.ascii_letters,string.digits">
                   <add value="_" />
@@ -52,9 +52,9 @@ Description
 
 The tool allow extracting datasets based on position (**The first dataset** and **Select by index** options) or name (**Select by element identifier** option). This tool effectively collapses the inner-most collection into a dataset. For nested collections (e.g a list of lists of lists: outer:middle:inner, extracting the inner dataset element) a new list is created where the selected element takes the position of the inner-most collection (so outer:middle, where middle is not a collection but the inner dataset element).
 
-.. class:: warningmark 
+.. class:: warningmark
 
-**Note**: Dataset index (numbering) begins with 0 (zero). 
+**Note**: Dataset index (numbering) begins with 0 (zero).
 
 .. class:: infomark
 

lib/galaxy/webapps/base/api.py

@@ -198,6 +198,11 @@ async def validate_exception_middleware(request: Request, exc: RequestValidation
 
     @app.exception_handler(MessageException)
     async def message_exception_middleware(request: Request, exc: MessageException) -> Response:
+        # Intentionally not logging traceback here as the full context will be
+        # dispatched to Sentry if configured.  This just makes logs less opaque
+        # when one sees a 500.
+        if exc.status_code >= 500:
+            log.info(f"MessageException: {exc}")
         return get_error_response_for_request(request, exc)
 
 

lib/galaxy/workflow/modules.py

@@ -2573,11 +2573,13 @@ def populate_module_and_state(
         step_args = param_map.get(step.id, {})
         step_errors = module_injector.compute_runtime_state(step, step_args=step_args)
         if step_errors:
-            raise exceptions.MessageException(step_errors, err_data={step.order_index: step_errors})
+            raise exceptions.MessageException(
+                "Error computing workflow step runtime state", err_data={step.order_index: step_errors}
+            )
         if step.upgrade_messages:
             if allow_tool_state_corrections:
                 log.debug('Workflow step "%i" had upgrade messages: %s', step.id, step.upgrade_messages)
             else:
                 raise exceptions.MessageException(
-                    step.upgrade_messages, err_data={step.order_index: step.upgrade_messages}
+                    "Workflow step has upgrade messages", err_data={step.order_index: step.upgrade_messages}
                 )

lib/galaxy_test/api/test_tools.py

@@ -701,23 +701,42 @@ def test_database_operation_tool_with_pending_inputs(self):
             hdca1_id = self.dataset_collection_populator.create_list_in_history(
                 history_id, contents=["a\nb\nc\nd", "e\nf\ng\nh"], wait=True
             ).json()["outputs"][0]["id"]
-            self.dataset_populator.run_tool(
+            run_response = self.dataset_populator.run_tool(
                 tool_id="cat_data_and_sleep",
                 inputs={
                     "sleep_time": 15,
                     "input1": {"batch": True, "values": [{"src": "hdca", "id": hdca1_id}]},
                 },
                 history_id=history_id,
             )
+            output_hdca_id = run_response["implicit_collections"][0]["id"]
             run_response = self.dataset_populator.run_tool(
                 tool_id="__EXTRACT_DATASET__",
                 inputs={
-                    "data_collection": {"src": "hdca", "id": hdca1_id},
+                    "data_collection": {"src": "hdca", "id": output_hdca_id},
                 },
                 history_id=history_id,
             )
             assert run_response["outputs"][0]["state"] != "ok"
 
+    @skip_without_tool("__EXTRACT_DATASET__")
+    def test_extract_dataset_invalid_element_identifier(self):
+        with self.dataset_populator.test_history(require_new=False) as history_id:
+            hdca1_id = self.dataset_collection_populator.create_list_in_history(
+                history_id, contents=["a\nb\nc\nd", "e\nf\ng\nh"], wait=True
+            ).json()["outputs"][0]["id"]
+            run_response = self.dataset_populator.run_tool_raw(
+                tool_id="__EXTRACT_DATASET__",
+                inputs={
+                    "data_collection": {"src": "hdca", "id": hdca1_id},
+                    "which": {"which_dataset": "by_index", "index": 100},
+                },
+                history_id=history_id,
+                input_format="21.01",
+            )
+            assert run_response.status_code == 400
+            assert run_response.json()["err_msg"] == "Dataset collection has no element_index with key 100."
+
     @skip_without_tool("__FILTER_FAILED_DATASETS__")
     def test_filter_failed_list(self):
         with self.dataset_populator.test_history(require_new=False) as history_id: