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

Jobspec emission script #1920

Merged
merged 3 commits into from Jan 17, 2019

Conversation

@SteVwonder
Copy link
Member

commented Jan 14, 2019

Adds a python script that emits jobspec json based on the common resource mananger CLI args: -N (number of nodes), -n (number of tasks), -c (cores per task), and a command.

Tests are added that validate the jobspec output against the full jobspec schema and the minimal schema that we whiteboarded. As part of these tests, I tweaked the validate.py script to support reading from stdin (as well as a few other hiccups I ran into).

I have not added tests for the minimal json schema. As low hanging fruit, it would be good to feed some of the more complex jobspecs through and ensure they fail validation. Potentially more useful would be to hand construct some simple jobspecs that still fail.

@garlick, @grondo: any other functionality that we want from emit-jobspec.py at this point? Potentially having it connect into a running instance and having it submit to the job-ingest?

@grondo

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2019

This is a useful and illustrative tool @SteVwonder! Thanks!

This reminded me of a question I had about python's argparse module. Is there any way to force it not to reorder option arguments and treat all arguments after the first non-option argument as non-options (.e.g GNU getopt's POSIXLY_CORRECT mode)? This is important for front-end utilities that take their own arguments plus a user command with its arguments which may use the same characters or strings as the front end tool.

E.g. for emit-jobspec.py:

$ ./emit-jobspec.py -N 4 -n 4 -c 4 myapp -p

argparse eats the -p and it doesn't get added to the myapp args in tasks section.

This isn't important for a test program, because -- can be used to force the parser to stop reordering arguments. However, it would probably cause confusion in a real front end job launcher...

@SteVwonder

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2019

argparse eats the -p and it doesn't get added to the myapp args in tasks section.

Ouch! Great catch! I didn't realize argparse did that. Luckily, there is an alternative to nargs="+" (i.e., nargs=argparse.REMAINDER). I just pushed a commit with that fix.

EDIT: black formatting fixes coming shortly

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Potentially having it connect into a running instance and having it submit to the job-ingest?

There is always:

./emit-jobspec.py -N 4 -n 4 -c 4 myapp -p | flux job submit -
@codecov-io

This comment has been minimized.

Copy link

commented Jan 15, 2019

Codecov Report

Merging #1920 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1920      +/-   ##
==========================================
+ Coverage   80.08%   80.09%   +<.01%     
==========================================
  Files         195      195              
  Lines       35080    35109      +29     
==========================================
+ Hits        28094    28120      +26     
- Misses       6986     6989       +3
Impacted Files Coverage Δ
src/modules/job-ingest/job-ingest.c 70.07% <0%> (-1.19%) ⬇️
src/cmd/flux-module.c 83.72% <0%> (-0.24%) ⬇️
src/common/libflux/message.c 81.51% <0%> (+0.24%) ⬆️
src/cmd/flux-job.c 90.41% <0%> (+0.84%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

This is pretty neat! Maybe squash the incremental development and should be fine to go in.

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

Since @SteVwonder is probably travelling, I'll go ahead and squash this and get it merged (unless he speaks up!)

@SteVwonder

This comment has been minimized.

Copy link
Member Author

commented Jan 17, 2019

Sorry for the radio-silenece, @garlick. I have some time now. I'll rebase and squash.

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

NP! Let's get this in before #1922 and then I'll rebase that one more time.

@SteVwonder SteVwonder force-pushed the SteVwonder:jobspec-emission branch from d3f6292 to 43175e5 Jan 17, 2019
SteVwonder added 3 commits Jan 14, 2019
split `validate_input` into `validate_input` and `validate_inputfile` to
support validating both files on the filesystem and file-like
objects (i.e., stdin)

Two minor refactors include:

Use `.format` to ensure that the string representations of the objects
are formatted and printed properly. Previously was erroring out with:
`TypeError: cannot concatenate 'str' and 'exceptions.ValueError'
objects`.

ensure files are closed by using a context manager
the script emits jobspec based on the common resource mananger CLI args:
`-N` (number of nodes), `-n` (number of tasks), `-c` (cores per task),
and a command
@SteVwonder SteVwonder force-pushed the SteVwonder:jobspec-emission branch from 43175e5 to f74e995 Jan 17, 2019
@garlick

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

Thanks!

@garlick garlick merged commit 111ce4b into flux-framework:master Jan 17, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 6, 2019
This should have been apart of flux-framework#1920. Whoops!
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 10, 2019
This should have been apart of flux-framework#1920. Whoops!
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 11, 2019
This should have been apart of flux-framework#1920. Whoops!
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 11, 2019
This should have been apart of flux-framework#1920. Whoops!
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 13, 2019
This should have been apart of flux-framework#1920. Whoops!
SteVwonder added a commit to SteVwonder/flux-core that referenced this pull request Feb 15, 2019
This should have been apart of flux-framework#1920. Whoops!
@SteVwonder SteVwonder deleted the SteVwonder:jobspec-emission branch Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.