add -B, --bank= option to submission commands
#6195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
garlick
approved these changes
Aug 6, 2024
| .. option:: -B, --bank=NAME | ||
|
|
||
| Set the bank name for this job to ``NAME``. This option is equivalent | ||
| to `--setattr=bank=NAME`, and results in the ``bank`` attribute being |
There was a problem hiding this comment.
Isn't it --setattr=system.bank=NAME?
There was a problem hiding this comment.
No. See flux-submit(1):
If KEY does not begin with system., user., or ., then system. is assumed.
|
Thanks! I've set MWP. |
Problem: Users are forced to use --setattr=bank=BANK to set a bank attribute in their jobspec, but this is an attribute documented in RFC 14 and is commonly required in production system instance usage. Add a `-B,--bank=` to all flux submission cli commands to make use of banks more user friendly.
Problem: The `-B, --bank=` option is not documented. Add documentation of this option to job-param-additional.rst so it appears in the manual of the submission commands.
Problem: The `-B, --bank=` option does not appear in the set of mutable options/args in submit/bulksubmit. Add it to mutable_args.
Problem: No tests ensure that the submission commands support the `-B, --bank=` option. Add a couple tests to t2710-python-cli-submit.t for this purpose.
Problem: The `-B, --bank` option is not supported in bash tab completions. Add this option to the supported bash tab completions for cli submission commands.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6195 +/- ##
==========================================
+ Coverage 83.35% 83.36% +0.01%
==========================================
Files 521 521
Lines 84942 84945 +3
==========================================
+ Hits 70807 70818 +11
+ Misses 14135 14127 -8
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that
bankandprojecthave been proposed as official members of canonical jobspec, and since they are supported by job-list, it seems like there are excellent arguments to at least support-B, --bank=options in our cli as suggested most recently by @ilumsden.This PR starts with @ilumsden's draft PR #6157 (thanks Ian!), reworks the commit message to project standards, then adds the docs, testing, and new option to tab completions.
I didn't add
--projecthere, even though it is also proposed as canonical in the rfc PR above. I like the idea of-Sproject=NAMEfor rarer cases such as this, though I'm not opposed to--projectif people think that's a good idea.For now, this seems like a good and simple ui improvement.