From c8891a5eeec540404c7aff02a200fd95f6dbe64b Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 29 Jan 2018 15:58:46 -0500 Subject: [PATCH] 12764: Improve error handling around staging writable files Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- sdk/cwl/arvados_cwl/__init__.py | 10 +++++++--- sdk/cwl/arvados_cwl/arvtool.py | 2 +- services/crunch-run/crunchrun.go | 30 +++++++++++++++++++----------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py index af6ab33f1f..a55db8d53e 100644 --- a/sdk/cwl/arvados_cwl/__init__.py +++ b/sdk/cwl/arvados_cwl/__init__.py @@ -227,9 +227,13 @@ class ArvCwlRunner(object): if obj.get("writable") and self.work_api != "containers": raise SourceLine(obj, "writable", UnsupportedRequirement).makeError("InitialWorkDir feature 'writable: true' not supported with --api=jobs") if obj.get("class") == "DockerRequirement": - if obj.get("dockerOutputDirectory") and self.work_api != "containers": - raise SourceLine(obj, "dockerOutputDirectory", UnsupportedRequirement).makeError( - "Option 'dockerOutputDirectory' of DockerRequirement not supported with --api=jobs.") + if obj.get("dockerOutputDirectory"): + if self.work_api != "containers": + raise SourceLine(obj, "dockerOutputDirectory", UnsupportedRequirement).makeError( + "Option 'dockerOutputDirectory' of DockerRequirement not supported with --api=jobs.") + if not obj.get("dockerOutputDirectory").startswith('/'): + raise SourceLine(obj, "dockerOutputDirectory", validate.ValidationException).makeError( + "Option 'dockerOutputDirectory' must be an absolute path.") for v in obj.itervalues(): self.check_features(v) elif isinstance(obj, list): diff --git a/sdk/cwl/arvados_cwl/arvtool.py b/sdk/cwl/arvados_cwl/arvtool.py index 52f7d8a235..81faff44e6 100644 --- a/sdk/cwl/arvados_cwl/arvtool.py +++ b/sdk/cwl/arvados_cwl/arvtool.py @@ -37,7 +37,7 @@ class ArvadosCommandTool(CommandLineTool): def job(self, joborder, output_callback, **kwargs): if self.work_api == "containers": dockerReq, is_req = self.get_requirement("DockerRequirement") - if dockerReq.get("dockerOutputDirectory") and dockerReq.get("dockerOutputDirectory").startswith('/'): + if dockerReq and dockerReq.get("dockerOutputDirectory"): kwargs["outdir"] = dockerReq.get("dockerOutputDirectory") kwargs["docker_outdir"] = dockerReq.get("dockerOutputDirectory") else: diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go index 852d5a27b1..1f94973b18 100644 --- a/services/crunch-run/crunchrun.go +++ b/services/crunch-run/crunchrun.go @@ -577,20 +577,28 @@ func (runner *ContainerRunner) SetupMounts() (err error) { for _, cp := range copyFiles { dir, err := os.Stat(cp.src) - if err == nil { - if dir.IsDir() { - err = filepath.Walk(cp.src, func(walkpath string, walkinfo os.FileInfo, walkerr error) error { - if walkinfo.Mode().IsRegular() { - return copyfile(walkpath, path.Join(cp.bind, walkpath[len(cp.src):])) - } + if err != nil { + return fmt.Errorf("While staging writable file from %q to %q: %v", cp.src, cp.bind, err) + } + if dir.IsDir() { + err = filepath.Walk(cp.src, func(walkpath string, walkinfo os.FileInfo, walkerr error) error { + if walkerr != nil { + return walkerr; + } + if walkinfo.Mode().IsRegular() { + return copyfile(walkpath, path.Join(cp.bind, walkpath[len(cp.src):])) + } else if walkinfo.Mode().IsDir() { + // will be visited by Walk() return nil - }) - } else { - err = copyfile(cp.src, cp.bind) - } + } else { + return fmt.Errorf("Source %q is not a regular file or directory", cp.src) + } + }) + } else { + err = copyfile(cp.src, cp.bind) } if err != nil { - return fmt.Errorf("While staging writable files: %v", err) + return fmt.Errorf("While staging writable file from %q to %q: %v", cp.src, cp.bind, err) } } -- 2.30.2