libjob: add infrastructure for non-integer counts #6832
Conversation
There was a problem hiding this comment.
@sam-maloney, thanks, this is really well done! So far I had only a couple trivial comments inline. Thanks also for the extensive testing.
I do wonder if counts.[ch] belong in libutil, but for now libjob actually does seem a bit more appropriate and it could always be moved.
mergify
bot
dismissed
grondo’s stale review
Approving reviews have been dismissed because this pull request
was updated.
I definitely wasn't sure at all where best to put this, but since it grew from the parsing of jobspec in the shell init, which uses |
Problem: to expand implementation of RFC 14, several components in core will need to handle non-integer counts, but there is currently no support. Add a new `count` wrapper struct to libjob that will contain one of an integer, RFC 14 range, or RFC 22 idset, with an interface modelled as a subset of the libidset interface. This commit implements the initial decoding and encoding infrastructure.
Problem: new count struct decoding/encoding is untested. Add unit test for count codec.
Problem: jobspec will specify count as JSON while currently only string decoding is implemented. Add function to parse JSON into a count and modify string decoder to check for and dispatch strings which appear to contain JSON.
Problem: new count creation from JSON is untested. Add initial JSON inputs to test cases for count codec.
Problem: cannot iterate through valid values for a count struct. Add count_first() and count_next() functions to iterate through valid values in a count struct, modelled after the interface of libidset.
Problem: new count struct iteration functions are untested. Add unit test for count iteration functions.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6832 +/- ##
==========================================
+ Coverage 83.83% 83.88% +0.04%
==========================================
Files 539 540 +1
Lines 90283 90489 +206
==========================================
+ Hits 75693 75905 +212
+ Misses 14590 14584 -6
🚀 New features to boost your workflow:
|
Problem: non-integer counts from RFC 14 are largely unsupported in flux-core.
This PR adds some initial infrastructure as a first step towards enabling non-integer counts; N.B. at present the new code is not actually used to do anything, this PR only includes the new struct definitions along with creation, encoding/decoding, and iteration functions (as well as initial unit testing).
The
countwrapper structs are intended to present a similar interface (albeit a limited subset thereof) aslibidset, particularly since flux-framework/rfc#442 proposes allowing counts to be given as idsets (which this PR implements). The iteration functions therefore intend to allow code to be agnostic to which type of count was actually given (integer, idset, or range). Any feedback on the design, etc. would be much appreciated!The string representation for decoding/encoding of ranges is documented in the proposed RFC 45 at flux-framework/rfc#453
In conjunction with #6705 this will then allow non-integer ranges to be enabled in more parts of the code, such as job-listing, sched-simple, etc.