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

Fixes for issue #94... #95

Merged
merged 7 commits into from
Aug 30, 2022
Merged

Fixes for issue #94... #95

merged 7 commits into from
Aug 30, 2022

Conversation

marcmengel
Copy link
Contributor

There are several DAG issues in #94 that this resolves:

  1. resources for dagbegin/end jobs (now we just specify fixed memory for them, not the resources from the submit line)
  2. getting distinct nodenames
  3. JOBSUBJOBSECTION value in each job (actually not mentioned in DAG issues  #94 but one I noticed while going through it).

[mengel@fifeutilgpvm01 2022_08_26_142625.bda07eeb-c497-4790-a5f9-c736dc144bfe]$ jobsub_q -G fermilab -nobatch -dag 792@jobsubdevgpvm01.fnal.gov

-- Schedd: jobsubdevgpvm01.fnal.gov : <131.225.240.23:9615?... @ 08/26/22 14:27:02
 ID      OWNER/NODENAME      SUBMITTED     RUN_TIME ST PRI SIZE CMD
 792.0   mengel             8/26 14:26   0+00:00:35 R  0    0.0 dagman_wrapper.
 794.0    |-WORKER_0        8/26 14:26   0+00:00:00 I  0    0.0 simple.sh --fin
 795.0    |-WORKER_1        8/26 14:26   0+00:00:00 I  0    0.0 simple.sh --fin
 796.0    |-WORKER_2        8/26 14:26   0+00:00:00 I  0    0.0 simple.sh --fin
 797.0    |-WORKER_3        8/26 14:26   0+00:00:00 I  0    0.0 simple.sh --fin
 798.0    |-WORKER_4        8/26 14:26   0+00:00:00 I  0    0.0 simple.sh --fin
@marcmengel
Copy link
Contributor Author

Note there look to be a lot of test changes, but it's just tagging the tests as "unit" or "integration" (== slow)

@marcmengel marcmengel requested a review from shreyb August 29, 2022 14:36
Copy link
Collaborator

@shreyb shreyb left a comment

Choose a reason for hiding this comment

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

A few things to summarize overall:

  1. I think there's a mistake in simple.cmd
  2. Aside from that, I had a couple of questions outside of the tests, which are in the various comments
  3. The tests stuff looks good. I've left comments to see what you think, but I like the overall idea of what you've done here, and have only general, "philosophical" questions about unit test vs integration test.

lib/condor.py Outdated Show resolved Hide resolved
templates/simple/simple.cmd Outdated Show resolved Hide resolved
templates/simple/simple.cmd Outdated Show resolved Hide resolved
tests/pytest.ini Show resolved Hide resolved
tests/test_condor_unit.py Show resolved Hide resolved
tests/test_condor_unit.py Show resolved Hide resolved
tests/test_condor_unit.py Show resolved Hide resolved
@marcmengel marcmengel merged commit 6d152fb into master Aug 30, 2022
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.

None yet

2 participants