miniwdl icon indicating copy to clipboard operation
miniwdl copied to clipboard

Accept WDL file as first argument to support shebang.

Open prihoda opened this issue 5 years ago • 2 comments

Since multiple runners are expected, it will be best to just provide the check/cromwell/... command when running the WDL file. This means that the shebang will essentially just create a shorthand from miniwdl COMMAND ./hello.wdl [ARGS...] to ./hello.wdl COMMAND [ARGS...].

Usage:

  1. Add #!/usr/bin/env miniwdl shebang line to your WDL file.

  2. Show help

./hello.wdl --help
# this translates to: miniwdl ./hello.wdl --help
  1. Check file
./hello.wdl check
# this translates to: miniwdl ./hello.wdl check
  1. Execute using cromwell
./hello.wdl cromwell name=MyName
# this translates to: miniwdl ./hello.wdl cromwell name=MyName
  1. Show normal help
miniwdl --help
# works as before, the new functionality is hidden:
# usage: miniwdl [-h] [--version] {check,cromwell} ...
#
# positional arguments:
#  {check,cromwell}
#    check           Load and typecheck a WDL document; show an outline with
#                    lint warnings
#    cromwell        Run workflow locally using Cromwell 40
#
# optional arguments:
#  -h, --help        show this help message and exit
#  --version         show miniwdl package version information

  1. Run non-existent file:
miniwdl ./nonExistent.wdl
# works as before - expects a command:
# usage: miniwdl [-h] [--version] {check,cromwell} ...
# miniwdl: error: argument command: invalid choice: './nonExistent.wdl' (choose from 'check', 'cromwell')

Introduced changes: Checks if first argument passed is not "cromwell" or "check" and is a valid file path instead, which means shebang was used. In such case, add the file path as the first argparse argument and skip adding it in the subparsers.

prihoda avatar Jun 17 '19 11:06 prihoda

Pull Request Test Coverage Report for Build 679

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.272%

Totals Coverage Status
Change from base Build 678: 0.0%
Covered Lines: 3280
Relevant Lines: 3407

💛 - Coveralls

coveralls avatar Jun 17 '19 11:06 coveralls

This is looking great! We should add some basic automated tests to prevent future subparser changes/refactoring from busting the shebang functionality.

I think the best way to set those up will be adding on to the existing bash-tap scripts, check.t and cromwell.t, probably a third .t file since the shebang script can invoke both 'check' and 'cromwell' (really cool touch!). The Makefile runs the .t scripts here. It may be tricky to figure out what to write on the shebang line for testing purposes, since we want it to invoke the development checkout of the miniwdl repo, not whatever released version pip has installed.

@kislyuk any advice on how/if argcomplete can do its thing via an executable shebang script (where the shebang invokes our python entry point) as contemplated here?

mlin avatar Jun 19 '19 06:06 mlin