Arvados

Am able to reproduce with the following workflow:

class: ExpressionTool
cwlVersion: v1.0

inputs:
  inputfile: File
outputs:
  out: Directory[]
requirements:
  InlineJavascriptRequirement: {}
expression: |
  ${
    var dirs = [];
    dirs.push({"class": "Directory",
               "basename": "testdir1",
               "listing": [inputs.inputfile]});
    dirs.push({"class": "Directory",
               "basename": "testdir2",
               "listing": [inputs.inputfile]});
    return {"out": dirs};
  }

and input file:

inputfile:
  class: File
  path: test.txt

The 14723-cwl-multiple-file-targets branch at 810fc3e7aab72aa5ae951141955b56f6d4e47cfd now maps files from their target destination instead. The exception is for literals, which store file contents instead of a resolved location.

I also added a test for this and found a bug where an ExpressionTool with a file array output of literals with the same name would not use name conflict resolution. I fixed this and also added a test.

Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1185/
C/I Tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/89/

Re-run of service/api tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests-services-api/1210/

Please add a comment about how StagingPathMapper, instead of internally mapping source → target, maps targets → source.

Also how mapper() is overridden to maintain the behavior of mapping source → target

Also how literals are a special case, they are still mapped by source (identifier) → target

                if v.resolved == src or (v.type == "CreateFile" and k == src):
                    return v

Let's make this logic exclusive:

if (v.type != "CreateFile" and v.resolved == src) or (v.type == "CreateFile" and k == src):

The code blocks at arvcontainer.py:150 and executor.py:444 are very similar if not identical in function, I wonder if they could be refactored to use the same logic.

Unrelated to this immediate bug, but I noticed this in executor.py:464

            except IOError as e:
                logger.warning("While preparing output collection: %s", e)

Seems like that should be logger.error() and/or re-raise the exception.

Peter Amstutz wrote:

Please add a comment about how StagingPathMapper, instead of internally mapping source → target, maps targets → source.

Also how mapper() is overridden to maintain the behavior of mapping source → target

Also how literals are a special case, they are still mapped by source (identifier) → target

Done

[...]

Let's make this logic exclusive:

[...]

Fixed

The code blocks at arvcontainer.py:150 and executor.py:444 are very similar if not identical in function, I wonder if they could be refactored to use the same logic.

I don't see a huge benefit to refactoring this at the moment, at least as a part of this bug. Maybe we can add a ticket for this?

Unrelated to this immediate bug, but I noticed this in executor.py:464

[...]

Seems like that should be logger.error() and/or re-raise the exception.

Fixed

Latest at 454ee2b8f0385c542b6f1165a3baf2820425e1a3.
Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1195/
C/I Tests: Run and passed on my machine.

Eric Biagiotti wrote:

Latest at 454ee2b8f0385c542b6f1165a3baf2820425e1a3.
Unit Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1195/
C/I Tests: Run and passed on my machine.

LGTM