Updatable inputs #317

tetron · 2017-03-02T22:20:33Z

Not ready to merge!

This is for tracking work on the feature of being able to safely update input files in-place.

tetron · 2017-03-03T15:11:36Z

Extends the existing functionality of staging "writable" files to the output directory. If inplaceUpdate is true, files will link (via symlink or bind mount) to the real files and permit updating.

Enforces sequencing of updates by incrementing a "generation" number with each update (*). Files can only be update by one workflow step at a time, and the updated file must be passed explicitly to downstream steps. If two steps try to update a file of the same generation it will raise an error.

(*) In writing this, I realized that while write-write conflicts are detected, read-write conflicts are not. So the lock management probably needs a bit more work.

tetron · 2017-03-07T02:49:24Z

@gijzelaerr this is ready for testing

gijzelaerr · 2017-03-07T09:17:03Z

awesome! Amazing! Thanks!

I'm trying it out now, but I run into troubles running your test/example:

gijs@sherlock:~/Work/cwltool (updatable-inputs) ✗
λ  .virtualenv/bin/cwltool tests/wf/updateval_inplace.cwl
.virtualenv/bin/cwltool 1.0.20170307024643
Resolved 'tests/wf/updateval_inplace.cwl' to 'file:///home/gijs/Work/cwltool/tests/wf/updateval_inplace.cwl'
Tool definition failed validation:
tests/wf/updateval_inplace.cwl:3:1: checking field `requirements`
tests/wf/updateval_inplace.cwl:8:3:   checking item
tests/wf/updateval_inplace.cwl:8:3:     Field `class` contains undefined reference to `file:///home/gijs/Work/cwltool/tests/wf/InplaceUpdateRequirement`

tetron · 2017-03-07T12:35:44Z

To enable the feature is hidden behind the flag --enable-ext

gijzelaerr · 2017-03-09T11:29:55Z

cwltool/job.py

+        if vol.type in ("File", "Directory") or (inplace_update and
+                                                 vol.type in ("WritableFile", "WritableDirectory")):
+            if os.path.exists(vol.target):
+                os.remove(vol.target)


this is giving me:

Traceback (most recent call last): File "/home/gijs/Work/cwltool/cwltool/job.py", line 209, in _execute relink_initialworkdir(self.generatemapper, inplace_update=self.inplace_update) File "/home/gijs/Work/cwltool/cwltool/job.py", line 99, in relink_initialworkdir os.remove(vol.target) OSError: [Errno 21] Is a directory: '/tmp/tmpC7NZ5I/1484694604_1284.hh_vv.ms' [job casa_flagdata.cwl] completed permanentFail {}

gijzelaerr · 2017-03-09T11:32:21Z

This fails in the case of a Directory input/output

Traceback (most recent call last):
  File "/home/gijs/Work/cwltool/cwltool/job.py", line 209, in _execute
    relink_initialworkdir(self.generatemapper, inplace_update=self.inplace_update)
  File "/home/gijs/Work/cwltool/cwltool/job.py", line 99, in relink_initialworkdir
    os.remove(vol.target)
OSError: [Errno 21] Is a directory: '/tmp/tmpC7NZ5I/1484694604_1284.hh_vv.ms'
[job casa_flagdata.cwl] completed permanentFail
{}

here is my CWL file
https://github.com/gijzelaerr/vermeerkat-cwl/blob/99d6e9456f18dc3e0a20f65729b27da6c8b02fb9/cwl/casa_flagdata.cwl

tetron · 2017-03-10T20:39:45Z

Oh, I forgot to even test it for WritableDirectory. Will take a look.

tetron · 2017-03-10T23:32:05Z

@gijzelaerr try it now, updatable directories should be fixed.

mr-c · 2017-03-11T13:05:56Z

@tetron FYI: I merged in master

gijzelaerr · 2017-03-13T08:56:28Z

I think the merge from master broke something?

 .virtualenv/bin/cwltool --enable-ext --outdir results --cachedir cache cwl/workflow.cwl cwl/job.cwl
.virtualenv/bin/cwltool 1.0.20170307024643
Resolved 'cwl/workflow.cwl' to 'file:///home/gijs/Work/vermeerkat-cwl/cwl/workflow.cwl'
Tool definition failed initialization:
cwl/aoflagger.cwl:15:5: Unsupported requirement InplaceUpdateRequirement

gijzelaerr · 2017-03-13T08:57:20Z

yes, f812858 seems to eat the InplaceUpdateRequirement just fine.

gijzelaerr · 2017-03-13T09:10:47Z

f812858 gives me

 .virtualenv/bin/cwltool --enable-ext --outdir results --cachedir cache cwl/workflow.cwl cwl/job.cwl
.virtualenv/bin/cwltool 1.0.20170310233005
Resolved 'cwl/workflow.cwl' to 'file:///home/gijs/Work/vermeerkat-cwl/cwl/workflow.cwl'
[job curl] Using cached output in /home/gijs/Work/vermeerkat-cwl/cache/e264380f650712a0bed41bd4e101b960
invalid field `_generation`, expected one of: 'class', 'location', 'path', 'basename', 'dirname', 'nameroot', 'nameext', 'checksum', 'size', 'secondaryFiles', 'format', 'contents'
Got workflow error
Traceback (most recent call last):
  File "/home/gijs/Work/cwltool/cwltool/main.py", line 238, in single_job_executor
    r.run(**kwargs)
  File "/home/gijs/Work/cwltool/cwltool/draft2tool.py", line 144, in run
    kwargs.get("compute_checksum", True)), "success")
  File "/home/gijs/Work/cwltool/cwltool/draft2tool.py", line 474, in collect_output_ports
    for r in readers.values():
AttributeError: 'NoneType' object has no attribute 'values'
Workflow error, try again with --debug for more information:
'NoneType' object has no attribute 'values'

mr-c · 2017-03-14T00:14:59Z

Oops, sorry. Looks like I did the merge from a stale browser window. I'll fix it now

mr-c · 2017-03-14T00:19:28Z

@tetron I've undone the merge from master with a force push and I'll keep my hands off this PR; mea culpa!

…ied in place.

Conflicts: cwltool/process.py

…lasses.

aa320ec Merge pull request common-workflow-language#342 from common-workflow-language/fix-mystery-package-errors 3c6eaff Update for rename of argparse2cwl to argparse2tool b14404b remove errant jsonldPredicate 7f116da Merge pull request common-workflow-language#339 from common-workflow-language/dockerOutputDirectory 4f65d1f Add test for dockerOutputDirectory option of DockerRequirement. cb1f928 Merge pull request common-workflow-language#335 from common-workflow-language/output-literals d8b6c8f Add tests for file and directory literals in output as produced by expression tool. ee3bb1c Merge pull request common-workflow-language#324 from common-workflow-language/secondaryfiles-array 5e6c2d4 Add NLeSC's Xenon cbe1c8d Merge pull request common-workflow-language#334 from StarvingMarvin/master 8300e43 output ids must start with '#' in draft-2 f87d218 draft-3 test fixes: explicit output for args 8b10319 Fixes for draft-2 tests: adding explicit args output 83ed4b4 Merge remote-tracking branch 'rabix-cwl/master' e0a5e7c Merge pull request common-workflow-language#328 from StarvingMarvin/master d906c4a dir4 test fix 21bdcaf Merge remote-tracking branch 'upstream/master' dee0f4e Disabled checksum check for listing input dir fc4cb0f Added args output and empty.json for null jobs a4950f4 Update tools list 8e1d67c Add Rabix Bunny CI badge 2a5b95e fixing mentions to draft3/4 1356c45 Add test for secondaryFiles with arrays b60125b Single instead of double quotes in doc 179835b Merge pull request common-workflow-language#319 from common-workflow-language/test-cwl-out2 ed40737 Add additional test for cwl.output.json behavior. a217bb6 Merge pull request common-workflow-language#317 from StarvingMarvin/master 8dd69f2 fixing test in v1.0 and v1.1.0-dev1 e831c20 Merge pull request common-workflow-language#314 from common-workflow-language/test-requirements-on-steps 8cad6f1 Add test for requirements/hints on workflow steps. 7679adb Merge pull request common-workflow-language#308 from common-workflow-language/embedded-subworkflow-test 1582677 Tighten up formatting 779c2d4 Add test for embedded subworkflow e22a9c2 Merge pull request common-workflow-language#307 from alaindomissy/patch-3 125cb5f Update UserGuide.yml 99f4f9f Merge pull request common-workflow-language#303 from common-workflow-language/include-stdin-in-docs 5324dbb put stdin shortcut in correct place git-subtree-dir: cwltool/schemas git-subtree-split: aa320ec

gijzelaerr · 2017-03-27T13:43:22Z

just did a rerun without debug, I think i have the same error

[step casa_flagdata] completed success
[workflow workflow.cwl] outdir is /home/gijs/Work/vermeerkat-cwl/cache/R88qhS
Unhandled error, try again with --debug for more information:
  [Errno 21] Is a directory: '/home/gijs/Work/vermeerkat-cwl/cache/cb5bf370951c5f2a7b4538e0e0cb7d41/result.ms'
make: *** [run] Error 1

tetron · 2017-03-29T20:36:41Z

@gijzelaerr I'm pretty swamped at the moment, would you be able to look into this? It looks like right at the end in relocateOutputs it assumes it is only copying files, it needs to detect if something is a directory and use shutil.copytree.

gijzelaerr · 2017-03-30T08:12:35Z

It seems to be a bit more complex than that. When I add support for recursive dir copy there I get:

[workflow workflow.cwl] outdir is /home/gijs/Work/vermeerkat-cwl/cache/kx9MZe
Copying /home/gijs/Work/vermeerkat-cwl/cache/a1f3006db20fae39c82ec21e8297dd1c/result.ms to results/result.ms
Unhandled error, try again with --debug for more information:
  [Errno 2] No such file or directory: u'/result.ms/table.f7_TSM1'
Traceback (most recent call last):
  File "/home/gijs/Work/cwltool/cwltool/main.py", line 768, in main
    **vars(args))
  File "/home/gijs/Work/cwltool/cwltool/main.py", line 260, in single_job_executor
    kwargs["make_fs_access"](""))
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 259, in relocateOutputs
    adjustFileObjs(outputObj, _check_adjust)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 41, in adjustFileObjs
    adjustFileObjs(rec[d], op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 41, in adjustFileObjs
    adjustFileObjs(rec[d], op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 44, in adjustFileObjs
    adjustFileObjs(d, op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 39, in adjustFileObjs
    op(rec)
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 256, in _check_adjust
    compute_checksums(fs_access, f)
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 798, in compute_checksums
    with fs_access.open(fileobj["location"], "rb") as f:
  File "/home/gijs/Work/cwltool/cwltool/stdfsaccess.py", line 26, in open
    return open(self._abs(fn), mode)
IOError: [Errno 2] No such file or directory: u'/result.ms/table.f7_TSM1'

Conflicts: cwltool/extensions.yml

tetron · 2017-03-31T13:46:16Z

@gijzelaerr how is it working now?

gijzelaerr · 2017-04-04T10:04:16Z

same error, full output here.

https://gist.github.com/gijzelaerr/11115e8ece7ef5e68e4a0823a77a24bd

mr-c · 2017-04-04T11:01:50Z

@gijzelaerr Thanks for the update. Can you try again w/o caching?

gijzelaerr · 2017-04-06T09:06:09Z

same.

$   .virtualenv/bin/cwltool --debug --enable-ext --outdir results/ --tmpdir-prefix tmp/ --tmp-outdir-prefix tmp-output/ cwl/workflow.cwl cwl/job.cwl

[...]

[job casa_flagdata] Removing input staging directory /home/gijs/Work/vermeerkat-cwl/tmp/eaDlUr
[job casa_flagdata] Removing temporary directory /home/gijs/Work/vermeerkat-cwl/tmp/ROxfh_
[workflow workflow.cwl] outdir is /home/gijs/Work/vermeerkat-cwl/tmp-output/ORBBrQ
Moving /home/gijs/Work/vermeerkat-cwl/tmp-output/NrCwpP/result.ms to results/result.ms
Unhandled error, try again with --debug for more information:
  [Errno 2] No such file or directory: u'/result.ms/table.f7_TSM1'
Traceback (most recent call last):
  File "/home/gijs/Work/cwltool/cwltool/main.py", line 776, in main
    **vars(args))
  File "/home/gijs/Work/cwltool/cwltool/main.py", line 260, in single_job_executor
    kwargs["make_fs_access"](""))
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 263, in relocateOutputs
    adjustFileObjs(outputObj, _check_adjust)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 41, in adjustFileObjs
    adjustFileObjs(rec[d], op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 41, in adjustFileObjs
    adjustFileObjs(rec[d], op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 44, in adjustFileObjs
    adjustFileObjs(d, op)
  File "/home/gijs/Work/cwltool/cwltool/pathmapper.py", line 39, in adjustFileObjs
    op(rec)
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 260, in _check_adjust
    compute_checksums(fs_access, f)
  File "/home/gijs/Work/cwltool/cwltool/process.py", line 802, in compute_checksums
    with fs_access.open(fileobj["location"], "rb") as f:
  File "/home/gijs/Work/cwltool/cwltool/stdfsaccess.py", line 26, in open
    return open(self._abs(fn), mode)
IOError: [Errno 2] No such file or directory: u'/result.ms/table.f7_TSM1'

tetron · 2017-04-06T13:19:10Z

@gijzelaerr as a workaround, can you try --no-compute-checksum ?

Looks like it is trying to resolve a relative path without having a base directory to start from.

gijzelaerr · 2017-04-07T09:53:29Z

No luck, same issue.

$  .virtualenv/bin/cwltool --no-compute-checksum --debug --enable-ext --outdir results/ --tmpdir-prefix tmp/ --tmp-outdir-prefix tmp-output/ cwl/workflow.cwl cwl/job.cwl

[...]
IOError: [Errno 2] No such file or directory: u'/result.ms/table.f7_TSM1'

mr-c · 2017-04-07T09:59:56Z

@gijzelaerr Bummer. How hard would it be to make a complete minimal test case with data included?

gijzelaerr · 2017-04-07T11:34:24Z

here you go. https://github.com/gijzelaerr/cwlfail

actually i think I isolated the error a bit more, it only occurs if you specify the --outdir

Conflicts: cwltool/draft2tool.py cwltool/process.py

tetron · 2017-05-10T15:11:58Z

@gijzelaerr thanks for the minimal test case, it was extremely helpful.

A bit late on this, but moving Directory outputs to their final location should be much improved now (your test case does the right thing).

fixing test in v1.0 and v1.1.0-dev1

tetron force-pushed the updatable-inputs branch from db84a1e to 3911170 Compare March 3, 2017 19:24

gijzelaerr reviewed Mar 9, 2017

View reviewed changes

mr-c force-pushed the updatable-inputs branch from 3557183 to f812858 Compare March 14, 2017 00:18

Peter Amstutz added 14 commits March 16, 2017 11:18

Implement mutation manager to validate that files can be safely modif…

2f9136d

…ied in place.

Test workflows for mutable inputs.

ca0611b

Add in-place update tests.

da1cea7

Fix elif

33a29c8

Set up Docker binds to support writable items.

e454c20

Didn't break anything.

a09cf8f

Inplace update works with & without Docker.

3c6bf89

Add InplaceUpdateRequirement.

887c685

Conflicts: cwltool/process.py

Refactor job to split Docker and non-Docker execution into separate c…

055efd9

…lasses.

Manage readers for inplace updates.

d0168e9

Record readers, last update step.

8c3e6d0

Add testing.

5cf82f9

Test & mypy fixes WIP.

a8d2a97

Make mypy happying.

35966c9

gijzelaerr and others added 3 commits March 30, 2017 10:15

Recursive copy (#356)

bef5afc

Merge branch 'master' into updatable-inputs

caa992a

Conflicts: cwltool/extensions.yml

Fix lint

5c9fc20

Peter Amstutz and others added 8 commits April 28, 2017 14:58

Merge branch 'master' into updatable-inputs

10ff572

Conflicts: cwltool/draft2tool.py cwltool/process.py

Merge branch 'master' into updatable-inputs

a80d8f1

Use logger filter, less hacky and should make tox happier.

07423ec

Better handling for relocating directory outputs to final output.

ca81c56

Tox fix

328d871

Fix outputdir

d86c923

Tox 2

cf5cad3

Tox 3

8ff477c

tetron merged commit a91c16c into master May 10, 2017

tetron deleted the updatable-inputs branch May 10, 2017 15:13

mr-c mentioned this pull request May 17, 2017

removal of warning due to updatable inputs feature #382

Closed

tetron pushed a commit that referenced this pull request Jul 23, 2017

Merge pull request #317 from StarvingMarvin/master

a217bb6

fixing test in v1.0 and v1.1.0-dev1

mr-c mentioned this pull request Dec 6, 2017

Be able to perform read/write operations on input common-workflow-language/common-workflow-language#380

Closed

3 tasks

psafont mentioned this pull request Aug 7, 2018

Make data relocation at the end of workflow runs faster #850

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updatable inputs #317

Updatable inputs #317

tetron commented Mar 2, 2017

tetron commented Mar 3, 2017

tetron commented Mar 7, 2017

gijzelaerr commented Mar 7, 2017

tetron commented Mar 7, 2017

gijzelaerr Mar 9, 2017

gijzelaerr commented Mar 9, 2017

tetron commented Mar 10, 2017

tetron commented Mar 10, 2017

mr-c commented Mar 11, 2017

gijzelaerr commented Mar 13, 2017

gijzelaerr commented Mar 13, 2017

gijzelaerr commented Mar 13, 2017

mr-c commented Mar 14, 2017

mr-c commented Mar 14, 2017

gijzelaerr commented Mar 27, 2017

tetron commented Mar 29, 2017

gijzelaerr commented Mar 30, 2017

tetron commented Mar 31, 2017

gijzelaerr commented Apr 4, 2017

mr-c commented Apr 4, 2017

gijzelaerr commented Apr 6, 2017

tetron commented Apr 6, 2017

gijzelaerr commented Apr 7, 2017

mr-c commented Apr 7, 2017

gijzelaerr commented Apr 7, 2017

tetron commented May 10, 2017

Updatable inputs #317

Updatable inputs #317

Conversation

tetron commented Mar 2, 2017

tetron commented Mar 3, 2017

tetron commented Mar 7, 2017

gijzelaerr commented Mar 7, 2017

tetron commented Mar 7, 2017

gijzelaerr Mar 9, 2017

Choose a reason for hiding this comment

gijzelaerr commented Mar 9, 2017

tetron commented Mar 10, 2017

tetron commented Mar 10, 2017

mr-c commented Mar 11, 2017

gijzelaerr commented Mar 13, 2017

gijzelaerr commented Mar 13, 2017

gijzelaerr commented Mar 13, 2017

mr-c commented Mar 14, 2017

mr-c commented Mar 14, 2017

gijzelaerr commented Mar 27, 2017

tetron commented Mar 29, 2017

gijzelaerr commented Mar 30, 2017

tetron commented Mar 31, 2017

gijzelaerr commented Apr 4, 2017

mr-c commented Apr 4, 2017

gijzelaerr commented Apr 6, 2017

tetron commented Apr 6, 2017

gijzelaerr commented Apr 7, 2017

mr-c commented Apr 7, 2017

gijzelaerr commented Apr 7, 2017

tetron commented May 10, 2017