Closed chaselgrove closed 3 years ago
I'd model off of condor and slurm, but I only see them in test_orchestrators.py (no references to CondorSubmitter or SlurmSubmitter in tests).
Yeah, test_orchestrators is currently the only place that exercises the condor and slurm submitters. The class names aren't there, but the submitter name gets passed to the orchestrator class ("slurm", "condor", ...). For example, here's the slurm fixture and one of the slurm tests:
@pytest.fixture(scope="module")
def ssh_slurm():
skipif.no_ssh()
skipif.no_slurm()
from reproman.resource.ssh import SSH
return SSH("slurm-res", host="slurm")
[...]
@pytest.mark.integration
def test_orc_datalad_slurm(check_orc_datalad, ssh_slurm):
check_orc_datalad(ssh_slurm, orcs.DataladLocalRunOrchestrator, "slurm")
So, if you wanted to follow the same approach, you could start by s/slurm/lsf/
and then implementing no_lsf
in the skip
module.
So, if you wanted to follow the same approach, you could start by
s/slurm/lsf/
and then implementingno_lsf
in theskip
module.
Thanks. I've been trying both that and with a local environment, but the tests are failing for unclear reasons. I'll return to this next week.
Merging #553 (cdc583c) into master (9bf7580) will decrease coverage by
0.19%
. The diff coverage is43.63%
.
@@ Coverage Diff @@
## master #553 +/- ##
==========================================
- Coverage 89.84% 89.64% -0.20%
==========================================
Files 149 149
Lines 12888 12943 +55
==========================================
+ Hits 11579 11603 +24
- Misses 1309 1340 +31
Impacted Files | Coverage Δ | |
---|---|---|
reproman/support/jobs/submitters.py | 73.19% <33.33%> (-7.22%) |
:arrow_down: |
reproman/support/jobs/tests/test_orchestrators.py | 94.48% <63.15%> (-1.10%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 9bf7580...cdc583c. Read the comment docs.
The test works as it should, but:
I don't use skipif
because I use the presence of an environment variable to both trigger and configure the test.
Ideally I'd create job_spec_lsf
for the LSF test, but I don't see how pytest would let me use that elegantly (without duplicating check_orc_datalad
completely, the only change being check_orc_datalad(job_spec, dataset) -> check_orc_datalad_lsf(job_spec_lsf, dataset)
). So I patch job_spec
in check_orc_datalad
(though I suppose this could happen in job_spec
). Happy to consider a more elegant option.
The test works as it should, but:
I don't use
skipif
because I use the presence of an environment variable to both trigger and configure the test.
Works for me.
Ideally I'd create
job_spec_lsf
for the LSF test, but I don't see how pytest would let me use that elegantly (without duplicatingcheck_orc_datalad
completely, the only change beingcheck_orc_datalad(job_spec, dataset) -> check_orc_datalad_lsf(job_spec_lsf, dataset)
). So I patchjob_spec
incheck_orc_datalad
(though I suppose this could happen injob_spec
). Happy to consider a more elegant option.
Yeah, I think what you landed on is probably the best option.
While I don't know anything about LSF, nothing looks off to me in terms of where the changes are being made or the general content.
Thanks.
Let's proceed!
This PR adds an LSF submitter.
@kyleam, can you recommend the best way to test this? I'd model off of condor and slurm, but I only see them in test_orchestrators.py (no references to CondorSubmitter or SlurmSubmitter in tests).