flux-mini: --setattr: place keys in attributes.system by default with and default values to 1
#4483
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 10, 2022
chu11
approved these changes
Aug 10, 2022
doc/man1/flux-mini.rst
Outdated
Comment on lines
370
to
371
| If KEY does not begin with ``system.``, ``user.``, or ``.``, then | ||
| KEY is set in ``attributes.system.`` by default. VAL is optional and |
There was a problem hiding this comment.
perhaps is confusing to say "does not begin with system. ... then later say defaults to attributes.system. Perhaps just say system. is assumed.
There was a problem hiding this comment.
You mean something like
Suggested change
| If KEY does not begin with ``system.``, ``user.``, or ``.``, then | |
| KEY is set in ``attributes.system.`` by default. VAL is optional and | |
| If KEY does not begin with ``system.``, ``user.``, or ``.``, then | |
| `system.` is assumed. VAL is optional and |
Yes, that sounds better.
There was a problem hiding this comment.
Ok, I made that change and forced a push. Thanks!
Problem: The flux mini --setattr and --setopt options are handled differently, which seems unnecessary and could be confusing to users. Add a new function parse_jobspec_keyval() which combines common code for handling these options which take jobspec key=value arguments. As a side effect, --setattr now defaults the value of keys to 1 if no value is specified, and --setopt can now read values from a file if the key is prefixed with "^". Update one test that expects --setattr=key with no value to fail.
Problem: The flux-mini --setattr option is almost always used to set attributes in the system dictionary, and therefore its use can be very repetitive and inefficient. Prepend "system." to the KEY specified in --setattar=KEY=VAL when it doesn't already begin with "system." or "user.". To force a different top-level target value, also allow keys to be prefixed with "." (e.g. --setattr=.foo will set attributes.foo=1). Fix one use of --setattr=foo=val in the testsuite with --setattr=.foo=val. Fixes flux-framework#4481
Problem: The description of --setattr in `flux mini --help` output is out of date. Update the description of the --setattr to match recent changes.
Problem: The description of the flux-mini --setattr option is out of date in the man page. Update the description of --setattr in flux-mini(1).
Problem: The flux-mini --setattr tests do not verify that --setattr=KEY sets the attribute KEY in the system dictionary by default, and that --setattr now defaults the value to 1 when value is not specified. Test these conditions in the existing --setattr tests in t2700-mini-cmd.t. Also, update the test use jq directly to compare values instead of reading the values with jq and comparing with test.
Codecov Report
@@ Coverage Diff @@
## master #4483 +/- ##
==========================================
- Coverage 83.37% 83.36% -0.01%
==========================================
Files 401 401
Lines 67517 67515 -2
==========================================
- Hits 56291 56284 -7
- Misses 11226 11231 +5
|
|
Thanks! Setting MWP. |
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.
This PR attempts to simplify the
flux-mini--setattrinterface by placing keys inattributes.systemby default (unless the key starts withsystem.,user.or.), since almost all use cases for--setattrare targetingattributes.system.anyway.Also, similar to
--setopt, if a value is not provided, have it default to1. This allows, for example,attributes.system.prolog.clear-ssd = 1to be set simply withflux mini batch --setattr=prolog.clear-ssdinstead of the more cumbersomeflux mini batch --setattr=system.prolog.clear-ssd=1.