Skip to content
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

jobspec: make attributes.system.duration a number of seconds #2096

Merged
merged 1 commit into from Mar 22, 2019

Conversation

@grondo
Copy link
Contributor

commented Mar 22, 2019

As mentioned in #2095, make jobspec and minimal jobspec attributes.system.duration into a strict number of seconds, instead of an arbitrary string.

Updates all the tests, parsers, schemas and modules. Unfortunately, this had to be done in one big commit.

This should be merged after #2095.

@grondo grondo force-pushed the grondo:rfc14-duration branch from 5a90f8e to 0e83f7c Mar 22, 2019
@@ -98,7 +98,7 @@
"system": {
"type": "object",
"properties": {
"duration": { "type": "string" }
"duration": { "type": "number" }

This comment has been minimized.

Copy link
@garlick

garlick Mar 22, 2019

Member

The schema could restrict duration to a positive number using

"duration": { "type": "number", "exclusiveMinimum": 0 }

(I assume zero is not allowed?)

This comment has been minimized.

Copy link
@grondo

grondo Mar 22, 2019

Author Contributor

Good point!

Actually we may need a way to communicate "unlimited" -- should 0 be allowed for that case? (wish I would have thought of this when modifying RFC 14)

This comment has been minimized.

Copy link
@grondo

grondo Mar 22, 2019

Author Contributor

Just pushed a patch to set the minimum to 0. I think we may have to allow 0 with special meaning -- maybe clarify this in RFC 14 if you agree?

This comment has been minimized.

Copy link
@garlick

This comment has been minimized.

Copy link
@grondo
Update the jobspec attribute.system.duration to a strict number
of seconds rather than an arbitrary string. Update all use cases,
tests, scripts, and readers of jobspec. Unfortunately this must
be done all at once to avoid mid-commit breakage.
@grondo grondo force-pushed the grondo:rfc14-duration branch from 8a25eb9 to 0969f74 Mar 22, 2019
@grondo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

squashed and force-pushed.

@codecov-io

This comment has been minimized.

Copy link

commented Mar 22, 2019

Codecov Report

Merging #2096 into master will increase coverage by 0.01%.
The diff coverage is 97.87%.

@@            Coverage Diff            @@
##           master   #2096      +/-   ##
=========================================
+ Coverage   80.29%   80.3%   +0.01%     
=========================================
  Files         196     196              
  Lines       31311   31315       +4     
=========================================
+ Hits        25140   25147       +7     
+ Misses       6171    6168       -3
Impacted Files Coverage Δ
src/cmd/builtin/hwloc.c 81.42% <100%> (ø) ⬆️
src/modules/job-exec/job-exec.c 73.43% <100%> (+0.23%) ⬆️
src/cmd/flux-start.c 78.21% <100%> (ø) ⬆️
src/cmd/flux-job.c 85.54% <100%> (ø) ⬆️
src/cmd/flux-aggregate.c 77.77% <100%> (ø) ⬆️
src/cmd/flux-ping.c 86.95% <100%> (ø) ⬆️
src/common/libutil/fsd.c 100% <100%> (ø) ⬆️
src/common/liboptparse/optparse.c 90.72% <88.88%> (ø) ⬆️
src/common/libflux/message.c 81.39% <0%> (-0.13%) ⬇️
src/broker/broker.c 77.92% <0%> (-0.02%) ⬇️
... and 7 more
@garlick garlick merged commit f08d6fe into flux-framework:master Mar 22, 2019
2 checks passed
2 checks passed
Mergify — Summary 1 potential rule
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@grondo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Thanks!

@grondo grondo deleted the grondo:rfc14-duration branch Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.