Skip to content

Commit

Permalink
Fix WDL argument parsing for 3.12 behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
adamnovak committed Nov 12, 2024
1 parent 1c8faf0 commit fb71178
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 16 deletions.
10 changes: 9 additions & 1 deletion src/toil/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -801,15 +801,23 @@ def check_arguments(typ: str) -> None:
# if wdl is set, format the namespace for wdl and check that cwl options are not set on the command line
if wdl:
parser.add_argument("wdl_uri", type=str, help="WDL document URI")
# We want to have an inputs_url that can be either a positional or a flag.
# We can't just have them share a single-item dest in Python 3.12;
# argparse does not guarantee that will work, and we can get the
# positional default value clobbering the flag. See
# <https://stackoverflow.com/a/60531838>.
# So we make them accumulate to the same list.
# Note that we will get a None in the list when there's no positional inputs.
parser.add_argument(
"inputs_uri", type=str, nargs="?", help="WDL input JSON URI"
"inputs_uri", type=str, nargs='?', action="append", help="WDL input JSON URI"
)
parser.add_argument(
"--input",
"--inputs",
"-i",
dest="inputs_uri",
type=str,
action="append",
help="WDL input JSON URI",
)
check_arguments(typ="wdl")
Expand Down
62 changes: 47 additions & 15 deletions src/toil/wdl/wdltoil.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
)
from toil.lib.accelerators import get_individual_local_accelerators
from toil.lib.conversions import VALID_PREFIXES, convert_units, human2bytes
from toil.lib.integration import resolve_workflow
from toil.lib.integration import resolve_workflow
from toil.lib.io import mkdtemp
from toil.lib.memoize import memoize
from toil.lib.misc import get_user_name
Expand Down Expand Up @@ -5325,6 +5325,15 @@ def main() -> None:
"Commandline arguments --runImportsOnWorkers and --importWorkersDisk must both be set to run file imports on workers."
)

# Having an nargs=? option can put a None in our inputs list, so drop that.
input_sources = [x for x in options.inputs_uri if x is not None]
if len(input_sources) > 1:
raise RuntimeError(
f"Workflow inputs cannot be specified with both the -i/--input/--inputs flag "
f"and as a positional argument at the same time. Cannot use both "
f"\"{input_sources[0]}\" and \"{input_sources[1]}\"."
)

# Make sure we have an output directory (or URL prefix) and we don't need
# to ever worry about a None, and MyPy knows it.
# If we don't have a directory assigned, make one in the current directory.
Expand All @@ -5339,6 +5348,10 @@ def main() -> None:
if options.restart:
output_bindings = toil.restart()
else:
# TODO: Move all the input parsing outside the Toil context
# manager to avoid leaving a job store behind if the workflow
# can't start.

# Load the WDL document
document: WDL.Tree.Document = WDL.load(
resolve_workflow(options.wdl_uri, supported_languages={"WDL"}),
Expand Down Expand Up @@ -5381,30 +5394,49 @@ def main() -> None:
)
options.all_call_outputs = True

if options.inputs_uri:
# If our input really comes from a URI or path, remember it.
input_source_uri = None
# Also remember where we need to report JSON parse errors as
# coming from if there's no actual URI/path.
input_source_name = "empty input"

if input_sources:
input_source = input_sources[0]
# Load the inputs. Use the same loading mechanism, which means we
# have to break into async temporarily.
if options.inputs_uri[0] == "{":
input_json = options.inputs_uri
elif options.inputs_uri == "-":
if input_source[0] == "{":
input_json = input_source
input_source_name = "inline JSON"
elif input_source == "-":
input_json = sys.stdin.read()
input_source_name = "standard input"
else:
input_source_uri = input_source
input_source_name = input_source_uri
input_json = asyncio.run(
toil_read_source(options.inputs_uri, [], None)
toil_read_source(input_source_uri, [], None)
).source_text
try:
inputs = json.loads(input_json)
except json.JSONDecodeError as e:
# Complain about the JSON document.

# We need the absolute path or URL to raise the error
inputs_abspath = (
options.inputs_uri
if not os.path.exists(options.inputs_uri)
else os.path.abspath(options.inputs_uri)
)
if input_source_uri is not None:
# If this is a local fike, use that as the abspath.
# Otherwise just pass through a URL.
inputs_abspath = (
input_source_uri
if not os.path.exists(input_source_uri)
else os.path.abspath(input_source_uri)
)
else:
# There's no local file and no URL.
inputs_abspath = input_source_name

raise WDL.Error.ValidationError(
WDL.Error.SourcePosition(
options.inputs_uri,
input_source_name,
inputs_abspath,
e.lineno,
e.colno,
Expand Down Expand Up @@ -5434,12 +5466,12 @@ def main() -> None:

# Determine where to look for files referenced in the inputs, in addition to here.
inputs_search_path = []
if options.inputs_uri:
inputs_search_path.append(options.inputs_uri)
if input_source_uri:
inputs_search_path.append(input_source_uri)

match = re.match(
r"https://raw\.githubusercontent\.com/[^/]*/[^/]*/[^/]*/",
options.inputs_uri,
input_source_uri,
)
if match:
# Special magic for Github repos to make e.g.
Expand Down

0 comments on commit fb71178

Please sign in to comment.