From e4f4731e086bcbd4a42cafa77930077a279f997d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 3 Feb 2021 10:53:05 +0100 Subject: [PATCH 1/5] Don't flush for each failed output dataset I noticed that main's job handler is spending a lot of time here: ``` Thread 21942 (active+gil): "SlurmRunner.work_thread-1" _remove_snapshot (sqlalchemy/orm/session.py:396) commit (sqlalchemy/orm/session.py:514) _flush (sqlalchemy/orm/session.py:2674) flush (sqlalchemy/orm/session.py:2536) do (sqlalchemy/orm/scoping.py:163) fail (galaxy/jobs/__init__.py:1314) fail_job (galaxy/jobs/runners/__init__.py:474) run_next (galaxy/jobs/runners/__init__.py:136) run (threading.py:864) _bootstrap_inner (threading.py:916) _bootstrap (threading.py:884) ``` We flush just a few lines below anyway, so this flush shouldn't be needed and could have a big impact if there are a lot of output datasets. --- lib/galaxy/jobs/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 3ab20ea94e6..6a50aa34d81 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1310,8 +1310,6 @@ class JobWrapper(HasResourceParameters): # Pause any dependent jobs (and those jobs' outputs) for dep_job_assoc in dataset.dependent_jobs: self.pause(dep_job_assoc.job, "Execution of this dataset's job is paused because its input datasets are in an error state.") - self.sa_session.add(dataset) - self.sa_session.flush() job.set_final_state(job.states.ERROR) job.command_line = unicodify(self.command_line) job.info = message From 30769dc71d7086123277aafc421118f09418167e Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 4 Feb 2021 13:34:46 +0000 Subject: [PATCH 2/5] Raise correct exceptions in `MetadataCollection` Issue found by @mvdbeek when testing https://github.com/galaxyproject/tools-iuc/blob/master/tools/jbrowse/jbrowse.xml with `planemo test`. Introduced in commit 335a89772012041538e86835a4e16f3a2e50d51f . --- lib/galaxy/model/metadata.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/metadata.py b/lib/galaxy/model/metadata.py index b570f05af54..a3f40dd11be 100644 --- a/lib/galaxy/model/metadata.py +++ b/lib/galaxy/model/metadata.py @@ -94,7 +94,7 @@ class MetadataCollection(Mapping): try: return self.__getattr__(key) except Exception: - return KeyError + raise KeyError def __len__(self): return len(self.spec) @@ -113,6 +113,7 @@ class MetadataCollection(Mapping): return self.spec[name].wrap(self.spec[name].default, object_session(self.parent)) if name in self.parent._metadata: return self.parent._metadata[name] + raise AttributeError def __setattr__(self, name, value): if name == "parent": From 5176a67a71a655845323ef9b37876901aa20f138 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 4 Feb 2021 13:38:49 +0000 Subject: [PATCH 3/5] Do not cast `value` to `str` when handling exception that may have been generated by `str(value)`. --- lib/galaxy/util/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index ce9f4c0260c..4637f00671d 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -1041,8 +1041,9 @@ def unicodify(value, encoding=DEFAULT_ENCODING, error='replace', strip_null=Fals if not isinstance(value, str): value = str(value, encoding, error) except Exception as e: - msg = "Value '{}' could not be coerced to Unicode: {}('{}')".format(value, type(e).__name__, e) - raise Exception(msg) + msg = "Value '{}' could not be coerced to Unicode: {}('{}')".format(repr(value), type(e).__name__, e) + log.exception(msg) + raise if strip_null: return value.replace('\0', '') return value From 6f365172d4c03d0e0ba4dd1e2848af8ac16a029f Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 4 Feb 2021 13:54:57 +0000 Subject: [PATCH 4/5] Include `wrapped_class_name` in warning --- lib/galaxy/util/object_wrapper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/util/object_wrapper.py b/lib/galaxy/util/object_wrapper.py index 341e10f9426..ada091e9bef 100644 --- a/lib/galaxy/util/object_wrapper.py +++ b/lib/galaxy/util/object_wrapper.py @@ -129,7 +129,7 @@ def wrap_with_safe_string(value, no_wrap_classes=None): if value_mod: wrapped_class_name = f"{value_mod.__name__}.{wrapped_class_name}" wrapped_class_name = "SafeStringWrapper({}:{})".format(wrapped_class_name, ",".join(sorted(map(str, no_wrap_classes)))) - do_wrap_func_name = "__do_wrap_%s" % (wrapped_class_name) + do_wrap_func_name = f"__do_wrap_{wrapped_class_name}" do_wrap_func = __do_wrap global_dict = globals() if wrapped_class_name in global_dict: @@ -141,7 +141,7 @@ def wrap_with_safe_string(value, no_wrap_classes=None): wrapped_class = type(wrapped_class_name, (safe_class, wrapped_class, ), {}) except TypeError as e: # Fail-safe for when a class cannot be dynamically subclassed. - log.warning("Unable to create dynamic subclass for %s, %s: %s", type(value), value, e) + log.warning(f"Unable to create dynamic subclass {wrapped_class_name} for {type(value)}, {value}: {e}") wrapped_class = type(wrapped_class_name, (safe_class, ), {}) if wrapped_class not in (SafeStringWrapper, CallableSafeStringWrapper): # Save this wrapper for reuse and pickling/copying From 036bedf58dc12e855f9335c079619db95df1f969 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 4 Feb 2021 16:44:16 +0000 Subject: [PATCH 5/5] Don't cancel other builds if one fails --- .github/workflows/integration.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 30c3e1f8174..9bcdcf5ed8b 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -8,6 +8,7 @@ jobs: name: Test runs-on: ubuntu-18.04 strategy: + fail-fast: false matrix: python-version: [3.7] subset: ['upload_datatype', 'extended_metadata', 'kubernetes', 'not (upload_datatype or extended_metadata or kubernetes)']