-
Notifications
You must be signed in to change notification settings - Fork 49
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: add schema for validation #1913
Merged
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
Add jsonschema 2.3.0+ to build dependency table.
Install python-jsonschema packages in the travis Dockerfile.
Ubuntu bionic suppiles 2.6.0, while RHEL 7 EPEL supplies 2.3.0. Both work, so require 2.3.0 or greater.
Add a json-schema (https://json-schema.org/) schema for jobspec, as defined in - Flux RFC 14 json content rules, examples, and text - Valid and invalid sharness test input - (some) peeking at jobspec.cpp Some caveats: - "complex count" cannot be checked for max < min (see json-schema-org/json-schema-spec#51)
Add python script to validate candidate JSON file(s) on command line against a configurable json-schema. Read candidate input(s) with the YAML parser, so jobspec can be supplied in either YAML or JSON form.
Problem: jobspec validator is an installed flux command, but jobspec library is being deprecated. Move jobspec validator to libjobspec and build as an noinst test program. Update sharness test for the new path.
Minor cleanup: - Define JOBSPEC and use it instead of full path - Distinguish invalid inputs from valid with a prefix in the output.
Run the valid and invalid jobspec input through the json-schema validator.
Thanks. I couldn't find anything wrong with this so merging! |
Thanks! |
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 json-schema schema for jobspec and a python script for validation based python-jsonschema. It might form the basis for a future ingest validation plugin, as described in #1908. The schema and test script are exercised by a new sharness test, using the test jobspec in
t/jobspec
.Walking through the explicit checks in libjobspec, validation by this schema is is on a par with the explicit checks there, as they exist now. I did find one check that seemed inexpressible in the schema - the test for
max < min
in the non-scalar count. This limitation of json-schema was discussed at length in json-schema-org/json-schema-spec#51.Other things that seemingly ought to be caught in validation, but aren't currently checked by libjobspec, would be inexpressible in json-schema. For example, as far as I understand, the schema cannot ensure that a task
slot
references a valid resource label.This PR includes a little bit of cleanup of the existing jobspec sharness test, including turning
flux-jobspec-validate
into an uninstalledcheck_PROGRAM
.The new dependency on
python-jsonschema
creates a build problem on our TOSS systems. I will open a jira ticket to ask Trent to pull it in from EPEL in the next release.