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

Describe application/schema-instance+json #405

Merged
merged 3 commits into from
Oct 20, 2017

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Sep 13, 2017

NOTE: This PR is best viewed with whitespace diff off due to indentation level changes

This just adds the general description and the starting point for
the IANA registration. It does not cover linking instances
to schemas in detail, which will be handled in a separate commit.

My goal for this step is just to get the media type information in the spec.
At some point, it will probably be good to consider reorganizing the information
to make it easier to find all necessary information for each media type separately.

Addresses part of #351

Fragment identifiers matching the JSON Pointer syntax, including
the empty string, MUST be interpreted as JSON Pointer fragment
For "application/schema+json", fragment identifiers matching the JSON Pointer
syntax, including the empty string, MUST be interpreted as JSON Pointer fragment
identifiers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood this paragraph as applicable to both "application/schema+json" and "application/schema-instance+json", am I wrong? Otherwise, why adding the For ""application/schema+json", restriction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

application/schema-instance+json only has one fragment identifier syntax, so this sentence is not necessary for it. It's also not incorrect, so you're not wrong. I'll try to tweak the wording.

I suspect that after all of the schema-instance+json stuff goes in, we'll want to rework the overall flow of the document before publishing draft-07, so I don't want to get too hung up on the wording. But I will update this with some improvements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

application/schema-instance+json only has one fragment identifier syntax, so this sentence is not necessary for it. It's also not incorrect, so you're not wrong. I'll try to tweak the wording.

I see, it makes sense now. Your clarification commit helps.

This just adds the general description and the starting point for
the IANA registration.  It does not cover linking instances
to schemas in detail, which will be handled in a separate commit.
Only schema+json supports two syntaxes, so only it needs a rule
for disambiguating them.  schema-instance+json only supports one.
Explain why schema-instance+json's "schema" parameter MAY include
schemas against which the instance may not validate.
@handrews
Copy link
Contributor Author

A question that comes up with respect to the required "schema" media type parameter: how do media type parameters work in media ranges? Can we have:

Accept: application/schema-instance+json

and the server can respond with any "schema" parameter value? Or would the Accept header have to include a valid schema parameter? As far as I can tell it doesn't look like you can use * for parameters? I was thinking of something like:

Accept: application/schema-instance+json; schema=*

@awwright do you know how this is supposed to work? In RFC 7231 media-type and media-range take the same ABNF token for parameters, which is why I'm guessing that ; schema=* wouldn't work in Accept.

@handrews
Copy link
Contributor Author

@awwright are you OK with this part or would you prefer that I drop this PR so you can write a new approach? If you're OK with it, it's been two week so it can be merged as of tomorrow, but I do not want to proceed with this PR without your sign-off. And I'm happy to drop it in favor of one you submit if you'd like.

@awwright
Copy link
Member

awwright commented Sep 26, 2017

If you want to accept any value for a media-type parameter, you just omit that parameter.

@handrews I don't see any major problems with this, but I'm just not entirely convinced we can't amend the application/json media type instead. This is fine to push ahead until we (I) can get a better understanding.

@handrews
Copy link
Contributor Author

@awwright I view this more as an insurance policy / experimentation lab. Amending application/json isn't something we can do in the next few weeks (or likely even the next few months). We can always drop the special media type if we can sort things out with the JSON folks.

But I took a look at the latest JSON spec work and decided that I do not have time to deal with people who refuse to leave the early 90s and collaborate with more useful tools than bare-bones mailing lists without even a decent search interface.

If/when JSON Schema attains working group status, I'll deal with the IETF's painfully anachronistic technology, but Hyper-Schema is taking far too much of my time for me to deal with things that noise.

I also assume that the very explicit disavowal of JSON Pointer being a fragment syntax for application/json has some history behind it. Why would they establish the fragment syntax and not register it for application/json if there wasn't serious opposition?

So I'm plenty happy to hold off on this if you think you can make progress with application/json in the next month or so, but otherwise I have deliverables and need a fully functional media type for instances.

Philosophically, I'm with you. But I can't take the philosophical approach and wait, because this is a job for me.

@handrews
Copy link
Contributor Author

But I would totally drop this media type in a second if you or anyone got application/json to accept the JSON pointer fragment syntax. You're absolutely right that it makes more sense.

@handrews
Copy link
Contributor Author

Well, and the media type parameter. Need that for version negotiation.

@awwright
Copy link
Member

@handrews Cool, just checking. Sounds like you have the same understanding as I do then!

@handrews
Copy link
Contributor Author

I think I'll hold this open until @awwright's had a chance to comment on #414, which is related. I suspect that it might cause this PR to be re-thought.

@handrews
Copy link
Contributor Author

Final review deadline for this alongside #414 is Friday, Oct. 20th. Barring actionable objections, this will be merged then to kick off the final review period.

@handrews handrews merged commit e53bb44 into json-schema-org:master Oct 20, 2017
@handrews handrews deleted the instance branch October 20, 2017 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants