rabix / bunny

[Legacy] Executor for CWL workflows. Executes sbg:draft-2 and CWL 1.0
http://rabix.io
Apache License 2.0
74 stars 28 forks source link

TES backend bugfix #426

Closed adamstruck closed 6 years ago

adamstruck commented 6 years ago

I messed up a few things in the last few commits of my previous PR.

adamstruck commented 6 years ago

I also seem to be having issues with InitialWorkDirRequirement. The input file doesn't seem to be mapped into the working directory. Not sure what has changed since I opened #409 but now instead of the input being passed to Funnel twice it is passed once, but is mounted incorrectly.

So I am not sure if this is something I messed up or due to another change.

milos-ljubinkovic commented 6 years ago

Yeah, you removed one of the 2 getRequirements(bindings) calls. InitialWorkDirRequirement is only properly created/populated after the bindings.preprocess method is called so you need to put the second one back.

adamstruck commented 6 years ago

That did not fix the problem. This InitialWorkDirRequirement issue appears to be present prior to my previous PR.

adamstruck commented 6 years ago

I was able to fix this issue in https://github.com/rabix/bunny/pull/426/commits/9da61f58e4bf957dc972323ba92a593f092adf55. Can you confirm that this fix looks correct?

milos-ljubinkovic commented 6 years ago

Looks ok just add a null check for the resources, some tests are failing. Also test to see if this wf properly stages the secondary file on s3

adamstruck commented 6 years ago

The workflow you referenced does work and stages the secondary file output on s3.

The TES backend currently uploads all files in its working directory. I think we may want to be more specific about defining outputs in the TES message in the future, but the current approach seems to be working for now.