-
Notifications
You must be signed in to change notification settings - Fork 44
Extra Flux Options #459
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
Merged
Merged
Extra Flux Options #459
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
6e74f8d
Update schema to support new exclusive syntax for separate launcher/a…
jwhite242 4bee81e
Initial prototype of optional launcher/allocation args pass through h…
jwhite242 4ccc470
Properly apply step exclusive key as override to batch/adapter settin…
jwhite242 ea2d3b4
update method type on exclusive sanitizer
jwhite242 cd632b9
Initial documentation for new optional argument handling in batch block
jwhite242 1e824e1
Fix stray refactoring debris
jwhite242 0a79699
Fixtures and tests for verifying flux script writing correctly passes…
jwhite242 dfb0aea
Guard flux script writer to only run when flux is present
jwhite242 4e70971
Add helper for flattening dictionaries into dot syntax strings
jwhite242 d450bbd
Update flux script test and test spec to use new dot string util for …
jwhite242 af62de2
Add test for verifying live jobspecs get correct allocation and launc…
jwhite242 567e51c
Add funcs for working with dotpath dicts/tuples and tests
jwhite242 cebd217
Add tests of non scalar leaf values in dotpath machinery
jwhite242 20d3409
Add value coercion for dotpath utils, recursive update
jwhite242 c923e74
Refactor additional arg handling to deal with python api end points n…
jwhite242 5236603
Replace fail with assertionerror to avoid stopping entire test sessio…
jwhite242 def5b65
Add extra info to failed jobspec tests to show full spec in case thin…
jwhite242 9bcc059
Finish wiring up queue/bank and update test specs/expected outputs
jwhite242 93c05f0
Move INFO lines to allow in step directives, fix test data to reflect…
jwhite242 fb11228
Add docstrings/tests for dict utils
jwhite242 4e50a92
Remove deepcopy comments
jwhite242 d4079bd
Misc tweaks and fixes, and update batch script outputs to reflect nor…
jwhite242 25d7482
Rework exclusive handling for proper layering of steps ontop of batch…
jwhite242 74c4361
Address review feedback to clarify examples
jwhite242 85864b6
Wire up proper filtering and logging of unhandled allocation args in …
jwhite242 69aa1cb
Cleanup comments/misc formatting fixes
jwhite242 5869383
Fix coverage to work properly when not at project root
jwhite242 8673a42
Add emacs tmp/backup files to gitignore
jwhite242 b6dd8f7
Tick dev version
jwhite242 122a97b
Rewrite flux jobspec diff fixture for improved flexibility on filteri…
jwhite242 1742833
Add target dependent coercion/normalization such that more user frien…
jwhite242 a3f2a97
Add jobname and output file templates to batch script target to match…
jwhite242 af938fb
Add job name, output files, and procs to header. Update INFO directi…
jwhite242 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,3 +248,108 @@ def key(self): | |
| :return: A string of the name of a FluxInterface class. | ||
| """ | ||
| ... | ||
|
|
||
| @classmethod | ||
| def addtl_alloc_arg_types(cls): | ||
| """ | ||
| Return set of additional allocation args that this adapter knows how | ||
| to wire up to the jobspec python apis, e.g. 'attributes', | ||
| 'shell_options', ... This is aimed specifically at the repeated types, | ||
| which collect many flags/key=value pairs which go through a specific | ||
| jobspec call. Everything not here gets dumped into a 'misc' group | ||
| for individual handling. | ||
|
|
||
| :return: List of string | ||
|
|
||
| .. note:: | ||
|
|
||
| Should we have an enum for these or something vs random strings? | ||
|
Comment on lines
+264
to
+266
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this idea. Could probably help with safeguarding against unsupported options |
||
| """ | ||
| # default nothing, overriding in implementation | ||
| return [] | ||
|
|
||
| @classmethod | ||
| def render_additional_args(cls, args_dict): | ||
| """ | ||
| Helper to render additional argument sets to flux cli format for | ||
| use in constructing $(LAUNCHER) line and flux batch directives. | ||
| This default implementation yields a single empty string. | ||
|
|
||
| :param args_dict: Dictionary of flux arg keys and name: value pairs | ||
| :yield: formatted strings of cli options/values | ||
|
|
||
| .. note:: | ||
|
|
||
| Promote this to the general/base adapters to handle non-normalizable | ||
| scheduler/machine specific options | ||
| """ | ||
| yield "" | ||
|
|
||
| @classmethod | ||
| def addtl_alloc_arg_type_map(cls, option): | ||
| """ | ||
| Map verbose/brief cli arg option name (o from -o, setopt from --setopt) | ||
| onto known alloc arg types this interface implements | ||
|
|
||
| :param option: option string corresponding to flux cli input | ||
| :return: string, one of known_alloc_arg_types | ||
| """ | ||
| # Default to pass through, override in implementation | ||
| return option | ||
|
|
||
| @classmethod | ||
| def get_addtl_arg_cli_key(cls, arg_type): | ||
| """ | ||
| Return expected cli key associated with each normalized arg type. | ||
| `arg_type` not in known_arg_types are assumed to be the key already | ||
| to facilitate flexible pass through to launcher | ||
|
|
||
| :param arg_type: string noting arg group or cli key | ||
| :returns: cli key used for this arg | ||
|
|
||
| .. note:: | ||
|
|
||
| Can we find a reasonable default prefix (where are things put | ||
| by default in flux, attributes.system?) | ||
| """ | ||
| # Default to pass through, handling known types/mapping in implementation | ||
| return arg_type | ||
|
|
||
| @staticmethod | ||
| def get_cli_arg_prefix_sep(cli_key): | ||
| """ | ||
| Helper for rendering extra options on cli/batch directives. Sets prefix | ||
| and value separator based on length of cli key. Flux has two conventions: | ||
| single letter cli_key has prefix of '-' and separator of ' ' while | ||
| multiletter cli_key has prefix of '--' and separator of '='. Examples | ||
| '-o foo=2' or '--setopt=foo=2' for single letter cli_key (o) and | ||
| multiletter (setopt) forms to set the same option. | ||
|
|
||
| :param cli_key: the key to use on the cli form of an argument | ||
| :type cli_key: str | ||
| :returns: dict containing 'prefix' and 'sep' for use in rendering | ||
| """ | ||
| if len(cli_key) == 1: | ||
| return {"prefix": "-", "sep": " "} | ||
| else: | ||
| return {"prefix": "--", "sep": "="} | ||
|
|
||
| @classmethod | ||
| def normalize_additional_args(cls, args_dict, group_name=None, filter_unknown=False): | ||
| """ | ||
| Helper to normalize additional arguments to known types and an | ||
| unflattened nested dictionary structure. This unflattens any | ||
| dotpath encoded nested dictionary keys. | ||
|
|
||
| :param args_dict: Dictionary of flux arg keys and name: value pairs | ||
| :type args_dict: dict | ||
| :param group_name: Optional name of group/tag to use in log messages | ||
| when filtering_unknown is on | ||
| :type group_name: str | ||
| :param filter_unknown: flag to block pass through of unknown args, e.g. | ||
| for allocation where we can't handle arbitrary | ||
| :type filter_unknown: bool | ||
| :return: dict of packed args with top level keys being the adapter | ||
| version specific addtl_alloc_arg_types | ||
| """ | ||
| return args_dict | ||
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question, "option a" or "optional"? From this context I'm assuming the former in which case we may want to change the naming convention for clarity?