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

schema/state.json: Add a JSON Schema for the state JSON #481

Merged
merged 4 commits into from
Jun 9, 2016

Conversation

wking
Copy link
Contributor

@wking wking commented Jun 1, 2016

Make it easy for folks to validate state JSON the same way they can
currently validate config.json.

This is spun off from #453, and I've left the config-oriented IDs
alone (with their /schema/bundle/… IDs) to avoid pulling in that
rejected adjustment.

@wking wking force-pushed the state-schema branch 2 times, most recently from ef4c6b0 to d31a3bf Compare June 1, 2016 22:59
@vbatts
Copy link
Member

vbatts commented Jun 2, 2016

I know you've mentioned your opinion on the redundancy of 'schema' in the filename, but I'm holding to that. Even in the image spec, for multiple schemas, they are named config-schema.json, image-manifest-schema.json and manifest-list-schema.json

@wking
Copy link
Contributor Author

wking commented Jun 2, 2016

On Thu, Jun 02, 2016 at 05:18:00AM -0700, Vincent Batts wrote:

I know you've mentioned your opinion on the redundancy of 'schema'
in the filename, but I'm holding to that. Even in the image spec,
for multiple schemas, they are named config-schema.json,
image-manifest-schema.json and manifest-list-schema.json

I don't see the point, but it's fine with me 1. Is that just for
entrypoint JSON, or do you also want config-linux-schema.json,
defs-schema.json, etc.?

@vbatts
Copy link
Member

vbatts commented Jun 3, 2016

The defs* files are not schema particularly themselves. I'm fairly confident that it isn't just my own opinion of logical file naming, that the schema files to validate against, would actually have 'schema' in the name. All other files would be sourced as needed.

@wking
Copy link
Contributor Author

wking commented Jun 3, 2016

On Fri, Jun 03, 2016 at 10:29:45AM -0700, Vincent Batts wrote:

The defs* files are not schema particularly themselves.

The current schema-linux.json is also not a stand-alone schema.

… the schema files to validate against, would actually have 'schema'
in the name.

That sounds like “only use -schema for entrypoint files” 1. I'll
reroll to use config-linux.json, config-solaris.json, config-schema.json,
defs-linux.json, defs.json, and state-schema.json.

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 3, 2016
To make it clear that these schemas are for validating config.json
(and not, for example, state JSON).  I've left the IDs alone for now,
because my PR adjusting those was rejected [1].

The rule for the -schema portion is "use it for entrypoint files" [2].

[1]: opencontainers#453
[2]: opencontainers#481 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@wking
Copy link
Contributor Author

wking commented Jun 3, 2016

On Fri, Jun 03, 2016 at 11:20:05AM -0700, W. Trevor King wrote:

On Fri, Jun 03, 2016 at 10:29:45AM -0700, Vincent Batts wrote:

… the schema files to validate against, would actually have
'schema' in the name.

That sounds like “only use -schema for entrypoint files” [1]. I'll
reroll to use config-linux.json, config-solaris.json,
config-schema.json, defs-linux.json, defs.json, and
state-schema.json.

Rerolled with ef4c6b0b4bcddb, which also rebases onto master
(around #482) and makes a few schema/README.md changes to more generic
language.

@vbatts
Copy link
Member

vbatts commented Jun 6, 2016

LGTM

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Jun 6, 2016

for reference, could provide a pastebin or gist to the state.json you were testing against?

@wking
Copy link
Contributor Author

wking commented Jun 6, 2016

On Mon, Jun 06, 2016 at 07:48:21AM -0700, Vincent Batts wrote:

for reference, could provide a pastebin or gist to the state.json
you were testing against?

I was just using the spec's example 1, although it looks like I need
to reroll to pickup #484 (so don't merge this yet ;).

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 6, 2016
To make it clear that these schemas are for validating config.json
(and not, for example, state JSON).  I've left the IDs alone for now,
because my PR adjusting those was rejected [1].

The rule for the -schema portion is "use it for entrypoint files" [2].

[1]: opencontainers#453
[2]: opencontainers#481 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@wking
Copy link
Contributor Author

wking commented Jun 6, 2016

On Mon, Jun 06, 2016 at 08:49:55AM -0700, W. Trevor King wrote:

… it looks like I need to reroll to pick up #484

Rerolled with b4bcddb8f67a50.

@crosbymichael
Copy link
Member

crosbymichael commented Jun 7, 2016

LGTM

Approved with PullApprove

1 similar comment
@vbatts
Copy link
Member

vbatts commented Jun 9, 2016

LGTM

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Jun 9, 2016

rebase please

wking added 4 commits June 8, 2016 20:43
To make it clear that these schemas are for validating config.json
(and not, for example, state JSON).  I've left the IDs alone for now,
because my PR adjusting those was rejected [1].

The rule for the -schema portion is "use it for entrypoint files" [2].

[1]: opencontainers#453
[2]: opencontainers#481 (comment)

Signed-off-by: W. Trevor King <[email protected]>
So we can use it in the coming state-schema.json without duplication.
While I'm touching it, I updated the spec title to match the project
README's header.  I also dropped the "id" because none of the other
defs.json entries had an ID.

Signed-off-by: W. Trevor King <[email protected]>
So we can use it in the coming state-schema.json without duplication.
I dropped the "id" because none of the other defs.json entries had an
ID.

Signed-off-by: W. Trevor King <[email protected]>
The IDs namespace the fields within the OCI, with /runtime to select
the opencontainers/runtime-spec project, and /state to select the
state JSON within runtime-spec.

Signed-off-by: W. Trevor King <[email protected]>
@wking
Copy link
Contributor Author

wking commented Jun 9, 2016

On Wed, Jun 08, 2016 at 08:16:49PM -0700, Vincent Batts wrote:

rebase please

Rebased around #491 with 8f67a502a5986f.

@vbatts
Copy link
Member

vbatts commented Jun 9, 2016

LGTM

Approved with PullApprove

1 similar comment
@dqminh
Copy link
Contributor

dqminh commented Jun 9, 2016

LGTM

Approved with PullApprove

@dqminh dqminh merged commit 2f0fa18 into opencontainers:master Jun 9, 2016
@wking wking deleted the state-schema branch June 17, 2016 03:14
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jul 19, 2016
This slipped through the rename in 2a5986f (schema/state-schema.json:
Add a JSON Schema for the state JSON, 2016-06-01, opencontainers#481) and the first
round of fixes in dfb85b1 (schema/README: Fix links to
(config|state)-schema.json, 2016-06-13, opencontainers#498).  Reported by hapnermw
[1].

[1]: opencontainers#517

Signed-off-by: W. Trevor King <[email protected]>
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
To make it clear that these schemas are for validating config.json
(and not, for example, state JSON).  I've left the IDs alone for now,
because my PR adjusting those was rejected [1].

The rule for the -schema portion is "use it for entrypoint files" [2].

[1]: opencontainers#453
[2]: opencontainers#481 (comment)

Signed-off-by: W. Trevor King <[email protected]>
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
This slipped through the rename in 2a5986f (schema/state-schema.json:
Add a JSON Schema for the state JSON, 2016-06-01, opencontainers#481) and the first
round of fixes in dfb85b1 (schema/README: Fix links to
(config|state)-schema.json, 2016-06-13, opencontainers#498).  Reported by hapnermw
[1].

[1]: opencontainers#517

Signed-off-by: W. Trevor King <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants