jobspec: make max/operator/operand optional #879
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
milroy
approved these changes
Oct 31, 2021
| throw parse_error (cnode, "Key \"max\" missing from count"); | ||
| } | ||
| if (!cnode["max"].IsScalar()) { | ||
| if (cnode["max"] && !cnode["max"].IsScalar()) { |
There was a problem hiding this comment.
Nits: you may want to fix the "interger" typo in line 48 and "existance" in line 60 above.
There was a problem hiding this comment.
:-). Great catch, human valgrind! They must have been in there from day 1. I will do a fix up commit.
Problem: libjobspec within Fluxion currently requires all four keys for a "moldable" jobspec to be present under the resources.count: min, max, operator, and operand. This does not comply with RFC14, as it specifies: "The default value for max SHALL be infinite, therefore a count which specifies only the min key SHALL be considered a request for at least that number of a resource, and the scheduler SHALL generate the R that contains the maximum number of the resource that is available and subject to the operator and operand. By contrast, if a fixed count is given to the count key, the scheduler SHALL match any resource that contains at least count of the resource, but its R SHALL contain exactly count of the resource (potentially leaving excess resources unutilized). Handle the optional keys (max, operator, operand) appropriately and adjust the relevant test cases.
dongahn
force-pushed
the
moldable-default-fix
branch
from
57c1833 to
c172bff
Compare
Codecov Report
@@ Coverage Diff @@
## master #879 +/- ##
========================================
- Coverage 73.3% 73.3% -0.1%
========================================
Files 76 76
Lines 8384 8381 -3
========================================
- Hits 6151 6147 -4
- Misses 2233 2234 +1
|
dongahn
added
the
merge-when-passing
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 adds a quick fix for Issue #876. This was written on top of PR #878 so don't merge it before PR #878.
Problem: libjobspec within Fluxion currently requires all
four keys for a "moldable" jobspec to be present under
the resources.count: min, max, operator, and operand.
This does not comply with RFC14, as it specifies:
"The default value for max SHALL be infinite, therefore
a count which specifies only the min key SHALL be considered
a request for at least that number of a resource, and
the scheduler SHALL generate the R that contains the maximum
number of the resource that is available and subject to the
operator and operand. By contrast, if a fixed count is given
to the count key, the scheduler SHALL match any resource that
contains at least count of the resource, but its R SHALL
contain exactly count of the resource (potentially leaving
excess resources unutilized).
Handle the optional keys (max, operator, operand) appropriately
and adjust the relevant test cases.
I wanted to do a bit more work but found that there are some issues we need to fix in flux-core before having an end-to-end demonstration. So I created an Issue at flux-framework/flux-core#3928 and decide to have this reviewed and landed into fluxion first.