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: allow non-string attributes #2081

Merged
merged 4 commits into from Mar 18, 2019

Conversation

@garlick
Copy link
Member

commented Mar 18, 2019

This relaxes the requirement that jobspec user and system attributes must be of type string. They can be anything with this change.

Adds a couple of valid yaml inputs that demonstrate this.

Also fixes a bug in the ingest sharness test that would mask ingest failures of valid jobspec.

Finally, makes the same change to minimal-schema.json in anticipation of adding the ability to set arbitrary attributes to flux jobspec.

A corresponding change (or is it a clarification?) to RFC 14 is proposed in flux-framework/rfc#172

@grondo

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

Thanks, this will be immediately helpful!

@grondo

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

This PR seems to need a rebase on the latest master.

garlick added 4 commits Mar 18, 2019
Jobspec should allow system and user attributes of
type other than string.  Relax this check in the
ingest jobspec json-schema.
Add two valid yaml jobspec test inputs with various
types for system and user attributes.
Problem: job-ingest test of valid jobspec inputs would
always succeed because the result was not tested properly.

Fix the test.
Relax the requirement that user and system attributes have
only a string type in minimal-schema.json, used by
  t0020-emit-jobspec.t
  t0021-flux-jobspec.t
@garlick garlick force-pushed the garlick:relax_jobspec_attrs branch from 8882bc8 to 23565b3 Mar 18, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Mar 18, 2019

Codecov Report

Merging #2081 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2081      +/-   ##
==========================================
+ Coverage   80.42%   80.43%   +<.01%     
==========================================
  Files         193      193              
  Lines       30540    30540              
==========================================
+ Hits        24562    24565       +3     
+ Misses       5978     5975       -3
Impacted Files Coverage Δ
src/cmd/flux-module.c 83.96% <0%> (+0.23%) ⬆️
src/common/libflux/message.c 81.39% <0%> (+0.24%) ⬆️
@grondo

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

Thanks!

@grondo grondo merged commit 0afdf79 into flux-framework:master Mar 18, 2019
4 checks passed
4 checks passed
Mergify — Summary 1 potential rule
Details
codecov/patch Coverage not affected when comparing aedccca...23565b3
Details
codecov/project 80.43% (+<.01%) compared to aedccca
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garlick garlick deleted the garlick:relax_jobspec_attrs branch Mar 19, 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.