Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass cmd args to toil-wdl-runner, resolves #2721 #2809

Closed
wants to merge 2 commits into from

Conversation

azzaea
Copy link
Contributor

@azzaea azzaea commented Oct 5, 2019

No description provided.

Copy link
Member

@adamnovak adamnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@azzaea This doesn't quite appear to work, according to our first batch of unit tests.

Specifically, src/toil/test/wdl/toilwdlTest.py::ToilWdlIntegrationTest::testPipe fails:

https://travis-ci.org/DataBiosphere/toil/jobs/594003669#L1048

It looks like when you run the argument parser, it doesn't know about the --out_dir argument that is supposed to be supported, so it bails.

You should be able to run the test yourself with:

virtualenv venv
. venv/bin/activate
pip install .
make test_offline tests_local=src/toil/test/wdl/toilwdlTest.py::ToilWdlIntegrationTest::testPipe

Can you please fix it? This looks like a very useful bugfix and we would like to merge it.

@azzaea
Copy link
Contributor Author

azzaea commented Oct 25, 2019

Oh! I see- I guess I was working with an overly simple example, so these did not show up. Thank you for your patience and clarification.

Will give it another shot this weekend

@azzaea
Copy link
Contributor Author

azzaea commented Oct 26, 2019

I think the last commit may do the trick, but not sure (I couldn't run the tests on my cluster, but tried with a more complicated wdl script and toil is now able to run it successfully. Also, tried to explicitly pass the out_dir argument to the toil-wdl-runner command, and as expected, the output file is created in that directory along with the compiled python script.

Is there an alternative way to run tests on my environment? (In the cluster, I can't follow the virtual environment approach described (worker nodes error about internet connection with pip install), and thus I resort to a classical python setup.py build & python setup.py install

@adamnovak
Copy link
Member

In order to run the tests the way I described, I think you need an Internet connection to set up the virtual environment.

If there's a way to set up a virtual environment without an Internet connection, I guess you could use that. Or you could try running your tests on a different machine that has Internet access; they don't need a particularly powerful system.

The Toil makefile insists that you do everything in a virtual environment. You can also bypass it by doing:

TRAVIS=true python -m pytest -vv --timeout=530 src/toil/test/wdl/toilwdlTest.py::ToilWdlIntegrationTest::testPipe

Our Travis tests are weirdly failing to run here. I will see if I can fix them.

@adamnovak adamnovak mentioned this pull request Oct 28, 2019
@adamnovak
Copy link
Member

OK, I got the tests to run.

It looks like one of them at least is still failing, due to looking for a jobStore argument that it can't find. I think the WDL generated code is maybe supposed to invent its own jobStore under the out_dir, and that logic isn't happening anymore?

�[31m�[1m_______________________ ToilWdlIntegrationTest.testPipe ________________________�[0m
Traceback (most recent call last):
  File "/home/travis/build/DataBiosphere/toil/src/toil/test/wdl/toilwdlTest.py", line 352, in testPipe
    ['python', self.program, wdl, json, '--out_dir', self.output_dir])
  File "/opt/python/3.5.6/lib/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['python', '/home/travis/build/DataBiosphere/toil/src/toil/wdl/toilwdl.py', '/home/travis/build/DataBiosphere/toil/src/toil/test/wdl/wdl_templates/testPipe/call.wdl', '/home/travis/build/DataBiosphere/toil/src/toil/test/wdl/wdl_templates/testPipe/call.json', '--out_dir', '/tmp/toil-wdl-test-de3689c0-b5fe-4cdc-85f2-5589086e7563']' returned non-zero exit status 1
----------------------------- Captured stderr call -----------------------------
usage: toilwdl_compiled.py [-h] [--logOff] [--logCritical] [--logError]
                           [--logWarning] [--logInfo] [--logDebug]
                           [--logLevel LOGLEVEL] [--logFile LOGFILE]
                           [--rotatingLogging] [--workDir WORKDIR]
                           [--noStdOutErr] [--stats]
                           [--clean {always,onError,never,onSuccess}]
                           [--cleanWorkDir {always,never,onSuccess,onError}]
                           [--clusterStats [CLUSTERSTATS]] [--restart]
                           [--batchSystem BATCHSYSTEM]
                           [--disableHotDeployment] [--disableAutoDeployment]
                           [--maxLocalJobs MAXLOCALJOBS] [--manualMemArgs]
                           [--runCwlInternalJobsOnWorkers]
                           [--parasolCommand PARASOLCOMMAND]
                           [--parasolMaxBatches PARASOLMAXBATCHES]
                           [--scale SCALE] [--linkImports]
                           [--mesosMaster MESOSMASTERADDRESS]
                           [--provisioner {aws,azure,gce}]
                           [--nodeTypes NODETYPES] [--nodeOptions NODEOPTIONS]
                           [--minNodes MINNODES] [--maxNodes MAXNODES]
                           [--targetTime TARGETTIME]
                           [--betaInertia BETAINERTIA]
                           [--scaleInterval SCALEINTERVAL]
                           [--preemptableCompensation PREEMPTABLECOMPENSATION]
                           [--nodeStorage NODESTORAGE] [--metrics]
                           [--maxServiceJobs MAXSERVICEJOBS]
                           [--maxPreemptableServiceJobs MAXPREEMPTABLESERVICEJOBS]
                           [--deadlockWait DEADLOCKWAIT]
                           [--statePollingWait STATEPOLLINGWAIT]
                           [--defaultMemory INT] [--defaultCores FLOAT]
                           [--defaultDisk INT] [--defaultPreemptable]
                           [--maxCores INT] [--maxMemory INT] [--maxDisk INT]
                           [--retryCount RETRYCOUNT]
                           [--maxJobDuration MAXJOBDURATION]
                           [--rescueJobsFrequency RESCUEJOBSFREQUENCY]
                           [--disableCaching [DISABLECACHING]]
                           [--disableChaining]
                           [--maxLogFileSize MAXLOGFILESIZE]
                           [--writeLogs [WRITELOGS]]
                           [--writeLogsGzip [WRITELOGSGZIP]]
                           [--writeLogsFromAllJobs] [--realTimeLogging]
                           [--sseKey SSEKEY] [--cseKey CSEKEY]
                           [--setEnv NAME=VALUE or NAME]
                           [--servicePollingInterval SERVICEPOLLINGINTERVAL]
                           [--forceDockerAppliance] [--debugWorker]
                           [--disableWorkerOutputCapture]
                           [--badWorker BADWORKER]
                           [--badWorkerFailInterval BADWORKERFAILINTERVAL]
                           jobStore
toilwdl_compiled.py: error: the following arguments are required: jobStore
Traceback (most recent call last):
  File "/home/travis/build/DataBiosphere/toil/src/toil/wdl/toilwdl.py", line 153, in <module>
    main()
  File "/home/travis/build/DataBiosphere/toil/src/toil/wdl/toilwdl.py", line 148, in main
    subprocess.check_call(cmd)
  File "/opt/python/3.5.6/lib/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/home/travis/virtualenv/python3.5.6/bin/python', '/tmp/toil-wdl-test-de3689c0-b5fe-4cdc-85f2-5589086e7563/toilwdl_compiled.py']' returned non-zero exit status 2
�[33m=============================== warnings summary ===============================�[0m

@DailyDreaming
Copy link
Member

@adamnovak That's accurate. I can take a look tomorrow and see what else would need to be reworked.

@melainalegaspi
Copy link

melainalegaspi commented Nov 30, 2021

@adamnovak : "This will be superseded by a WDL refactor to use miniWDL. Will close this then."

@adamnovak
Copy link
Member

The new WDL interpreter no longer has the problems that this fixes, so I am going to close out the PR.

@adamnovak adamnovak closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants