From aa1b7c996040026101e095526a06b088f88c0c36 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 6 Jul 2017 23:37:56 -0400 Subject: [PATCH 1/4] Refactor job script/dependency stuff to allow toil pickle-ing. Does this look cleaner anyway? I suspect yes? --- cwltool/builder.py | 10 +++++++++- cwltool/job.py | 17 ++++++++--------- cwltool/main.py | 4 ++-- cwltool/process.py | 7 +------ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/cwltool/builder.py b/cwltool/builder.py index 76a46a146..0a535800a 100644 --- a/cwltool/builder.py +++ b/cwltool/builder.py @@ -42,7 +42,6 @@ def __init__(self): # type: () -> None self.pathmapper = None # type: PathMapper self.stagedir = None # type: Text self.make_fs_access = None # type: Type[StdFsAccess] - self.build_job_script = None # type: Callable[[List[str]], Text] self.debug = False # type: bool self.mutation_manager = None # type: MutationManager @@ -51,6 +50,15 @@ def __init__(self): # type: () -> None self.loadListing = "deep_listing" # type: Union[None, str] self.find_default_container = None # type: Callable[[], Text] + self.job_script_provider = None # type: Any + + def build_job_script(self, commands): + # type: (List[str]) -> Text + build_job_script_method = getattr(self.job_script_provider, "build_job_script", None) # type: Callable[[Builder, List[str]], Text] + if build_job_script_method: + return build_job_script_method(self, commands) + else: + return None def bind_input(self, schema, datum, lead_pos=None, tail_pos=None): # type: (Dict[Text, Any], Any, Union[int, List[int]], List[int]) -> List[Dict[Text, Any]] diff --git a/cwltool/job.py b/cwltool/job.py index 6aea779c0..8498c3897 100644 --- a/cwltool/job.py +++ b/cwltool/job.py @@ -190,15 +190,19 @@ def _execute(self, runtime, env, rm_tmpdir=True, move_outputs="move"): os.makedirs(dn) stdout_path = absout - build_job_script = self.builder.build_job_script # type: Callable[[List[str]], Text] + commands = [Text(x).encode('utf-8') for x in runtime + self.command_line] + job_script_contents = None # type: Text + builder = getattr(self, "builder", None) # type: Builder + if builder is not None: + job_script_contents = builder.build_job_script(commands) rcode = _job_popen( - [Text(x).encode('utf-8') for x in runtime + self.command_line], + commands, stdin_path=stdin_path, stdout_path=stdout_path, stderr_path=stderr_path, env=env, cwd=self.outdir, - build_job_script=build_job_script, + job_script_contents=job_script_contents, ) if self.successCodes and rcode in self.successCodes: @@ -401,14 +405,9 @@ def _job_popen( env, # type: Union[MutableMapping[Text, Text], MutableMapping[str, str]] cwd, # type: Text job_dir=None, # type: Text - build_job_script=None, # type: Callable[[List[str]], Text] + job_script_contents=None, # type: Text ): # type: (...) -> int - - job_script_contents = None # type: Text - if build_job_script: - job_script_contents = build_job_script(commands) - if not job_script_contents and not FORCE_SHELLED_POPEN: stdin = None # type: Union[IO[Any], int] diff --git a/cwltool/main.py b/cwltool/main.py index 20bbc8c58..2e3f218df 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -728,10 +728,10 @@ def main(argsl=None, # type: List[str] make_tool_kwds = vars(args) - build_job_script = None # type: Callable[[Any, List[str]], Text] + job_script_provider = None # type: Callable[[Any, List[str]], Text] if conf_file or use_conda_dependencies: dependencies_configuration = DependenciesConfiguration(args) # type: DependenciesConfiguration - make_tool_kwds["build_job_script"] = dependencies_configuration.build_job_script + make_tool_kwds["job_script_provider"] = dependencies_configuration make_tool_kwds["find_default_container"] = functools.partial(find_default_container, args) diff --git a/cwltool/process.py b/cwltool/process.py index 7fdd99d3d..04d7c6e4d 100644 --- a/cwltool/process.py +++ b/cwltool/process.py @@ -598,12 +598,7 @@ def _init_job(self, joborder, **kwargs): builder.resources = self.evalResources(builder, kwargs) - build_job_script = kwargs.get("build_job_script", None) # type: Callable[[Builder, List[str]], Text] - curried_build_job_script = None # type: Callable[[List[str]], Text] - if build_job_script: - curried_build_job_script = lambda commands: build_job_script(builder, commands) - builder.build_job_script = curried_build_job_script - + builder.job_script_provider = kwargs.get("job_script_provider", None) return builder def evalResources(self, builder, kwargs): From 33d1e96b90054c94ba2d3f93fda9a6f6e4d2190a Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 6 Jul 2017 23:39:23 -0400 Subject: [PATCH 2/4] If a builder is supplied to single_job_executor - be sure the job uses it. This was required to get dependency management working with toil - but it looks really ugly so I suspect I am doing something suboptimially. --- cwltool/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cwltool/main.py b/cwltool/main.py index 2e3f218df..c7141b4b0 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -255,6 +255,9 @@ def output_callback(out, processStatus): try: for r in jobiter: if r: + builder = kwargs.get("builder", None) # type: Builder + if builder is not None: + r.builder = builder if r.outdir: output_dirs.add(r.outdir) r.run(**kwargs) From 5069580853d6f2287a590a4d9b87d8793ebb0470 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 7 Jul 2017 21:03:37 -0400 Subject: [PATCH 3/4] Revert broken change to "galaxy packages" test environment made in 5522e882d8feeb666230c5e19fbb3b5b2ccc634a. Not sure why it seemed like I needed that when testing the docs branch - clearly I hadn't originally and now it is broken after that change. --- tests/test_deps_env/random-lines/1.0/env.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_deps_env/random-lines/1.0/env.sh b/tests/test_deps_env/random-lines/1.0/env.sh index 48e1611b6..bdbb67f6f 100644 --- a/tests/test_deps_env/random-lines/1.0/env.sh +++ b/tests/test_deps_env/random-lines/1.0/env.sh @@ -4,5 +4,5 @@ # This shouldn't need to use bash-isms - but we don't know the full path to this file, # so for testing it is setup this way. For actual deployments just using full paths # directly would be preferable. -PACKAGE_DIRECTORY="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/tests/test_deps_env/random-lines/1.0/" +PACKAGE_DIRECTORY="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/" export PATH=$PATH:$PACKAGE_DIRECTORY/scripts From 8af03a6e34d6df7d16628bdb16c3bea68c6adabc Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 7 Jul 2017 21:17:05 -0400 Subject: [PATCH 4/4] Another type signature improvement? --- cwltool/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cwltool/main.py b/cwltool/main.py index c7141b4b0..94672ac96 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -21,6 +21,7 @@ from schema_salad.sourceline import strip_dup_lineno from . import draft2tool, workflow +from .builder import Builder from .cwlrdf import printdot, printrdf from .errors import UnsupportedRequirement, WorkflowException from .load_tool import fetch_document, make_tool, validate_document, jobloaderctx