Merge branch 'release_23.1' into dev

client/src/components/Markdown/Elements/HistoryDatasetCollection/CollectionDisplay.vue

@@ -86,9 +86,7 @@ export default {
             return collection && collection.name;
         },
         itemUrl() {
-            const collectionId = this.args.history_dataset_collection_id;
-            const collection = this.collections[collectionId];
-            return collection.url;
+            return `${getAppRoot()}api/dataset_collections/${this.args.history_dataset_collection_id}`;
         },
         downloadUrl() {
             return `${getAppRoot()}api/dataset_collections/${this.args.history_dataset_collection_id}/download`;

client/src/components/Notifications/Categories/SharedItemNotification.vue

@@ -7,6 +7,7 @@ import { computed } from "vue";
 
 import type { SharedItemNotification } from "@/components/Notifications";
 import { useNotificationsStore } from "@/stores/notificationsStore";
+import { absPath } from "@/utils/redirect";
 
 import Heading from "@/components/Common/Heading.vue";
 import NotificationActions from "@/components/Notifications/NotificationActions.vue";
@@ -43,9 +44,12 @@ const notificationVariant = computed(() => {
     }
 });
 
-function onClick(link: string) {
+const sharedItemUrl = computed(() => {
+    return absPath(content.value.slug);
+});
+
+function markNotificationAsSeen() {
     notificationsStore.updateNotification(props.notification, { seen: true });
-    window.open(link, "_blank");
 }
 </script>
 
@@ -67,7 +71,9 @@ function onClick(link: string) {
                     v-b-tooltip.bottom
                     :title="`View ${content.item_type} in new tab`"
                     class="text-primary"
-                    @click="onClick(content.slug)">
+                    :href="sharedItemUrl"
+                    target="_blank"
+                    @click="markNotificationAsSeen()">
                     {{ content.item_name }}
                     <FontAwesomeIcon icon="external-link-alt" />
                 </BLink>

lib/galaxy/jobs/__init__.py

@@ -1025,6 +1025,7 @@ class MinimalJobWrapper(HasResourceParameters):
                 self.job_id,
                 metadata_strategy_override=self.metadata_strategy,
                 tool_id=self.tool.id,
+                tool_type=self.tool.tool_type,
             )
         return self.__external_output_metadata
 
@@ -1857,7 +1858,7 @@ class MinimalJobWrapper(HasResourceParameters):
             # the tasks failed. So include the stderr, stdout, and exit code:
             return fail()
 
-        extended_metadata = self.external_output_metadata.extended and not self.tool.tool_type == "interactive"
+        extended_metadata = self.external_output_metadata.extended
 
         # We collect the stderr from tools that write their stderr to galaxy.json
         tool_provided_metadata = self.get_tool_provided_job_metadata()

lib/galaxy/jobs/mapper.py

@@ -63,7 +63,7 @@ class JobRunnerMapper:
             module_name = job_config.dynamic_params["rules_module"]
             self.rules_module = importlib.import_module(module_name)
 
-    def __invoke_expand_function(self, expand_function, destination):
+    def __invoke_expand_function(self, expand_function, destination, resource_params_from_job_state=True):
         function_arg_names = getfullargspec(expand_function).args
         app = self.job_wrapper.app
         possible_args = {
@@ -110,7 +110,9 @@ class JobRunnerMapper:
             elif require_db and arg in db_param_mapping:
                 actual_args[arg] = db_param_mapping[arg]
             elif arg == "resource_params":
-                actual_args["resource_params"] = self.job_wrapper.get_resource_parameters(job)
+                actual_args["resource_params"] = (
+                    self.job_wrapper.get_resource_parameters(job) if resource_params_from_job_state else {}
+                )
             elif arg == "workflow_invocation_uuid":
                 param_values = job.raw_param_dict()
                 workflow_invocation_uuid = param_values.get("__workflow_invocation_uuid__", None)
@@ -203,7 +205,23 @@ class JobRunnerMapper:
         return self.__handle_rule(expand_function, destination)
 
     def __handle_rule(self, rule_function, destination):
-        job_destination = self.__invoke_expand_function(rule_function, destination)
+        try:
+            job_destination = self.__invoke_expand_function(rule_function, destination)
+        except Exception as e:
+            # Rules have varying quality and don't raise a consistent set of standard exceptions.
+            # so ... if we get an error here let's try again without resource params encoded
+            # in the job state. They're evil anyway.
+            try:
+                job_destination = self.__invoke_expand_function(
+                    rule_function, destination, resource_params_from_job_state=False
+                )
+            except Exception:
+                # raise original exception, dropping resource param from job state didn't help.
+                raise e
+            else:
+                log.warning(
+                    f"Ignored user-specified invalid resource parameter request because it failed with {str(e)}"
+                )
         if not isinstance(job_destination, galaxy.jobs.JobDestination):
             job_destination_rep = str(job_destination)  # Should be either id or url
             if "://" in job_destination_rep:
@@ -220,11 +238,16 @@ class JobRunnerMapper:
         if raw_job_destination is None:
             raw_job_destination = self.job_wrapper.tool.get_job_destination(params)
         if raw_job_destination.runner == DYNAMIC_RUNNER_NAME:
-            job_destination = self.__handle_dynamic_job_destination(raw_job_destination)
-            log.debug("(%s) Mapped job to destination id: %s", self.job_wrapper.job_id, job_destination.id)
-            # Recursively handle chained dynamic destinations
-            if job_destination.runner == DYNAMIC_RUNNER_NAME:
-                return self.__determine_job_destination(params, raw_job_destination=job_destination)
+            try:
+                job_destination = self.__handle_dynamic_job_destination(raw_job_destination)
+                log.debug("(%s) Mapped job to destination id: %s", self.job_wrapper.job_id, job_destination.id)
+                # Recursively handle chained dynamic destinations
+                if job_destination.runner == DYNAMIC_RUNNER_NAME:
+                    return self.__determine_job_destination(params, raw_job_destination=job_destination)
+            except AssertionError:
+                if params and "job_resource" in params:
+                    params = params.copy()
+                    del params["job_resource"]
         else:
             job_destination = raw_job_destination
             log.debug("(%s) Mapped job to destination id: %s", self.job_wrapper.job_id, job_destination.id)

lib/galaxy/metadata/__init__.py

@@ -37,11 +37,11 @@ except Exception:
 """
 
 
-def get_metadata_compute_strategy(config, job_id, metadata_strategy_override=None, tool_id=None):
+def get_metadata_compute_strategy(config, job_id, metadata_strategy_override=None, tool_id=None, tool_type=None):
     metadata_strategy = metadata_strategy_override or config.metadata_strategy
     if metadata_strategy == "legacy":
         raise Exception("legacy metadata_strategy has been removed")
-    elif "extended" in metadata_strategy and tool_id != "__SET_METADATA__":
+    elif "extended" in metadata_strategy and tool_id != "__SET_METADATA__" and tool_type != "interactive":
         return ExtendedDirectoryMetadataGenerator(job_id)
     else:
         return PortableDirectoryMetadataGenerator(job_id)

lib/galaxy/tool_util/deps/docker_util.py

@@ -4,6 +4,7 @@
 """
 import os
 import shlex
+import sys
 from typing import (
     List,
     Optional,
@@ -26,7 +27,7 @@ DEFAULT_NET = None
 DEFAULT_MEMORY = None
 DEFAULT_VOLUMES_FROM = None
 DEFAULT_AUTO_REMOVE = True
-DEFAULT_SET_USER = "$UID"
+DEFAULT_SET_USER = None if sys.platform == "darwin" else "$UID"
 DEFAULT_RUN_EXTRA_ARGUMENTS = None
 
 
@@ -106,7 +107,7 @@ def build_docker_run_command(
     sudo: bool = DEFAULT_SUDO,
     sudo_cmd: str = DEFAULT_SUDO_COMMAND,
     auto_rm: bool = DEFAULT_AUTO_REMOVE,
-    set_user: str = DEFAULT_SET_USER,
+    set_user: Optional[str] = DEFAULT_SET_USER,
     host: Optional[str] = DEFAULT_HOST,
     guest_ports: Union[bool, List[str]] = False,
     container_name: Optional[str] = None,

lib/galaxy/webapps/galaxy/api/__init__.py

@@ -216,9 +216,7 @@ class GalaxyASGIRequest(GalaxyAbstractRequest):
 
     @property
     def host(self) -> str:
-        client = self.__request.client
-        assert client is not None
-        return str(client.host)
+        return self.__request.base_url.netloc
 
     @property
     def environ(self) -> MutableMapping[str, Any]:

lib/galaxy/workflow/modules.py

@@ -122,6 +122,71 @@ class ConditionalStepWhen(BooleanToolParameter):
     pass
 
 
+def to_cwl(value, hda_references, step):
+    element_identifier = None
+    if isinstance(value, model.DatasetCollectionElement) and value.hda:
+        element_identifier = value.element_identifier
+        value = value.hda
+    if isinstance(value, model.HistoryDatasetAssociation):
+        # I think the following two checks are needed but they may
+        # not be needed.
+        if not value.dataset.in_ready_state():
+            why = "dataset [%s] is needed for valueFrom expression and is non-ready" % value.id
+            raise DelayedWorkflowEvaluation(why=why)
+        if not value.is_ok:
+            raise FailWorkflowEvaluation(
+                why=InvocationFailureDatasetFailed(
+                    reason=FailureReason.dataset_failed, hda_id=value.id, workflow_step_id=step.id
+                )
+            )
+        if value.ext == "expression.json":
+            with open(value.file_name) as f:
+                # OUR safe_loads won't work, will not load numbers, etc...
+                return json.load(f)
+        else:
+            hda_references.append(value)
+            properties = {
+                "class": "File",
+                "location": "step_input://%d" % len(hda_references),
+            }
+            set_basename_and_derived_properties(
+                properties, value.dataset.created_from_basename or element_identifier or value.name
+            )
+            return properties
+    elif hasattr(value, "collection"):
+        collection = value.collection
+        if collection.collection_type == "list":
+            return [to_cwl(dce, hda_references=hda_references, step=step) for dce in collection.dataset_elements]
+        else:
+            # Could be record or nested lists
+            rval = {}
+            for element in collection.elements:
+                rval[element.element_identifier] = to_cwl(
+                    element.element_object, hda_references=hda_references, step=step
+                )
+            return rval
+    elif isinstance(value, list):
+        return [to_cwl(v, hda_references=hda_references, step=step) for v in value]
+    else:
+        return value
+
+
+def from_cwl(value, hda_references, progress):
+    # TODO: turn actual files into HDAs here ... somehow I suppose. Things with
+    # file:// locations for instance.
+    if isinstance(value, dict) and "class" in value and "location" in value:
+        if value["class"] == "File":
+            # This is going to re-file -> HDA this each iteration I think, not a good
+            # implementation.
+            return progress.raw_to_galaxy(value)
+        assert value["location"].startswith("step_input://"), "Invalid location %s" % value
+        return hda_references[int(value["location"][len("step_input://") :]) - 1]
+    elif isinstance(value, dict):
+        raise NotImplementedError()
+    else:
+        return value
+
+
 def evaluate_value_from_expressions(progress, step, execution_state, extra_step_state):
     when_expression = step.when_expression
     value_from_expressions = {}
@@ -135,73 +200,14 @@ def evaluate_value_from_expressions(progress, step, execution_state, extra_step_
     if not value_from_expressions and when_expression is None:
         return {}
 
-    hda_references = []
-
-    def to_cwl(value):
-        element_identifier = None
-        if isinstance(value, model.DatasetCollectionElement) and value.hda:
-            element_identifier = value.element_identifier
-            value = value.hda
-        if isinstance(value, model.HistoryDatasetAssociation):
-            # I think the following two checks are needed but they may
-            # not be needed.
-            if not value.dataset.in_ready_state():
-                why = "dataset [%s] is needed for valueFrom expression and is non-ready" % value.id
-                raise DelayedWorkflowEvaluation(why=why)
-            if not value.is_ok:
-                raise FailWorkflowEvaluation(
-                    why=InvocationFailureDatasetFailed(
-                        reason=FailureReason.dataset_failed, hda_id=value.id, workflow_step_id=step.id
-                    )
-                )
-            if value.ext == "expression.json":
-                with open(value.file_name) as f:
-                    # OUR safe_loads won't work, will not load numbers, etc...
-                    return json.load(f)
-            else:
-                hda_references.append(value)
-                properties = {
-                    "class": "File",
-                    "location": "step_input://%d" % len(hda_references),
-                }
-                set_basename_and_derived_properties(
-                    properties, value.dataset.created_from_basename or element_identifier or value.name
-                )
-                return properties
-        elif hasattr(value, "collection"):
-            collection = value.collection
-            if collection.collection_type == "list":
-                return [to_cwl(dce) for dce in collection.dataset_elements]
-            else:
-                # Could be record or nested lists
-                rval = {}
-                for element in collection.elements:
-                    rval[element.element_identifier] = to_cwl(element.element_object)
-                return rval
-        else:
-            return value
-
-    def from_cwl(value):
-        # TODO: turn actual files into HDAs here ... somehow I suppose. Things with
-        # file:// locations for instance.
-        if isinstance(value, dict) and "class" in value and "location" in value:
-            if value["class"] == "File":
-                # This is going to re-file -> HDA this each iteration I think, not a good
-                # implementation.
-                return progress.raw_to_galaxy(value)
-            assert value["location"].startswith("step_input://"), "Invalid location %s" % value
-            return hda_references[int(value["location"][len("step_input://") :]) - 1]
-        elif isinstance(value, dict):
-            raise NotImplementedError()
-        else:
-            return value
+    hda_references: List[model.HistoryDatasetAssociation] = []
 
     step_state = {}
     for key, value in extra_step_state.items():
-        step_state[key] = to_cwl(value)
+        step_state[key] = to_cwl(value, hda_references=hda_references, step=step)
     if execution_state:
         for key, value in execution_state.inputs.items():
-            step_state[key] = to_cwl(value)
+            step_state[key] = to_cwl(value, hda_references=hda_references, step=step)
 
     if when_expression is not None:
         if when_expression == "${inputs.when}":
@@ -229,7 +235,7 @@ def evaluate_value_from_expressions(progress, step, execution_state, extra_step_
                     reason=FailureReason.expression_evaluation_failed, workflow_step_id=step.id
                 )
             )
-        when_value = from_cwl(as_cwl_value)
+        when_value = from_cwl(as_cwl_value, hda_references=hda_references, progress=progress)
         if not isinstance(when_value, bool):
             raise FailWorkflowEvaluation(
                 InvocationFailureWhenNotBoolean(

test/integration/dummy_job_resource_parameters.xml

@@ -0,0 +1,7 @@
+<parameters>
+    <param label="Compute Resource" name="multi_compute_resource" type="select"
+        help="Need help selecting a compute resource? Options and limits are explained in detail &lt;a href='https://galaxyproject.org/main/' target='_blank'&gt;in the Galaxy Hub&lt;/a&gt;">
+        <option value="does not matter">Galaxy cluster</option>
+        <option value="or this">Jetstream 2</option>
+    </param>
+</parameters>

test/integration/job_resource_error_recovery_job_conf.yml

@@ -0,0 +1,22 @@
+# job config that fails submission if job resource parameters are requested
+runners:
+  local:
+    load: galaxy.jobs.runners.local:LocalJobRunner
+    workers: 1
+  dynamic:
+    rules_module: integration.job_resource_rules
+
+execution:
+  default: initial_destination
+  environments:
+    initial_destination:
+      runner: dynamic
+      type: python
+      function: local_or_exception
+    local:
+      runner: local
+
+tools:
+  - class: local
+    environment: local
+    resources: upload

test/integration/job_resource_rules/fail_if_job_resource_present.py

@@ -0,0 +1,5 @@
+def local_or_exception(resource_params):
+    """Build environment that fails if resource_params are passed."""
+    if resource_params:
+        raise Exception("boo!")
+    return "local"

test/integration/test_job_resource_error_recovery.py

@@ -0,0 +1,43 @@
+import os
+
+from galaxy_test.base.populators import (
+    DatasetPopulator,
+    WorkflowPopulator,
+)
+from galaxy_test.driver import integration_util
+
+SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__))
+JOB_RESOURCES_CONFIG_FILE = os.path.join(SCRIPT_DIRECTORY, "dummy_job_resource_parameters.xml")
+JOB_CONFIG = os.path.join(SCRIPT_DIRECTORY, "job_resource_error_recovery_job_conf.yml")
+
+
+class TestJobRecoveryBeforeHandledIntegration(integration_util.IntegrationTestCase):
+    dataset_populator: DatasetPopulator
+    framework_tool_and_types = True
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.dataset_populator = DatasetPopulator(self.galaxy_interactor)
+        self.workflow_populator = WorkflowPopulator(self.galaxy_interactor)
+
+    @classmethod
+    def handle_galaxy_config_kwds(cls, config) -> None:
+        super().handle_galaxy_config_kwds(config)
+        config["job_config_file"] = JOB_CONFIG
+        config["job_resource_params_file"] = JOB_RESOURCES_CONFIG_FILE
+
+    def test_recovers_from_job_resource_errors(self):
+        with self.dataset_populator.test_history() as history_id:
+            self.workflow_populator.run_workflow(
+                """
+class: GalaxyWorkflow
+steps:
+  simple_step:
+    tool_id: create_2
+    tool_state:
+      __job_resource:
+        __job_resource__select: 'yes'
+        cores: '32'
+""",
+                history_id=history_id,
+            )

test/unit/workflows/test_modules.py

@@ -249,6 +249,18 @@ def test_subworkflow_new_outputs():
     assert output2["name"] == "4:out_file1", output2["name"]
 
 
+def test_to_cwl():
+    hda = model.HistoryDatasetAssociation(create_dataset=True, flush=False)
+    hda.dataset.state = model.Dataset.states.OK
+    hdas = [hda]
+    hda_references = []
+    result = modules.to_cwl(hdas, hda_references, model.WorkflowStep())
+    assert isinstance(result, list)
+    assert len(result) == 1
+    assert result[0]["class"] == "File"
+    assert hda_references == hdas
+
+
 class MapOverTestCase(NamedTuple):
     data_input: str
     step_input_def: Union[str, List[str]]