From 23c7346e177ff4de5d0c6d6cd5cb1ce6e881f4ed Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 13:00:58 -0700 Subject: [PATCH 01/20] Add a bunch of value resolving logging --- src/toil/cwl/cwltoil.py | 53 ++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 651d27e305..36ab03d09b 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -26,6 +26,7 @@ import json import logging import os +import pprint import shutil import socket import stat @@ -352,16 +353,26 @@ def __init__( def __repr__(self) -> str: """Allow for debug printing.""" - try: - return "ResolveSource(" + repr(self.resolve()) + ")" - except Exception: - return ( - f"ResolveSource({self.name}, {self.input}, {self.source_key}, " - f"{self.promise_tuples})" - ) + + parts = [f"source key {self.source_key}"] + + if "pickValue" in self.input: + parts.append(f"pick value {self.input['pickValue']} from") + + if isinstance(self.promise_tuples, list): + names = [n for n, _ in self.promise_tuples] + parts.append(f"names {names} in promises") + else: + name, _ = self.promise_tuples + parts.append(f"name {name} in promise") + + return f"ResolveSource({', '.join(parts)})" def resolve(self) -> Any: """First apply linkMerge then pickValue if either present.""" + + logger.debug("Resolving %s", self.name) + result: Optional[Any] = None if isinstance(self.promise_tuples, list): result = self.link_merge( @@ -385,8 +396,11 @@ def link_merge( :param values: result of step """ + link_merge_type = self.input.get("linkMerge", "merge_nested") + logger.debug("Applying linkMerge for %s type %s to:\n%s", self.name, link_merge_type, pprint.pformat(values)) + if link_merge_type == "merge_nested": return values @@ -412,8 +426,11 @@ def pick_value(self, values: Union[List[Union[str, SkipNull]], Any]) -> Any: without modification. :return: """ + pick_value_type = cast(str, self.input.get("pickValue")) + logger.debug("Applying pickValue for %s type %s to:\n%s", self.name, pick_value_type, pprint.pformat(values)) + if pick_value_type is None: return values @@ -428,6 +445,7 @@ def pick_value(self, values: Union[List[Union[str, SkipNull]], Any]) -> Any: if pick_value_type == "first_non_null": if len(result) < 1: + logger.error("Could not find non-null entry for %s:\n%s", self.name, pprint.pformat(self.promise_tuples)) raise cwl_utils.errors.WorkflowException( "%s: first_non_null operator found no non-null values" % self.name ) @@ -482,6 +500,13 @@ def __init__( self.req = req self.container_engine = container_engine + def __repr__(self) -> str: + """Allow for debug printing.""" + + parts = [f"expression {self.expr}", f"source {self.source}"] + + return f"StepValueFrom({', '.join(parts)})" + def eval_prep( self, step_inputs: CWLObjectType, file_store: AbstractFileStore ) -> None: @@ -554,6 +579,11 @@ def __init__(self, default: Any, source: Any): self.default = default self.source = source + def __repr__(self) -> str: + """Allow for debug printing.""" + + return f"DefaultWithSource({self.default}, {self.source})" + def resolve(self) -> Any: """ Determine the final input value when the time is right. @@ -576,6 +606,11 @@ def __init__(self, val: Any): """Store the value.""" self.val = val + def __repr__(self) -> str: + """Allow for debug printing.""" + + return f"JustAValue({self.val})" + def resolve(self) -> Any: """Return the value.""" return self.val @@ -2307,7 +2342,7 @@ def run(self, file_store: AbstractFileStore) -> Any: cwllogger.removeHandler(defaultStreamHandler) cwllogger.setLevel(logger.getEffectiveLevel()) - logger.debug("Loaded order: %s", self.cwljob) + logger.debug("Loaded order:\n%s", pprint.pformat(self.cwljob)) cwljob = resolve_dict_w_promises(self.cwljob, file_store) @@ -2886,6 +2921,8 @@ def run( get_container_engine(self.runtime_context), ) + logger.debug("Value will come from %s", jobobj.get(key, None)) + conditional = Conditional( expression=step.tool.get("when"), outputs=step.tool["out"], From eb2821585c4b308d16e1411eaf4242e267f35694 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 13:04:04 -0700 Subject: [PATCH 02/20] Quiet debugging a bit --- src/toil/cwl/cwltoil.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 36ab03d09b..cc129007fd 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -371,8 +371,6 @@ def __repr__(self) -> str: def resolve(self) -> Any: """First apply linkMerge then pickValue if either present.""" - logger.debug("Resolving %s", self.name) - result: Optional[Any] = None if isinstance(self.promise_tuples, list): result = self.link_merge( @@ -399,8 +397,6 @@ def link_merge( link_merge_type = self.input.get("linkMerge", "merge_nested") - logger.debug("Applying linkMerge for %s type %s to:\n%s", self.name, link_merge_type, pprint.pformat(values)) - if link_merge_type == "merge_nested": return values @@ -429,8 +425,6 @@ def pick_value(self, values: Union[List[Union[str, SkipNull]], Any]) -> Any: pick_value_type = cast(str, self.input.get("pickValue")) - logger.debug("Applying pickValue for %s type %s to:\n%s", self.name, pick_value_type, pprint.pformat(values)) - if pick_value_type is None: return values @@ -503,9 +497,7 @@ def __init__( def __repr__(self) -> str: """Allow for debug printing.""" - parts = [f"expression {self.expr}", f"source {self.source}"] - - return f"StepValueFrom({', '.join(parts)})" + return f"StepValueFrom({self.expr}, {self.source}, {self.req}, {self.container_engine})" def eval_prep( self, step_inputs: CWLObjectType, file_store: AbstractFileStore @@ -2342,7 +2334,7 @@ def run(self, file_store: AbstractFileStore) -> Any: cwllogger.removeHandler(defaultStreamHandler) cwllogger.setLevel(logger.getEffectiveLevel()) - logger.debug("Loaded order:\n%s", pprint.pformat(self.cwljob)) + logger.debug("Loaded order:\n%s", self.cwljob) cwljob = resolve_dict_w_promises(self.cwljob, file_store) From f78f3b608f5513eac1d9523a0c1d2b37f886df64 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 13:11:09 -0700 Subject: [PATCH 03/20] Move default setting for workflows so it works on subworkflows --- src/toil/cwl/cwltoil.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index cc129007fd..0e59885c7c 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -2851,6 +2851,10 @@ def run( if self.conditional.is_false(cwljob): return self.conditional.skipped_outputs() + # Apply default values set in the workflow + fs_access = ToilFsAccess(runtime_context.basedir, file_store=file_store) + fill_in_defaults(self.cwlwf.tool["inputs"], cwljob, fs_access) + # `promises` dict # from: each parameter (workflow input or step output) # that may be used as a "source" for a step input workflow output @@ -3820,10 +3824,8 @@ def main(args: Optional[List[str]] = None, stdout: TextIO = sys.stdout) -> int: ) raise - # We make a ToilFSAccess to access URLs with, but it has no - # FileStore so it can't do toildir: and toilfile: - fs_access = ToilFsAccess(options.basedir) - fill_in_defaults(tool.tool["inputs"], initialized_job_order, fs_access) + # Leave the defaults un-filled in the top-level order. The tool or + # workflow will fill them when it runs for inp in tool.tool["inputs"]: if ( From b21648eaf0786d9448672b45dd701cfa55847098 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 13:15:16 -0700 Subject: [PATCH 04/20] Remember to keep making a ToilFsAccess on the leader --- src/toil/cwl/cwltoil.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 0e59885c7c..205413aa7c 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -3883,6 +3883,7 @@ def main(args: Optional[List[str]] = None, stdout: TextIO = sys.stdout) -> int: # Import all the input files, some of which may be missing optional # files. + fs_access = ToilFsAccess(options.basedir) import_files( file_import_function, fs_access, From 856e2d6abd4b041bfbf9bcbb637e1e30fc55de47 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 13:24:38 -0700 Subject: [PATCH 05/20] Satisfy MyPy --- src/toil/cwl/cwltoil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 205413aa7c..d9b10c7f92 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -2852,7 +2852,7 @@ def run( return self.conditional.skipped_outputs() # Apply default values set in the workflow - fs_access = ToilFsAccess(runtime_context.basedir, file_store=file_store) + fs_access = ToilFsAccess(self.runtime_context.basedir, file_store=file_store) fill_in_defaults(self.cwlwf.tool["inputs"], cwljob, fs_access) # `promises` dict From 5ca06b62020f663c9fc65eb103b39139e7f388d0 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Wed, 18 Oct 2023 14:00:13 -0700 Subject: [PATCH 06/20] Stop giving CWL containers directories full of broken symlinks --- src/toil/cwl/cwltoil.py | 3 ++- src/toil/cwl/utils.py | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index d9b10c7f92..d1c863040f 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -1232,7 +1232,8 @@ def _abs(self, path: str) -> str: logger.debug("ToilFsAccess downloading %s to %s", cache_key, temp_dir) - # Save it all into this new temp directory + # Save it all into this new temp directory. + # Guaranteed to fill it with real files and not symlinks. download_structure(self.file_store, {}, {}, contents, temp_dir) # Make sure we use the same temp directory if we go traversing diff --git a/src/toil/cwl/utils.py b/src/toil/cwl/utils.py index 60159fb564..d45cddae1d 100644 --- a/src/toil/cwl/utils.py +++ b/src/toil/cwl/utils.py @@ -140,6 +140,9 @@ def download_structure( """ Download nested dictionary from the Toil file store to a local path. + Guaranteed to fill the structure with real files, and not symlinks out of + it to elsewhere. + :param file_store: The Toil file store to download from. :param index: Maps from downloaded file path back to input Toil URI. @@ -173,9 +176,11 @@ def download_structure( raise RuntimeError(f"Did not find a filestore file at {value}") logger.debug("Downloading contained file '%s'", name) dest_path = os.path.join(into_dir, name) - # So download the file into place + # So download the file into place. + # Make sure to get a real copy of the file because we may need to + # mount the directory into a container as a whole. file_store.readGlobalFile( - FileID.unpack(value[len("toilfile:") :]), dest_path, symlink=True + FileID.unpack(value[len("toilfile:") :]), dest_path, symlink=False ) # Update the index dicts # TODO: why? From f1bf10cd2dcebc9d03cf0052d6c0fc427d8dab7d Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 19 Oct 2023 09:54:42 -0700 Subject: [PATCH 07/20] Update test to expect no symlinks --- src/toil/test/cwl/cwlTest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index 1463ca0999..2401608cee 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -1530,9 +1530,9 @@ def test_download_structure(tmp_path) -> None: # The file store should have been asked to do the download file_store.readGlobalFile.assert_has_calls( [ - call(fid1, os.path.join(to_dir, "dir1/dir2/f1"), symlink=True), - call(fid1, os.path.join(to_dir, "dir1/dir2/f1again"), symlink=True), - call(fid2, os.path.join(to_dir, "anotherfile"), symlink=True), + call(fid1, os.path.join(to_dir, "dir1/dir2/f1"), symlink=False), + call(fid1, os.path.join(to_dir, "dir1/dir2/f1again"), symlink=False), + call(fid2, os.path.join(to_dir, "anotherfile"), symlink=False), ], any_order=True, ) From 2e2eff38166da7e27ad6ffe9fe9599c6a6ce356e Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 19 Oct 2023 14:37:33 -0700 Subject: [PATCH 08/20] Move CWL integration tests for bioconda/biocontainers to integration test runs --- .gitlab-ci.yml | 5 ++++- src/toil/test/cwl/cwlTest.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3a738b74f2..98358a466d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -209,6 +209,9 @@ cwl_v1.2: - pwd - ${MAIN_PYTHON_PKG} -m virtualenv venv && . venv/bin/activate && pip install -U pip wheel && make prepare && make develop extras=[cwl,aws] - python setup_gitlab_docker.py # login to increase the docker.io rate limit + # Run CWL integration tests excluded from cwl_misc + - make test threads="${TEST_THREADS}" tests="src/toil/test/cwl/cwlTest.py -k '(CWLWorkflowTest or cwl_small) and integrative'" + # Run CWL conformance tests - make test threads="${TEST_THREADS}" tests=src/toil/test/cwl/cwlTest.py::CWLv12Test::test_run_conformance_with_in_place_update cwl_on_arm: @@ -232,7 +235,7 @@ cwl_misc: - pwd - ${MAIN_PYTHON_PKG} -m virtualenv venv && . venv/bin/activate && pip install -U pip wheel && make prepare && make develop extras=[cwl,aws] - python setup_gitlab_docker.py # login to increase the docker.io rate limit - - make test threads="${TEST_THREADS}" tests='src/toil/test/cwl/cwlTest.py -k "CWLWorkflowTest or cwl_small"' + - make test threads="${TEST_THREADS}" tests="src/toil/test/cwl/cwlTest.py -k '(CWLWorkflowTest or cwl_small) and not integrative'" #cwl_v1.2_kubernetes: # stage: main_tests diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index 2401608cee..b793d69fe1 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -415,6 +415,7 @@ def test_load_contents_file(self): self.load_contents("download_file.json", self._tester) @slow + @pytest.mark.integrative def test_bioconda(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl", @@ -425,6 +426,7 @@ def test_bioconda(self): ) @needs_docker + @pytest.mark.integrative def test_biocontainers(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl", From 6fd95a392b113c4731af261721befa26c461e57b Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Tue, 24 Oct 2023 08:59:53 -0700 Subject: [PATCH 09/20] Wrap mkdtemp to fix #4644 --- src/toil/cwl/cwltoil.py | 13 ++++++------ src/toil/fileStores/abstractFileStore.py | 8 +++---- src/toil/fileStores/cachingFileStore.py | 7 ++++--- src/toil/jobStores/fileJobStore.py | 6 +++--- src/toil/lib/io.py | 21 +++++++++++++++++++ src/toil/resource.py | 2 +- src/toil/server/wes/abstract_backend.py | 5 +++-- src/toil/test/__init__.py | 7 ++++--- .../test/docs/scripts/tutorial_cwlexample.py | 4 ++-- src/toil/test/docs/scripts/tutorial_docker.py | 4 ++-- .../test/docs/scripts/tutorial_dynamic.py | 4 ++-- .../docs/scripts/tutorial_encapsulation.py | 4 ++-- .../docs/scripts/tutorial_encapsulation2.py | 4 ++-- .../docs/scripts/tutorial_invokeworkflow.py | 4 ++-- .../docs/scripts/tutorial_invokeworkflow2.py | 4 ++-- .../docs/scripts/tutorial_jobfunctions.py | 4 ++-- .../test/docs/scripts/tutorial_managing.py | 4 ++-- .../test/docs/scripts/tutorial_managing2.py | 4 ++-- .../test/docs/scripts/tutorial_promises.py | 4 ++-- .../test/docs/scripts/tutorial_promises2.py | 4 ++-- .../test/docs/scripts/tutorial_quickstart.py | 4 ++-- .../docs/scripts/tutorial_requirements.py | 4 ++-- .../test/docs/scripts/tutorial_services.py | 4 ++-- .../test/docs/scripts/tutorial_staging.py | 4 ++-- src/toil/test/jobStores/jobStoreTest.py | 15 ++++++------- src/toil/test/src/miscTests.py | 5 ++--- src/toil/test/src/systemTest.py | 4 ++-- .../utils/ABCWorkflowDebug/debugWorkflow.py | 4 ++-- src/toil/wdl/wdl_synthesis.py | 4 ++-- src/toil/wdl/wdltoil.py | 9 ++++---- 30 files changed, 100 insertions(+), 74 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index d1c863040f..f3fb939bc0 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -31,7 +31,6 @@ import socket import stat import sys -import tempfile import textwrap import urllib import uuid @@ -102,6 +101,7 @@ from schema_salad.exceptions import ValidationException from schema_salad.ref_resolver import file_uri, uri_file_path from schema_salad.sourceline import SourceLine +from tempfile import NamedTemporaryFile, gettempdir from typing_extensions import Literal from toil.batchSystems.registry import DEFAULT_BATCH_SYSTEM @@ -121,6 +121,7 @@ from toil.jobStores.abstractJobStore import AbstractJobStore, NoSuchFileException from toil.jobStores.fileJobStore import FileJobStore from toil.jobStores.utils import JobStoreUnavailableException, generate_locator +from toil.lib.io import mkdtemp from toil.lib.threading import ExceptionalThread from toil.statsAndLogging import DEFAULT_LOGLEVEL from toil.version import baseVersion @@ -128,7 +129,7 @@ logger = logging.getLogger(__name__) # Find the default temporary directory -DEFAULT_TMPDIR = tempfile.gettempdir() +DEFAULT_TMPDIR = gettempdir() # And compose a CWL-style default prefix inside it. # We used to not put this inside anything and we would drop loads of temp # directories in the current directory and leave them there. @@ -1263,7 +1264,7 @@ def _abs(self, path: str) -> str: logger.debug( "ToilFsAccess fetching directory %s from a JobStore", path ) - dest_dir = tempfile.mkdtemp() + dest_dir = mkdtemp() # Recursively fetch all the files in the directory. def download_to(url: str, dest: str) -> None: @@ -1286,7 +1287,7 @@ def download_to(url: str, dest: str) -> None: logger.debug("ToilFsAccess fetching file %s from a JobStore", path) # Try to grab it with a jobstore implementation, and save it # somewhere arbitrary. - dest_file = tempfile.NamedTemporaryFile(delete=False) + dest_file = NamedTemporaryFile(delete=False) AbstractJobStore.read_from_url(path, dest_file) dest_file.close() self.dir_to_download[path] = dest_file.name @@ -2041,7 +2042,7 @@ def _realpath( "CreateFile", "CreateWritableFile", ]: # TODO: CreateFile for buckets is not under testing - with tempfile.NamedTemporaryFile() as f: + with NamedTemporaryFile() as f: # Make a file with the right contents f.write(file_id_or_contents.encode("utf-8")) f.close() @@ -3621,7 +3622,7 @@ def main(args: Optional[List[str]] = None, stdout: TextIO = sys.stdout) -> int: workdir = cwltool.utils.create_tmp_dir(options.tmpdir_prefix) else: # Use a directory in the default tmpdir - workdir = tempfile.mkdtemp() + workdir = mkdtemp() # Make sure workdir doesn't exist so it can be a job store os.rmdir(workdir) diff --git a/src/toil/fileStores/abstractFileStore.py b/src/toil/fileStores/abstractFileStore.py index fc714681ca..600a7d3f36 100644 --- a/src/toil/fileStores/abstractFileStore.py +++ b/src/toil/fileStores/abstractFileStore.py @@ -13,9 +13,9 @@ # limitations under the License. import logging import os -import tempfile from abc import ABC, abstractmethod from contextlib import contextmanager +from tempfile import mkstemp from threading import Event, Semaphore from typing import (IO, TYPE_CHECKING, @@ -40,7 +40,7 @@ from toil.job import Job, JobDescription from toil.jobStores.abstractJobStore import AbstractJobStore from toil.lib.compatibility import deprecated -from toil.lib.io import WriteWatchingStream +from toil.lib.io import WriteWatchingStream, mkdtemp logger = logging.getLogger(__name__) @@ -207,7 +207,7 @@ def getLocalTempDir(self) -> str: to be deleted once the job terminates, removing all files it contains recursively. """ - return os.path.abspath(tempfile.mkdtemp(dir=self.localTempDir)) + return os.path.abspath(mkdtemp(dir=self.localTempDir)) def getLocalTempFile(self, suffix: Optional[str] = None, prefix: Optional[str] = None) -> str: """ @@ -223,7 +223,7 @@ def getLocalTempFile(self, suffix: Optional[str] = None, prefix: Optional[str] = for the duration of the job only, and is guaranteed to be deleted once the job terminates. """ - handle, tmpFile = tempfile.mkstemp( + handle, tmpFile = mkstemp( suffix=".tmp" if suffix is None else suffix, prefix="tmp" if prefix is None else prefix, dir=self.localTempDir diff --git a/src/toil/fileStores/cachingFileStore.py b/src/toil/fileStores/cachingFileStore.py index e216d1a57a..feb57346d3 100644 --- a/src/toil/fileStores/cachingFileStore.py +++ b/src/toil/fileStores/cachingFileStore.py @@ -20,10 +20,10 @@ import shutil import sqlite3 import stat -import tempfile import threading import time from contextlib import contextmanager +from tempfile import mkstemp from typing import Any, Callable, Generator, Iterator, Optional, Sequence, Tuple from toil.common import cacheDirName, getDirSizeRecursively, getFileSystemSize @@ -36,6 +36,7 @@ from toil.lib.io import (atomic_copy, atomic_copyobj, make_public_dir, + mkdtemp, robust_rmtree) from toil.lib.retry import ErrorCondition, retry from toil.lib.threading import get_process_name, process_name_exists @@ -600,7 +601,7 @@ def cachingIsFree(self): emptyID = self.jobStore.getEmptyFileStoreID() # Read it out to a generated name. - destDir = tempfile.mkdtemp(dir=self.localCacheDir) + destDir = mkdtemp(dir=self.localCacheDir) cachedFile = os.path.join(destDir, 'sniffLinkCount') self.jobStore.read_file(emptyID, cachedFile, symlink=False) @@ -644,7 +645,7 @@ def _getNewCachingPath(self, fileStoreID): # sure we can never collide even though we are going to remove the # file. # TODO: use a de-slashed version of the ID instead? - handle, path = tempfile.mkstemp(dir=self.localCacheDir, suffix=hasher.hexdigest()) + handle, path = mkstemp(dir=self.localCacheDir, suffix=hasher.hexdigest()) os.close(handle) os.unlink(path) diff --git a/src/toil/jobStores/fileJobStore.py b/src/toil/jobStores/fileJobStore.py index b3b009bcf6..17e88050db 100644 --- a/src/toil/jobStores/fileJobStore.py +++ b/src/toil/jobStores/fileJobStore.py @@ -20,7 +20,6 @@ import shutil import stat import sys -import tempfile import time import uuid from contextlib import contextmanager @@ -42,6 +41,7 @@ from toil.lib.io import (AtomicFileCreate, atomic_copy, atomic_copyobj, + mkdtemp, robust_rmtree) logger = logging.getLogger(__name__) @@ -147,8 +147,8 @@ def assign_job_id(self, job_description): # Make a unique temp directory under a directory for this job name, # possibly sprayed across multiple levels of subdirectories. - absJobDir = tempfile.mkdtemp(prefix=self.JOB_DIR_PREFIX, - dir=self._get_arbitrary_jobs_dir_for_name(usefulFilename)) + absJobDir = mkdtemp(prefix=self.JOB_DIR_PREFIX, + dir=self._get_arbitrary_jobs_dir_for_name(usefulFilename)) job_description.jobStoreID = self._get_job_id_from_dir(absJobDir) diff --git a/src/toil/lib/io.py b/src/toil/lib/io.py index 8f9389c205..69fd60b68d 100644 --- a/src/toil/lib/io.py +++ b/src/toil/lib/io.py @@ -2,6 +2,7 @@ import os import shutil import stat +import tempfile import uuid from contextlib import contextmanager from io import BytesIO @@ -9,6 +10,26 @@ logger = logging.getLogger(__name__) +def mkdtemp(suffix: Optional[str] = None, prefix: Optional[str] = None, dir: Optional[str] = None) -> str: + """ + Make a temporary directory like tempfile.mkdtemp, but with relaxed permissions. + + The permissions on the directory will be 711 instead of 700, allowing the + group and all other users to traverse the directory. This is necessary if + the direcotry is on NFS and the Docker daemon would like to mount it or a + file inside it into a container, because on NFS even the Docker daemon + appears bound by the file permissions. + + See , and + which talks about a similar problem + but in the context of user namespaces. + """ + # Make the directory + result = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir) + # Grant all the permissions: full control for user, and execute for group and other + os.chmod(result, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + # Return the path created + return result def robust_rmtree(path: Union[str, bytes]) -> None: """ diff --git a/src/toil/resource.py b/src/toil/resource.py index 705fbe1253..739326e30a 100644 --- a/src/toil/resource.py +++ b/src/toil/resource.py @@ -23,7 +23,6 @@ from contextlib import closing from io import BytesIO from pydoc import locate -from tempfile import mkdtemp from urllib.error import HTTPError from urllib.request import urlopen from zipfile import ZipFile @@ -37,6 +36,7 @@ BinaryIO) from toil import inVirtualEnv +from toil.lib.io import mkdtemp from toil.lib.iterables import concat from toil.lib.memoize import strict_bool from toil.lib.retry import ErrorCondition, retry diff --git a/src/toil/server/wes/abstract_backend.py b/src/toil/server/wes/abstract_backend.py index 3eef604067..32abff6e3b 100644 --- a/src/toil/server/wes/abstract_backend.py +++ b/src/toil/server/wes/abstract_backend.py @@ -3,7 +3,6 @@ import json import logging import os -import tempfile from abc import abstractmethod from typing import Any, Callable, Dict, List, Optional, Tuple, Union from urllib.parse import urldefrag @@ -11,6 +10,8 @@ import connexion # type: ignore from werkzeug.utils import secure_filename +from toil.lib.io import mkdtemp + logger = logging.getLogger(__name__) # Define a type for WES task log entries in responses @@ -210,7 +211,7 @@ def collect_attachments(self, run_id: Optional[str], temp_dir: Optional[str]) -> If None, a temporary directory is created. """ if not temp_dir: - temp_dir = tempfile.mkdtemp() + temp_dir = mkdtemp() body: Dict[str, Any] = {} has_attachments = False for key, ls in connexion.request.files.lists(): diff --git a/src/toil/test/__init__.py b/src/toil/test/__init__.py index 7a10e68e84..3734853385 100644 --- a/src/toil/test/__init__.py +++ b/src/toil/test/__init__.py @@ -21,7 +21,6 @@ import signal import subprocess import sys -import tempfile import threading import time import unittest @@ -30,6 +29,7 @@ from contextlib import contextmanager from inspect import getsource from shutil import which +from tempfile import mkstemp from textwrap import dedent from typing import (Any, Callable, @@ -57,6 +57,7 @@ from toil.lib.accelerators import (have_working_nvidia_docker_runtime, have_working_nvidia_smi) from toil.lib.aws import running_on_ec2 +from toil.lib.io import mkdtemp from toil.lib.iterables import concat from toil.lib.memoize import memoize from toil.lib.threading import ExceptionalThread, cpu_count @@ -188,7 +189,7 @@ def _createTempDirEx(cls, *names: Optional[str]) -> str: prefix.extend([_f for _f in names if _f]) prefix.append('') temp_dir_path = os.path.realpath( - tempfile.mkdtemp(dir=cls._tempBaseDir, prefix="-".join(prefix)) + mkdtemp(dir=cls._tempBaseDir, prefix="-".join(prefix)) ) cls._tempDirs.append(temp_dir_path) return temp_dir_path @@ -314,7 +315,7 @@ def _mark_test(name: str, test_item: MT) -> MT: def get_temp_file(suffix: str = "", rootDir: Optional[str] = None) -> str: """Return a string representing a temporary file, that must be manually deleted.""" if rootDir is None: - handle, tmp_file = tempfile.mkstemp(suffix) + handle, tmp_file = mkstemp(suffix) os.close(handle) return tmp_file else: diff --git a/src/toil/test/docs/scripts/tutorial_cwlexample.py b/src/toil/test/docs/scripts/tutorial_cwlexample.py index ed9db885af..aaf3adbb04 100644 --- a/src/toil/test/docs/scripts/tutorial_cwlexample.py +++ b/src/toil/test/docs/scripts/tutorial_cwlexample.py @@ -1,6 +1,6 @@ import os import subprocess -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -26,7 +26,7 @@ def runQC(job, cwl_file, cwl_filename, yml_file, yml_filename, outputs_dir, outp if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_cwlexample") + jobstore: str = mkdtemp("tutorial_cwlexample") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_docker.py b/src/toil/test/docs/scripts/tutorial_docker.py index 955720d50c..e9361c12d7 100644 --- a/src/toil/test/docs/scripts/tutorial_docker.py +++ b/src/toil/test/docs/scripts/tutorial_docker.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -11,7 +11,7 @@ parameters=['ls', '-lha']) if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_docker") + jobstore: str = mkdtemp("tutorial_docker") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_dynamic.py b/src/toil/test/docs/scripts/tutorial_dynamic.py index 6324e344bf..db96cf9701 100644 --- a/src/toil/test/docs/scripts/tutorial_dynamic.py +++ b/src/toil/test/docs/scripts/tutorial_dynamic.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -14,7 +14,7 @@ def binaryStringFn(job, depth, message=""): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_dynamic") + jobstore: str = mkdtemp("tutorial_dynamic") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_encapsulation.py b/src/toil/test/docs/scripts/tutorial_encapsulation.py index a8c91ce663..b640f89142 100644 --- a/src/toil/test/docs/scripts/tutorial_encapsulation.py +++ b/src/toil/test/docs/scripts/tutorial_encapsulation.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -18,7 +18,7 @@ Ap.addChild(A) Ap.addFollowOn(B) - jobstore: str = tempfile.mkdtemp("tutorial_encapsulations") + jobstore: str = mkdtemp("tutorial_encapsulations") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_encapsulation2.py b/src/toil/test/docs/scripts/tutorial_encapsulation2.py index 546a1a15e8..ed5d803742 100644 --- a/src/toil/test/docs/scripts/tutorial_encapsulation2.py +++ b/src/toil/test/docs/scripts/tutorial_encapsulation2.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -19,7 +19,7 @@ # With encapsulation A and its successor subgraph appear to be a single job, hence: A.addChild(B) - jobstore: str = tempfile.mkdtemp("tutorial_encapsulations2") + jobstore: str = mkdtemp("tutorial_encapsulations2") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_invokeworkflow.py b/src/toil/test/docs/scripts/tutorial_invokeworkflow.py index c6866ac824..de433d5778 100644 --- a/src/toil/test/docs/scripts/tutorial_invokeworkflow.py +++ b/src/toil/test/docs/scripts/tutorial_invokeworkflow.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -15,7 +15,7 @@ def run(self, fileStore): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_invokeworkflow") + jobstore: str = mkdtemp("tutorial_invokeworkflow") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "OFF" diff --git a/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py b/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py index 639cc83f2f..5a9eb96622 100644 --- a/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py +++ b/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -14,7 +14,7 @@ def run(self, fileStore): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_invokeworkflow2") + jobstore: str = mkdtemp("tutorial_invokeworkflow2") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_jobfunctions.py b/src/toil/test/docs/scripts/tutorial_jobfunctions.py index 37d189a760..a903f9a63b 100644 --- a/src/toil/test/docs/scripts/tutorial_jobfunctions.py +++ b/src/toil/test/docs/scripts/tutorial_jobfunctions.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -10,7 +10,7 @@ def helloWorld(job, message): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_jobfunctions") + jobstore: str = mkdtemp("tutorial_jobfunctions") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_managing.py b/src/toil/test/docs/scripts/tutorial_managing.py index 78542c8efc..0a6b1227e4 100644 --- a/src/toil/test/docs/scripts/tutorial_managing.py +++ b/src/toil/test/docs/scripts/tutorial_managing.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -15,7 +15,7 @@ def run(self, fileStore): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_managing") + jobstore: str = mkdtemp("tutorial_managing") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_managing2.py b/src/toil/test/docs/scripts/tutorial_managing2.py index 80f594e7f1..f578706a2c 100644 --- a/src/toil/test/docs/scripts/tutorial_managing2.py +++ b/src/toil/test/docs/scripts/tutorial_managing2.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -44,7 +44,7 @@ def globalFileStoreJobFn(job): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_managing2") + jobstore: str = mkdtemp("tutorial_managing2") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_promises.py b/src/toil/test/docs/scripts/tutorial_promises.py index a9a850eadc..78edc0d39d 100644 --- a/src/toil/test/docs/scripts/tutorial_promises.py +++ b/src/toil/test/docs/scripts/tutorial_promises.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -11,7 +11,7 @@ def fn(job, i): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_promises") + jobstore: str = mkdtemp("tutorial_promises") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_promises2.py b/src/toil/test/docs/scripts/tutorial_promises2.py index ea689f12ac..b40846586a 100644 --- a/src/toil/test/docs/scripts/tutorial_promises2.py +++ b/src/toil/test/docs/scripts/tutorial_promises2.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -18,7 +18,7 @@ def merge(strings): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_promises2") + jobstore: str = mkdtemp("tutorial_promises2") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.loglevel = "OFF" diff --git a/src/toil/test/docs/scripts/tutorial_quickstart.py b/src/toil/test/docs/scripts/tutorial_quickstart.py index a06f2e017f..1ea771e6c7 100644 --- a/src/toil/test/docs/scripts/tutorial_quickstart.py +++ b/src/toil/test/docs/scripts/tutorial_quickstart.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -10,7 +10,7 @@ def helloWorld(message): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_quickstart") + jobstore: str = mkdtemp("tutorial_quickstart") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "OFF" diff --git a/src/toil/test/docs/scripts/tutorial_requirements.py b/src/toil/test/docs/scripts/tutorial_requirements.py index 0e0ba9409a..b5e15f4f13 100644 --- a/src/toil/test/docs/scripts/tutorial_requirements.py +++ b/src/toil/test/docs/scripts/tutorial_requirements.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job, PromisedRequirement @@ -26,7 +26,7 @@ def analysisJob(job, fileStoreID): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_requirements") + jobstore: str = mkdtemp("tutorial_requirements") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_services.py b/src/toil/test/docs/scripts/tutorial_services.py index 5bcf9faa47..825fc15bad 100644 --- a/src/toil/test/docs/scripts/tutorial_services.py +++ b/src/toil/test/docs/scripts/tutorial_services.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -35,7 +35,7 @@ def dbFn(loginCredentials): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_services") + jobstore: str = mkdtemp("tutorial_services") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/docs/scripts/tutorial_staging.py b/src/toil/test/docs/scripts/tutorial_staging.py index e5f6e27fc9..3ddaf7973b 100644 --- a/src/toil/test/docs/scripts/tutorial_staging.py +++ b/src/toil/test/docs/scripts/tutorial_staging.py @@ -1,5 +1,5 @@ import os -import tempfile +from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job @@ -18,7 +18,7 @@ def run(self, fileStore): if __name__ == "__main__": - jobstore: str = tempfile.mkdtemp("tutorial_staging") + jobstore: str = mkdtemp("tutorial_staging") os.rmdir(jobstore) options = Job.Runner.getDefaultOptions(jobstore) options.logLevel = "INFO" diff --git a/src/toil/test/jobStores/jobStoreTest.py b/src/toil/test/jobStores/jobStoreTest.py index 3e91822992..461778a5a4 100644 --- a/src/toil/test/jobStores/jobStoreTest.py +++ b/src/toil/test/jobStores/jobStoreTest.py @@ -18,7 +18,6 @@ import os import shutil import socketserver -import tempfile import threading import time import urllib.parse as urlparse @@ -27,6 +26,7 @@ from io import BytesIO from itertools import chain, islice from queue import Queue +from tempfile import mkstemp from threading import Thread from typing import Any, Tuple from urllib.request import Request, urlopen @@ -41,6 +41,7 @@ NoSuchJobException) from toil.jobStores.fileJobStore import FileJobStore from toil.lib.aws.utils import create_s3_bucket, get_object_for_url +from toil.lib.io import mkdtemp from toil.lib.memoize import memoize from toil.lib.retry import retry from toil.statsAndLogging import StatsAndLogging @@ -443,7 +444,7 @@ def testPerJobFiles(self): self.assertEqual(f.read(), one) # ... and copy it to a temporary physical file on the jobstore1. - fh, path = tempfile.mkstemp() + fh, path = mkstemp() try: os.close(fh) tmpPath = path + '.read-only' @@ -858,7 +859,7 @@ def checksumThreadFn(): # Multi-part upload from file checksum = hashlib.md5() - fh, path = tempfile.mkstemp() + fh, path = mkstemp() try: with os.fdopen(fh, 'wb+') as writable: with open('/dev/urandom', 'rb') as readable: @@ -1046,7 +1047,7 @@ def testDestructionIdempotence(self): def testEmptyFileStoreIDIsReadable(self): """Simply creates an empty fileStoreID and attempts to read from it.""" id = self.jobstore_initialized.get_empty_file_store_id() - fh, path = tempfile.mkstemp() + fh, path = mkstemp() try: self.jobstore_initialized.read_file(id, path) self.assertTrue(os.path.isfile(path)) @@ -1075,7 +1076,7 @@ class Test(AbstractJobStoreTest.Test, metaclass=ABCMeta): def setUp(self): # noinspection PyAttributeOutsideInit - self.sseKeyDir = tempfile.mkdtemp() + self.sseKeyDir = mkdtemp() super().setUp() def tearDown(self): @@ -1143,14 +1144,14 @@ def _hashTestFile(self, url): return hashlib.md5(f.read()).hexdigest() def _createExternalStore(self): - return tempfile.mkdtemp() + return mkdtemp() def _cleanUpExternalStore(self, dirPath): shutil.rmtree(dirPath) def testPreserveFileName(self): """Check that the fileID ends with the given file name.""" - fh, path = tempfile.mkstemp() + fh, path = mkstemp() try: os.close(fh) job = self.arbitraryJob() diff --git a/src/toil/test/src/miscTests.py b/src/toil/test/src/miscTests.py index 24705aef29..e03aa27cbd 100644 --- a/src/toil/test/src/miscTests.py +++ b/src/toil/test/src/miscTests.py @@ -16,12 +16,11 @@ import os import random import sys -import tempfile from uuid import uuid4 from toil.common import getNodeID from toil.lib.exceptions import panic, raise_ -from toil.lib.io import AtomicFileCreate, atomic_install, atomic_tmp_file +from toil.lib.io import AtomicFileCreate, atomic_install, atomic_tmp_file, mkdtemp from toil.lib.misc import CalledProcessErrorStderr, call_command from toil.test import ToilTest, slow @@ -63,7 +62,7 @@ def testGetSizeOfDirectoryWorks(self): files = {} # Create a random directory structure for i in range(0,10): - directories.append(tempfile.mkdtemp(dir=random.choice(directories), prefix='test')) + directories.append(mkdtemp(dir=random.choice(directories), prefix='test')) # Create 50 random file entries in different locations in the directories. 75% of the time # these are fresh files of size [1, 10] MB and 25% of the time they are hard links to old # files. diff --git a/src/toil/test/src/systemTest.py b/src/toil/test/src/systemTest.py index 560d885a7e..3576165700 100644 --- a/src/toil/test/src/systemTest.py +++ b/src/toil/test/src/systemTest.py @@ -1,9 +1,9 @@ import errno import multiprocessing import os -import tempfile from functools import partial +from toil.lib.io import mkdtemp from toil.lib.threading import cpu_count from toil.test import ToilTest @@ -37,7 +37,7 @@ def testAtomicityOfNonEmptyDirectoryRenames(self): def _testAtomicityOfNonEmptyDirectoryRenamesTask(parent, child, _): - tmpChildDir = tempfile.mkdtemp(dir=parent, prefix='child', suffix='.tmp') + tmpChildDir = mkdtemp(dir=parent, prefix='child', suffix='.tmp') grandChild = os.path.join(tmpChildDir, 'grandChild') open(grandChild, 'w').close() grandChildId = os.stat(grandChild).st_ino diff --git a/src/toil/test/utils/ABCWorkflowDebug/debugWorkflow.py b/src/toil/test/utils/ABCWorkflowDebug/debugWorkflow.py index 6278a2417d..ba953bfb2a 100644 --- a/src/toil/test/utils/ABCWorkflowDebug/debugWorkflow.py +++ b/src/toil/test/utils/ABCWorkflowDebug/debugWorkflow.py @@ -2,9 +2,9 @@ import os import sys import subprocess -import tempfile from toil.common import Toil +from toil.lib.io import mkdtemp from toil.job import Job from toil.version import python @@ -137,7 +137,7 @@ def broken_job(job, num): file = toil.importFile(None) if __name__=="__main__": - jobStorePath = sys.argv[1] if len(sys.argv) > 1 else tempfile.mkdtemp("debugWorkflow") + jobStorePath = sys.argv[1] if len(sys.argv) > 1 else mkdtemp("debugWorkflow") options = Job.Runner.getDefaultOptions(jobStorePath) options.clean = "never" options.stats = True diff --git a/src/toil/wdl/wdl_synthesis.py b/src/toil/wdl/wdl_synthesis.py index 395c5cc458..1ea8efec70 100644 --- a/src/toil/wdl/wdl_synthesis.py +++ b/src/toil/wdl/wdl_synthesis.py @@ -13,9 +13,9 @@ # limitations under the License. import logging import os -import tempfile from typing import Optional +from toil.lib.io import mkdtemp from toil.wdl.wdl_functions import heredoc_wdl from toil.wdl.wdl_types import (WDLArrayType, WDLCompoundType, @@ -69,7 +69,7 @@ def __init__(self, if jobstore: self.jobstore = jobstore else: - self.jobstore = tempfile.mkdtemp(prefix=f"{os.getcwd()}{os.sep}toilWorkflowRun") + self.jobstore = mkdtemp(prefix=f"{os.getcwd()}{os.sep}toilWorkflowRun") os.rmdir(self.jobstore) if docker_user != 'None': diff --git a/src/toil/wdl/wdltoil.py b/src/toil/wdl/wdltoil.py index c29afb4bdc..fb2ec5b363 100755 --- a/src/toil/wdl/wdltoil.py +++ b/src/toil/wdl/wdltoil.py @@ -28,11 +28,11 @@ import shutil import subprocess import sys -import tempfile import uuid from contextlib import ExitStack, contextmanager from graphlib import TopologicalSorter +from tempfile import mkstemp from typing import cast, Any, Callable, Union, Dict, List, Optional, Set, Sequence, Tuple, Type, TypeVar, Iterator, \ Iterable, Generator from urllib.parse import urlsplit, urljoin, quote, unquote @@ -50,6 +50,7 @@ from toil.fileStores import FileID from toil.fileStores.abstractFileStore import AbstractFileStore from toil.jobStores.abstractJobStore import AbstractJobStore, UnimplementedURLException +from toil.lib.io import mkdtemp from toil.lib.memoize import memoize from toil.lib.conversions import convert_units, human2bytes from toil.lib.misc import get_user_name @@ -2485,12 +2486,12 @@ def main() -> None: # Make sure we have a jobStore if options.jobStore is None: # TODO: Move cwltoil's generate_default_job_store where we can use it - options.jobStore = os.path.join(tempfile.mkdtemp(), 'tree') + options.jobStore = os.path.join(mkdtemp(), 'tree') # Make sure we have an output directory (or URL prefix) and we don't need # to ever worry about a None, and MyPy knows it. # If we don't have a directory assigned, make one in the current directory. - output_directory: str = options.output_directory if options.output_directory else tempfile.mkdtemp(prefix='wdl-out-', dir=os.getcwd()) + output_directory: str = options.output_directory if options.output_directory else mkdtemp(prefix='wdl-out-', dir=os.getcwd()) with Toil(options) as toil: if options.restart: @@ -2602,7 +2603,7 @@ def devirtualize_output(filename: str) -> str: else: # Export output to path or URL. # So we need to import and then export. - fd, filename = tempfile.mkstemp() + fd, filename = mkstemp() with open(fd, 'w') as handle: # Populate the file handle.write(json.dumps(outputs)) From 78033df768e055c95855b9609710c0a1c11606c7 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Tue, 24 Oct 2023 09:00:43 -0700 Subject: [PATCH 10/20] Sort imports in example scripts --- src/toil/test/docs/scripts/example_alwaysfail.py | 3 ++- src/toil/test/docs/scripts/example_cachingbenchmark.py | 4 ++-- src/toil/test/docs/scripts/tutorial_cwlexample.py | 2 +- src/toil/test/docs/scripts/tutorial_docker.py | 2 +- src/toil/test/docs/scripts/tutorial_dynamic.py | 2 +- src/toil/test/docs/scripts/tutorial_encapsulation.py | 2 +- src/toil/test/docs/scripts/tutorial_encapsulation2.py | 2 +- src/toil/test/docs/scripts/tutorial_invokeworkflow.py | 2 +- src/toil/test/docs/scripts/tutorial_invokeworkflow2.py | 3 ++- src/toil/test/docs/scripts/tutorial_jobfunctions.py | 2 +- src/toil/test/docs/scripts/tutorial_managing.py | 2 +- src/toil/test/docs/scripts/tutorial_managing2.py | 2 +- src/toil/test/docs/scripts/tutorial_promises.py | 2 +- src/toil/test/docs/scripts/tutorial_promises2.py | 2 +- src/toil/test/docs/scripts/tutorial_quickstart.py | 2 +- src/toil/test/docs/scripts/tutorial_requirements.py | 2 +- src/toil/test/docs/scripts/tutorial_services.py | 2 +- src/toil/test/docs/scripts/tutorial_staging.py | 2 +- 18 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/toil/test/docs/scripts/example_alwaysfail.py b/src/toil/test/docs/scripts/example_alwaysfail.py index 2b4c3a55ba..9c47ddca1f 100644 --- a/src/toil/test/docs/scripts/example_alwaysfail.py +++ b/src/toil/test/docs/scripts/example_alwaysfail.py @@ -1,6 +1,7 @@ -from configargparse import ArgumentParser import sys +from configargparse import ArgumentParser + from toil.common import Toil from toil.job import Job diff --git a/src/toil/test/docs/scripts/example_cachingbenchmark.py b/src/toil/test/docs/scripts/example_cachingbenchmark.py index 04eb70d68d..463a3f46be 100755 --- a/src/toil/test/docs/scripts/example_cachingbenchmark.py +++ b/src/toil/test/docs/scripts/example_cachingbenchmark.py @@ -17,8 +17,6 @@ """ import argparse -from configargparse import ArgumentParser - import collections import os import random @@ -26,6 +24,8 @@ import sys import time +from configargparse import ArgumentParser + from toil.common import Toil from toil.job import Job from toil.realtimeLogger import RealtimeLogger diff --git a/src/toil/test/docs/scripts/tutorial_cwlexample.py b/src/toil/test/docs/scripts/tutorial_cwlexample.py index aaf3adbb04..7bf63bb2c1 100644 --- a/src/toil/test/docs/scripts/tutorial_cwlexample.py +++ b/src/toil/test/docs/scripts/tutorial_cwlexample.py @@ -1,9 +1,9 @@ import os import subprocess -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def initialize_jobs(job): diff --git a/src/toil/test/docs/scripts/tutorial_docker.py b/src/toil/test/docs/scripts/tutorial_docker.py index e9361c12d7..c002881fde 100644 --- a/src/toil/test/docs/scripts/tutorial_docker.py +++ b/src/toil/test/docs/scripts/tutorial_docker.py @@ -1,9 +1,9 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job from toil.lib.docker import apiDockerCall +from toil.lib.io import mkdtemp align = Job.wrapJobFn(apiDockerCall, image='ubuntu', diff --git a/src/toil/test/docs/scripts/tutorial_dynamic.py b/src/toil/test/docs/scripts/tutorial_dynamic.py index db96cf9701..76f7e82d49 100644 --- a/src/toil/test/docs/scripts/tutorial_dynamic.py +++ b/src/toil/test/docs/scripts/tutorial_dynamic.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def binaryStringFn(job, depth, message=""): diff --git a/src/toil/test/docs/scripts/tutorial_encapsulation.py b/src/toil/test/docs/scripts/tutorial_encapsulation.py index b640f89142..ce8447c45b 100644 --- a/src/toil/test/docs/scripts/tutorial_encapsulation.py +++ b/src/toil/test/docs/scripts/tutorial_encapsulation.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp if __name__ == "__main__": # A is a job with children and follow-ons, for example: diff --git a/src/toil/test/docs/scripts/tutorial_encapsulation2.py b/src/toil/test/docs/scripts/tutorial_encapsulation2.py index ed5d803742..5f4fa1f2ab 100644 --- a/src/toil/test/docs/scripts/tutorial_encapsulation2.py +++ b/src/toil/test/docs/scripts/tutorial_encapsulation2.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp if __name__ == "__main__": # A diff --git a/src/toil/test/docs/scripts/tutorial_invokeworkflow.py b/src/toil/test/docs/scripts/tutorial_invokeworkflow.py index de433d5778..fabac31fc7 100644 --- a/src/toil/test/docs/scripts/tutorial_invokeworkflow.py +++ b/src/toil/test/docs/scripts/tutorial_invokeworkflow.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp class HelloWorld(Job): diff --git a/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py b/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py index 5a9eb96622..728dd5c497 100644 --- a/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py +++ b/src/toil/test/docs/scripts/tutorial_invokeworkflow2.py @@ -1,8 +1,9 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp + class HelloWorld(Job): def __init__(self, message): diff --git a/src/toil/test/docs/scripts/tutorial_jobfunctions.py b/src/toil/test/docs/scripts/tutorial_jobfunctions.py index a903f9a63b..35c328d327 100644 --- a/src/toil/test/docs/scripts/tutorial_jobfunctions.py +++ b/src/toil/test/docs/scripts/tutorial_jobfunctions.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def helloWorld(job, message): diff --git a/src/toil/test/docs/scripts/tutorial_managing.py b/src/toil/test/docs/scripts/tutorial_managing.py index 0a6b1227e4..48e7a1ee43 100644 --- a/src/toil/test/docs/scripts/tutorial_managing.py +++ b/src/toil/test/docs/scripts/tutorial_managing.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp class LocalFileStoreJob(Job): diff --git a/src/toil/test/docs/scripts/tutorial_managing2.py b/src/toil/test/docs/scripts/tutorial_managing2.py index f578706a2c..f1eb748dc1 100644 --- a/src/toil/test/docs/scripts/tutorial_managing2.py +++ b/src/toil/test/docs/scripts/tutorial_managing2.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def globalFileStoreJobFn(job): diff --git a/src/toil/test/docs/scripts/tutorial_promises.py b/src/toil/test/docs/scripts/tutorial_promises.py index 78edc0d39d..99c88b2305 100644 --- a/src/toil/test/docs/scripts/tutorial_promises.py +++ b/src/toil/test/docs/scripts/tutorial_promises.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def fn(job, i): diff --git a/src/toil/test/docs/scripts/tutorial_promises2.py b/src/toil/test/docs/scripts/tutorial_promises2.py index b40846586a..99bd28341d 100644 --- a/src/toil/test/docs/scripts/tutorial_promises2.py +++ b/src/toil/test/docs/scripts/tutorial_promises2.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def binaryStrings(job, depth, message=""): diff --git a/src/toil/test/docs/scripts/tutorial_quickstart.py b/src/toil/test/docs/scripts/tutorial_quickstart.py index 1ea771e6c7..4bea3d9e07 100644 --- a/src/toil/test/docs/scripts/tutorial_quickstart.py +++ b/src/toil/test/docs/scripts/tutorial_quickstart.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp def helloWorld(message): diff --git a/src/toil/test/docs/scripts/tutorial_requirements.py b/src/toil/test/docs/scripts/tutorial_requirements.py index b5e15f4f13..8cfaa5ab28 100644 --- a/src/toil/test/docs/scripts/tutorial_requirements.py +++ b/src/toil/test/docs/scripts/tutorial_requirements.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job, PromisedRequirement +from toil.lib.io import mkdtemp def parentJob(job): diff --git a/src/toil/test/docs/scripts/tutorial_services.py b/src/toil/test/docs/scripts/tutorial_services.py index 825fc15bad..94bc7218f7 100644 --- a/src/toil/test/docs/scripts/tutorial_services.py +++ b/src/toil/test/docs/scripts/tutorial_services.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp class DemoService(Job.Service): diff --git a/src/toil/test/docs/scripts/tutorial_staging.py b/src/toil/test/docs/scripts/tutorial_staging.py index 3ddaf7973b..57a1431ed9 100644 --- a/src/toil/test/docs/scripts/tutorial_staging.py +++ b/src/toil/test/docs/scripts/tutorial_staging.py @@ -1,8 +1,8 @@ import os -from toil.lib.io import mkdtemp from toil.common import Toil from toil.job import Job +from toil.lib.io import mkdtemp class HelloWorld(Job): From 23b1cc7733dd3322603836b0e7d6e5ce53abaa12 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Tue, 24 Oct 2023 14:16:07 -0700 Subject: [PATCH 11/20] Use absolute-ized paths for work and coordination directories --- src/toil/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index eb7053fe18..5b4c1729a4 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -711,7 +711,7 @@ def __call__(self, parser: Any, namespace: Any, values: Any, option_string: Any logger.warning(f'Length of workDir path "{workDir}" is {len(workDir)} characters. ' f'Consider setting a shorter path with --workPath or setting TMPDIR to something ' f'like "/tmp" to avoid overly long paths.') - setattr(namespace, self.dest, values) + setattr(namespace, self.dest, workDir) class CoordinationDirAction(Action): """ @@ -725,7 +725,7 @@ def __call__(self, parser: Any, namespace: Any, values: Any, option_string: Any if not os.path.exists(coordination_dir): raise RuntimeError( f"The path provided to --coordinationDir ({coordination_dir}) does not exist.") - setattr(namespace, self.dest, values) + setattr(namespace, self.dest, coordination_dir) def make_closed_interval_action(min: Union[int, float], max: Optional[Union[int, float]] = None) -> Type[ Action]: From e0131f5b6ed1b734f809e62306311b02c173efe4 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 2 Nov 2023 12:20:12 -0400 Subject: [PATCH 12/20] Split CI run conditions into different rules --- .gitlab-ci.yml | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8f558097f3..408c1e7926 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -109,7 +109,9 @@ quick_test_offline: py38_appliance_build: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG || $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH stage: basic_tests script: - pwd @@ -120,7 +122,9 @@ py38_appliance_build: py39_appliance_build: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG || $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH stage: basic_tests script: - pwd @@ -131,7 +135,9 @@ py39_appliance_build: py310_appliance_build: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG || $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH stage: basic_tests script: - pwd @@ -195,7 +201,8 @@ batch_systems: slurm_test: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -205,7 +212,8 @@ slurm_test: cwl_v1.2: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -218,7 +226,8 @@ cwl_v1.2: cwl_on_arm: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -231,7 +240,8 @@ cwl_on_arm: cwl_misc: rules: - - if: $CI_PIPELINE_SOURCE != "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE != "schedule" + - if: $CI_COMMIT_TAG stage: main_tests script: - pwd @@ -295,7 +305,8 @@ provisioner: jobstore_integration: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -310,7 +321,8 @@ jobstore_integration: server_integration: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -326,7 +338,8 @@ server_integration: provisioner_integration: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -343,7 +356,8 @@ provisioner_integration: google_jobstore: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd @@ -359,7 +373,8 @@ google_jobstore: mesos: rules: - - if: $CI_PIPELINE_SOURCE == "schedule" || $CI_COMMIT_TAG + - if: $CI_PIPELINE_SOURCE == "schedule" + - if: $CI_COMMIT_TAG stage: integration script: - pwd From ddf653bce086ffa9f8398ae7375ef78826282751 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 2 Nov 2023 12:27:00 -0400 Subject: [PATCH 13/20] Run the CWL integration tests on any branch that changes the CWL runner --- .gitlab-ci.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 408c1e7926..a1b0c4ae8b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -214,6 +214,11 @@ cwl_v1.2: rules: - if: $CI_PIPELINE_SOURCE == "schedule" - if: $CI_COMMIT_TAG + - if: $CI_COMMIT_BRANCH + changes: + compare_to: 'refs/heads/master' + paths: + - 'src/toil/cwl/*' stage: integration script: - pwd @@ -221,13 +226,18 @@ cwl_v1.2: - python setup_gitlab_docker.py # login to increase the docker.io rate limit # Run CWL integration tests excluded from cwl_misc - make test threads="${TEST_THREADS}" tests="src/toil/test/cwl/cwlTest.py -k '(CWLWorkflowTest or cwl_small) and integrative'" - # Run CWL conformance tests + # Run CWL conformance tests, with file store bypassed - make test threads="${TEST_THREADS}" tests=src/toil/test/cwl/cwlTest.py::CWLv12Test::test_run_conformance_with_in_place_update cwl_on_arm: rules: - if: $CI_PIPELINE_SOURCE == "schedule" - if: $CI_COMMIT_TAG + - if: $CI_COMMIT_BRANCH + changes: + compare_to: 'refs/heads/master' + paths: + - 'src/toil/cwl/*' stage: integration script: - pwd @@ -236,6 +246,7 @@ cwl_on_arm: # This reads GITLAB_SECRET_FILE_SSH_KEYS - python setup_gitlab_ssh.py - chmod 400 /root/.ssh/id_rsa + # Run CWL conformance tests, on an ARM cluster on AWS, using the file store - make test threads="${TEST_THREADS}" tests=src/toil/test/cwl/cwlTest.py::CWLOnARMTest cwl_misc: From cb14501c6affd43d9f570dd17096e696c79ad33d Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 2 Nov 2023 17:49:58 -0400 Subject: [PATCH 14/20] Ship CWL test logs as junit files as unbounded size so we can get the failures --- .gitlab-ci.yml | 10 ++++++++ src/toil/test/cwl/cwlTest.py | 29 +++++++++++++++++++---- src/toil/test/provisioners/clusterTest.py | 11 +++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a1b0c4ae8b..ee64df9daa 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -228,6 +228,11 @@ cwl_v1.2: - make test threads="${TEST_THREADS}" tests="src/toil/test/cwl/cwlTest.py -k '(CWLWorkflowTest or cwl_small) and integrative'" # Run CWL conformance tests, with file store bypassed - make test threads="${TEST_THREADS}" tests=src/toil/test/cwl/cwlTest.py::CWLv12Test::test_run_conformance_with_in_place_update + artifacts: + reports: + junit: "*.junit.xml" + paths: + - "*.junit.xml cwl_on_arm: rules: @@ -248,6 +253,11 @@ cwl_on_arm: - chmod 400 /root/.ssh/id_rsa # Run CWL conformance tests, on an ARM cluster on AWS, using the file store - make test threads="${TEST_THREADS}" tests=src/toil/test/cwl/cwlTest.py::CWLOnARMTest + artifacts: + reports: + junit: "*.junit.xml" + paths: + - "*.junit.xml cwl_misc: rules: diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index d52a8d048e..b50d87ea03 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -903,12 +903,21 @@ def tearDown(self): @slow @pytest.mark.timeout(CONFORMANCE_TEST_TIMEOUT) def test_run_conformance(self, **kwargs): + if "junit_file" not in kwargs: + kwargs["junit_file"] = os.path.join( + self.rootDir, "conformance-1.2.junit.xml" + ) run_conformance_tests(workDir=self.cwlSpec, yml=self.test_yaml, **kwargs) @slow @pytest.mark.timeout(CONFORMANCE_TEST_TIMEOUT) def test_run_conformance_with_caching(self): - self.test_run_conformance(caching=True) + self.test_run_conformance( + caching=True, + junit_file = os.path.join( + self.rootDir, "caching-conformance-1.2.junit.xml" + ) + ) @slow @pytest.mark.timeout(CONFORMANCE_TEST_TIMEOUT) @@ -919,7 +928,10 @@ def test_run_conformance_with_in_place_update(self): features. """ self.test_run_conformance( - extra_args=["--bypass-file-store"], must_support_all_features=True + extra_args=["--bypass-file-store"], must_support_all_features=True, + junit_file = os.path.join( + self.rootDir, "in-place-update-conformance-1.2.junit.xml" + ) ) @slow @@ -927,7 +939,7 @@ def test_run_conformance_with_in_place_update(self): def test_kubernetes_cwl_conformance(self, **kwargs): if "junit_file" not in kwargs: kwargs["junit_file"] = os.path.join( - self.rootDir, "kubernetes-conformance.junit.xml" + self.rootDir, "kubernetes-conformance-1.2.junit.xml" ) return self.test_run_conformance( batchSystem="kubernetes", @@ -946,7 +958,7 @@ def test_kubernetes_cwl_conformance_with_caching(self): return self.test_kubernetes_cwl_conformance( caching=True, junit_file=os.path.join( - self.rootDir, "kubernetes-caching-conformance.junit.xml" + self.rootDir, "kubernetes-caching-conformance-1.2.junit.xml" ), ) @@ -1049,6 +1061,15 @@ def test_cwl_on_arm(self): ] ) + # We know if it succeeds it should save a junit XML for us to read. + # Bring it back to be an artifact. + self.rsync_util( + f":{self.cwl_test_dir}/toil/conformance-1.2.junit.xml", + os.path.join( + self._projectRootPath(), + "arm-conformance-1.2.junit.xml" + ) + ) @needs_cwl @pytest.mark.cwl_small_log_dir diff --git a/src/toil/test/provisioners/clusterTest.py b/src/toil/test/provisioners/clusterTest.py index 41f201b126..aef2b6224a 100644 --- a/src/toil/test/provisioners/clusterTest.py +++ b/src/toil/test/provisioners/clusterTest.py @@ -143,6 +143,17 @@ def sshUtil(self, command): log.error("Failed to run %s.", str(cmd)) raise subprocess.CalledProcessError(p.returncode, ' '.join(cmd)) + @retry(errors=[subprocess.CalledProcessError], intervals=[1, 1]) + def rsync_util(self, from_file: str, to_file: str) -> None: + """ + Transfer a file to/from the cluster. + + The cluster-side path should have a ':' in front of it. + """ + cmd = ['toil', 'rsync-cluster', '--insecure', '-p=aws', '-z', self.zone, self.clusterName, from_file, to_file] + log.info("Running %s.", str(cmd)) + subprocess.check_call(cmd) + @retry(errors=[subprocess.CalledProcessError], intervals=[1, 1]) def createClusterUtil(self, args=None): args = [] if args is None else args From a322c92b3c87baaf313953ca2dbd8e4c0eb83614 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 2 Nov 2023 18:58:35 -0400 Subject: [PATCH 15/20] Fix yaml --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ee64df9daa..cbcf0ebfcb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -232,7 +232,7 @@ cwl_v1.2: reports: junit: "*.junit.xml" paths: - - "*.junit.xml + - "*.junit.xml cwl_on_arm: rules: @@ -257,7 +257,7 @@ cwl_on_arm: reports: junit: "*.junit.xml" paths: - - "*.junit.xml + - "*.junit.xml cwl_misc: rules: From ac1c7a4d76d0a28f20b33f35fb64157b9616b0dc Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 2 Nov 2023 19:00:00 -0400 Subject: [PATCH 16/20] Add missing keys and close quotes --- .gitlab-ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cbcf0ebfcb..afe1cc62f1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -232,7 +232,9 @@ cwl_v1.2: reports: junit: "*.junit.xml" paths: - - "*.junit.xml + - "*.junit.xml" + when: always + expire_in: 14 day cwl_on_arm: rules: @@ -257,7 +259,9 @@ cwl_on_arm: reports: junit: "*.junit.xml" paths: - - "*.junit.xml + - "*.junit.xml" + when: always + expire_in: 14 day cwl_misc: rules: From 1f2df97b88064a036341ec58c5359f62e9cae134 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 9 Nov 2023 15:02:06 -0500 Subject: [PATCH 17/20] Revert "cwl: use the latest commit from the proposed CWL v1.2.1 branch (#4565)" This reverts commit 68e05e1aa42b1407e23d167bba6047756a6d8567. --- src/toil/test/cwl/cwlTest.py | 37 ++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index b50d87ea03..bb840590d7 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -98,11 +98,11 @@ def run_conformance_tests( :param batchSystem: If set, use this batch system instead of the default single_machine. - :param selected_tests: If set, use this list of tests to run (comma-separated test IDs) + :param selected_tests: If set, use this description of test numbers to run (comma-separated numbers or ranges) :param selected_tags: As an alternative to selected_tests, run tests with the given tags. - :param skipped_tests: Comma-separated string IDs of tests to skip. + :param skipped_tests: Comma-separated string labels of tests to skip. :param extra_args: Provide these extra arguments to runner for each test. @@ -121,7 +121,7 @@ def run_conformance_tests( f"--basedir={workDir}", ] if selected_tests: - cmd.append(f"-s={selected_tests}") + cmd.append(f"-n={selected_tests}") if selected_tags: cmd.append(f"--tags={selected_tags}") if skipped_tests: @@ -139,9 +139,10 @@ def run_conformance_tests( "--logDebug", "--statusWait=10", "--retryCount=2", - f"--caching={caching}" ] + args_passed_directly_to_runner.append(f"--caching={caching}") + if extra_args: args_passed_directly_to_runner += extra_args @@ -696,7 +697,7 @@ def setUp(self): self.workDir = os.path.join(self.cwlSpec, "v1.0") # The latest cwl git commit hash from https://github.com/common-workflow-language/common-workflow-language. # Update it to get the latest tests. - testhash = "06c0cba1a178e20af2634b33dee648faff144bf8" # Date: Thu Mar 23 19:07:05 2023 +0900 (move label to id) + testhash = "6a955874ade22080b8ef962b4e0d6e408112c1ef" # Date: Tue Dec 16 2020 8:43pm PST url = ( "https://github.com/common-workflow-language/common-workflow-language/archive/%s.zip" % testhash @@ -774,8 +775,9 @@ def test_kubernetes_cwl_conformance(self, **kwargs): return self.test_run_conformance( batchSystem="kubernetes", extra_args=["--retryCount=3"], - # This CWL v1.0 test doesn't work with Singularity; see - # https://github.com/common-workflow-language/cwltool/blob/9398f3253558b6c972033b5f4ac397a61f355556/conformance-test.sh#L97-L99 + # This test doesn't work with + # Singularity; see + # https://github.com/common-workflow-language/cwltool/blob/7094ede917c2d5b16d11f9231fe0c05260b51be6/conformance-test.sh#L99-L117 skipped_tests="docker_entrypoint", **kwargs, ) @@ -836,7 +838,7 @@ def setUpClass(cls): cls.test_yaml = os.path.join(cls.cwlSpec, "conformance_tests.yaml") # TODO: Use a commit zip in case someone decides to rewrite master's history? url = "https://github.com/common-workflow-language/cwl-v1.1.git" - commit = "b1d4a69df86350059bd49aa127c02be0c349f7de" + commit = "664835e83eb5e57eee18a04ce7b05fb9d70d77b7" p = subprocess.Popen( f"git clone {url} {cls.cwlSpec} && cd {cls.cwlSpec} && git checkout {commit}", shell=True, @@ -863,8 +865,9 @@ def test_kubernetes_cwl_conformance(self, **kwargs): return self.test_run_conformance( batchSystem="kubernetes", extra_args=["--retryCount=3"], - # These CWL v1.1 tests don't work with Singularity; see - # https://github.com/common-workflow-language/cwltool/blob/9398f3253558b6c972033b5f4ac397a61f355556/conformance-test.sh#L97-L105 + # These tests don't work with + # Singularity; see + # https://github.com/common-workflow-language/cwltool/blob/7094ede917c2d5b16d11f9231fe0c05260b51be6/conformance-test.sh#L99-L117 skipped_tests="docker_entrypoint,stdin_shorcut", **kwargs, ) @@ -889,7 +892,7 @@ def setUpClass(cls): cls.test_yaml = os.path.join(cls.cwlSpec, "conformance_tests.yaml") # TODO: Use a commit zip in case someone decides to rewrite master's history? url = "https://github.com/common-workflow-language/cwl-v1.2.git" - commit = "5f4a24cfd46aa9072d8418733b61a42261365b7b" + commit = "8c3fd9d9f0209a51c5efacb1c7bc02a1164688d6" p = subprocess.Popen( f"git clone {url} {cls.cwlSpec} && cd {cls.cwlSpec} && git checkout {commit}", shell=True, @@ -944,8 +947,9 @@ def test_kubernetes_cwl_conformance(self, **kwargs): return self.test_run_conformance( batchSystem="kubernetes", extra_args=["--retryCount=3"], - # This CWL v1.2 test doesn't work with Singularity; see - # https://github.com/common-workflow-language/cwltool/blob/9398f3253558b6c972033b5f4ac397a61f355556/conformance-test.sh#L97-L99 + # This test doesn't work with + # Singularity; see + # https://github.com/common-workflow-language/cwltool/blob/7094ede917c2d5b16d11f9231fe0c05260b51be6/conformance-test.sh#L99-L117 # and # https://github.com/common-workflow-language/cwltool/issues/1441#issuecomment-826747975 skipped_tests="docker_entrypoint", @@ -980,13 +984,18 @@ def test_wes_server_cwl_conformance(self): endpoint = os.environ.get("TOIL_WES_ENDPOINT") extra_args = [f"--wes_endpoint={endpoint}"] + # These are the ones that currently fail: + # - 310: mixed_version_v10_wf + # - 311: mixed_version_v11_wf + # - 312: mixed_version_v12_wf + # Main issues: # 1. `cwltool --print-deps` doesn't seem to include secondary files from the default # e.g.: https://github.com/common-workflow-language/cwl-v1.2/blob/1.2.1_proposed/tests/mixed-versions/wf-v10.cwl#L4-L10 return self.test_run_conformance( runner="toil-wes-cwl-runner", - skipped_tests="mixed_version_v10_wf,mixed_version_v11_wf,mixed_version_v12_wf", + selected_tests="1-309,313-337", extra_args=extra_args, ) From e3444bc321b82c2f8bb5a1c12a79fd3f0226e158 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 9 Nov 2023 15:02:25 -0500 Subject: [PATCH 18/20] Run CWL tests if they themselves change --- .gitlab-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index afe1cc62f1..15ad7148e9 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -219,6 +219,7 @@ cwl_v1.2: compare_to: 'refs/heads/master' paths: - 'src/toil/cwl/*' + - 'src/toil/test/cwl/*' stage: integration script: - pwd From 50d6ceaeef08c864a15b0bd970549763b0516138 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 9 Nov 2023 15:04:23 -0500 Subject: [PATCH 19/20] Turn off bioconda/biocontainers tests since they fail a nontrivial amount of the time because something at the other end is broken --- src/toil/test/cwl/cwlTest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index bb840590d7..7fe7fec14f 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -427,6 +427,7 @@ def test_load_contents_file(self): @slow @pytest.mark.integrative + @pytest.skip("Fails too often due to remote service") def test_bioconda(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl", @@ -438,6 +439,7 @@ def test_bioconda(self): @needs_docker @pytest.mark.integrative + @pytest.skip("Fails too often due to remote service") def test_biocontainers(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl", From 04ce883519e74a4266a01fa4da99474f6215c8d5 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Thu, 9 Nov 2023 15:05:44 -0500 Subject: [PATCH 20/20] Skip differently --- src/toil/test/cwl/cwlTest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index 7fe7fec14f..86452c24f0 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -427,7 +427,7 @@ def test_load_contents_file(self): @slow @pytest.mark.integrative - @pytest.skip("Fails too often due to remote service") + @unittest.skip def test_bioconda(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl", @@ -439,7 +439,7 @@ def test_bioconda(self): @needs_docker @pytest.mark.integrative - @pytest.skip("Fails too often due to remote service") + @unittest.skip def test_biocontainers(self): self._tester( "src/toil/test/cwl/seqtk_seq.cwl",