Bug #11693

[Crunch2] Correctly handle symlinks to input files in output dir

Added by Peter Amstutz 2 months ago. Updated 2 months ago.

Status:ResolvedStart date:05/15/2017
Priority:NormalDue date:
Assignee:Peter Amstutz% Done:

100%

Category:-
Target version:2017-05-24 sprint
Story points1.0Remaining (hours)0.00 hour
Velocity based estimate-

Description

Crunch v1 and cwltool allow the output directory to have symlinks to input files, which are then efficiently propagated to output. Unfortunately, crunch-run fails in this case. Some scripts rely on this behavior. It needs to support dereferencing symlinks and adding them to the output collection.


Subtasks

Task #11696: Review 11693-crunch2-output-symlinksResolvedLucas Di Pentima

Associated revisions

Revision f26f70d0
Added by Peter Amstutz 2 months ago

Merge branch '11693-crunch2-output-symlinks' closes #11693

History

#1 Updated by Peter Amstutz 2 months ago

  • Description updated (diff)

#2 Updated by Peter Amstutz 2 months ago

  • Assignee set to Peter Amstutz

#3 Updated by Lucas Di Pentima 2 months ago

Reviewed a340487a7
Did a jenkins test run because my local tests kept failing for some reason, all ok: https://ci.curoverse.com/job/developer-run-tests/286/

Just a minor comment and a couple of questions:
  • File services/crunch-run/crunchrun.go:962: outputSuffix already assigned on L952
  • What should happen when the symlink is pointing to a mount, but that mount is not a collection? (asking because of the ...&& mnt.Kind == “collection” bit on L958). Can a mount be other thing than a collection? Shouldn't the collection mounts be filtered when building the binds list on L924?

#4 Updated by Peter Amstutz 2 months ago

Lucas Di Pentima wrote:

Reviewed a340487a7
Did a jenkins test run because my local tests kept failing for some reason, all ok: https://ci.curoverse.com/job/developer-run-tests/286/

Just a minor comment and a couple of questions:
  • File services/crunch-run/crunchrun.go:962: outputSuffix already assigned on L952

Copy and paste error. Fixed.

  • What should happen when the symlink is pointing to a mount, but that mount is not a collection? (asking because of the ...&& mnt.Kind == “collection” bit on L958). Can a mount be other thing than a collection? Shouldn't the collection mounts be filtered when building the binds list on L924?

Inputs are always collections. Symlinks to files in temporary directories isn't supported.

Filtering the binds array to just collections is a good idea. Fixed.

Now at aceb2d1ed239fa82fcb8bb352b632a8d92251dac

#5 Updated by Lucas Di Pentima 2 months ago

LGTM, thanks.

#6 Updated by Peter Amstutz 2 months ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:f26f70d0a60798065c5f7a5cb91b95587cc9e9ef.

Also available in: Atom PDF