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

"deprecation" property #74

Open
awwright opened this Issue Oct 8, 2016 · 80 comments

Comments

Projects
None yet
@awwright
Copy link
Member

awwright commented Oct 8, 2016

Create a schema keyword that specifies an instance (property in an instance, usually) as deprecated -- it shouldn't be used, but exists only for internal use, or for reverse compatibility with obsolete or old products.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Oct 9, 2016

+1 to this. The point here, I assume, is for documentation and perhaps to allow implementations to issue warnings, rather than to affect validation.

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Nov 2, 2016

Interesting proposal.

I know that JSON Schema isn't specifically for API's, but we should consider how API's might version their endpoints. Could this proposal actually be part of a seperate doc about API driven information. For example, included in deprecated, how it's deprecated and if it's replaced, and possibly other upgrade path type data.
Very much a straw man comment, not well thought through.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Nov 3, 2016

@Relequestual I'm not sure how far we want to chase the versioning idea here, but my approach to API evolution (rather than versioning) is to only change the version of the schema, and retain compatibility by (as much as possible) avoiding both required and "additionalProperties": false. I use the following rules:

  • Resources add new schema versions to their list of profiles, and do not remove old versions
  • If an instance validates against a schema that is a valid profile for the resource, no matter what version that schema is, than the instance can be used according to that version of the schema.
    • So if your code was written for a specific version, as long as that still validates, you can keep using that without worrying about the new version.
    • This can allow for continuing to function in a scenario where some new instances may not validate against the old schema, but if the instances you work with still validate, you don't have to update to support the things you don't need. For instance, a value was added to an enum, but you never use the new value.
  • If you can't keep compatibility at all, make a new resource and handle the change through HATEOAS, use of 410 GONE, redirects, etc.
  • There are no versions in the URI or primary media type (the schema versions likely show up in their profile URIs, since those need to be distinct somehow)
@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Nov 4, 2016

Interesting. Most of the time our API (and others I've worked on) have needed "new versions" because of new fields or new allowed values. Sometimes that's backwards compatible, but often it isn't for me, as much as I'd like it to be.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Nov 4, 2016

As long as you avoid setting additionalProperties to false, then new fields should be fine (unless you're generating strictly typed code off of the schemas without any provision for unrecognized fields, but I avoid that in favor of more generic client libraries).

Enumerations get tricky, but that's where the "if it still validates you can still use it" rule comes into play- technically the new version is not fully compatible but most instances will validate against either version.

My approach to API evolution is to do as much as possible to allow users to keep using the API as they have been unless their specific usage is broken by the update. It requires some design discipline, and the willingness to add a new resource as a replacement and deprecate the old instead of doing a major version change on the entire API and breaking a bunch of clients that don't actually care one way or the other.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Nov 4, 2016

Admittedly, I haven't actually had a chance to prove that this can work long-term, but it's something I'm trying with the current project I'm working on.

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Nov 22, 2016

I think Protocol Buffers allows for flagging deprecation. If someone has time, may be worth seeing what that system allows for.

@awwright

This comment has been minimized.

Copy link
Member

awwright commented Nov 30, 2016

#173 is a sufficient solution for me, so I'll close this out in favor of the PR

@awwright awwright closed this Nov 30, 2016

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Nov 30, 2016

@awwright please don't close issues while the corresponding PR is still open. PRs are not always accepted.

@awwright

This comment has been minimized.

Copy link
Member

awwright commented Nov 30, 2016

@handrews A PR is just a type of issue on GitHub, and I can indefinitely re-edit it if necessary, or re-open this issue of mine, or someone can file a new issue for me if it really comes down to that

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Dec 1, 2016

@awwright This is yet another instance of you randomly choosing how to use GitHub in whatever ways that suit you, no matter whether anyone else finds it confusing. Some issues you want to keep open, some you don't, we just get to guess at what you'll complain about, and if we want to keep track of things in some consistent way, we're out of luck if you decide not to.

All of the discussion about wanting a clear and defined process that the community has had clearly means nothing to you. Your response to any complaint or request for clarity is always just "well I see it this way so that's what I'm doing".

@awwright

This comment has been minimized.

Copy link
Member

awwright commented Dec 1, 2016

@handrews It's my issue! Anyone is allowed to close their own issue for any reason. There's nothing special about it.

@Relequestual Relequestual reopened this Dec 15, 2016

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Dec 15, 2016

I'll re-open this issue as I agree issues that do not have accepted pull requests should not be closed.

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Dec 15, 2016

It seems like there's quite a lot of discussion and decisions to be made on this issue. Specifcially the comments in the PR review: #173 (comment)

It's clear there are a number of unresolved abiguities.

In a root schema with a "self" link is means that the resource itself is deprecated? But it's still there currently?

In a root schema without a "self" link it means....?

In a property subschema (in any of the property keywords) it means that all matching properties are deprecated but will still be present?

In a schema applying to a single position in an array/tuple ("items" is an array) it means...?

In a schema applying to multiple array positions ("items" as one schema, or "additionalItems") it means...?

In the schema for "schema" or "targetSchema" it means...?

This concept is not ready for a PR. There are far too many cases that need discussing and a coherent view of hypermedia resources that need to be developed before we can just toss this in.

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Dec 15, 2016

I've set this as priority: low. People aren't crying out for it, and it doesn't fix an existing bug. It would be nice to go in the next release if the above issues can be resolved, but otherwise it shouldn't become a blocker.

@discordier

This comment has been minimized.

Copy link

discordier commented Dec 20, 2016

I just ran into the same wall. I want to denote in a schema that a property will get removed in a later version.
So currently I have a schema with id: https://example.org/schemas/1.0/payload-schema.json.
Now we are releasing the next version adding new properties and types named https://example.org/schemas/1.1/payload-schema.json but also want to deprecate old stuff which got superseeded by the new types to be ultimately get removed in https://example.org/schemas/2.0/payload-schema.json (long term future).

Simply as all document types evolve over time you need a migration phase (1.x schema series in above example).
So we want all 1.x documents to validate against all 1.x schemas. This is possible, as pointed out by @handrews above when "avoiding both required and "additionalProperties": false, all types are preserved and only non mandatory new stuff gets added (See also SemVer).
Now we also want validation to break when the deprecated stuff is validated against the 2.x schema.

All this is currently possible from validation POV but we lack a way to tell the user of the schema (developer) about the migration phase. Of course this can be handled by defining proprietary keys, used by whatever validator one is using or attaching a new validator to validate twice, but this will cause fragmentation of standards.
Therefore defining this right in the spec would be better - or as @Relequestual stated "nice to have".

Yet, I wonder how big need is as next to no one seems to feel the need (with me being the second guy in this thread apparently).
The fact that XML XSDs also lack a standardized way to define something like this makes me wonder if the schema really is the right place to add this but I fail to see a better alternative (which can be utilized by CI and yet not break production).

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Dec 21, 2016

@discordier we've generally had trouble finding anyone who has even used hyper-schema enough to participate in discussions. So don't take the low count as necessarily indicating lack of support. I encourage you to comment on any hyper-schema issues and pull requests you see- it is a challenge for us to decide on directions when only two or three people have opinions they are comfortable expressing. Here are the issues and PRs (including but not limited to hyper-schema) that we're targeting for the next draft: https://github.com/json-schema-org/json-schema-spec/milestone/2

@Relequestual marked this low priority because we're trying to quickly resolve enough things to publish Draft 6 and this is both not essential for that immediate effort and because there are several questions without obvious answers. I'd personally be in favor of getting this into Draft 7, though, if we can't sort it out in time for Draft 6.

I'm only familiar with XSD in the most basic sense, but I gather it's more analogous to JSON Schema Validation rather than JSON Hyper-Schema. I definitely think this feature belongs in hyper-schema assuming we can sort out the details.

@awwright awwright removed this from the draft-next (draft-6) milestone Jan 23, 2017

@lidio601

This comment has been minimized.

Copy link

lidio601 commented Apr 7, 2017

+1

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Apr 7, 2017

@lidio601 Please avoid +1 comments. You can thumbs up any comment you agree with. Thanks.

@brendajin

This comment has been minimized.

Copy link

brendajin commented Apr 13, 2017

I would also like a deprecation system, or some type of meta-tagging for fields that are being added, replaced, and deprecated. As others mentioned before, it should have a date to accompany it.

@handrews handrews added the hypermedia label May 16, 2017

@handrews handrews added annotation and removed Priority: Low labels Sep 14, 2017

@philsturgeon

This comment has been minimized.

Copy link
Collaborator

philsturgeon commented Sep 19, 2017

So long as we're 100% focusing on field deprecation in this issue and not hypermedia stuff (that'll be another issue/PR), I'm confident we can get this thing back on track.

It seems to me now, despite my earlier interest in "deprecated" being a keyword in the property's schema, that approach is a non-starter. The examples from @handrews make that clear to me.

GraphQL Types can get away with that simple annotation next to the field declaration as they don't have any of the if/then/else stuff JSON Schema does.

Is there any agreement on these statements:

  1. The deprecated keyword will need to work very much like required

  2. Trying to get messages involved is overkill, as the field is going away and the why doesn't really matter

  3. Trying to get versions in when JSON Schema has no opinion on versions is going to be a mess

  4. Dates are necessary to avoid panic attacks that a field might go away tomorrow, or to avoid complacency and the warning being ignores. A date provides the right level of urgency for the resolution, even though the date might not be 100% abided to on the server

@Relequestual @handrews @dlax

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 19, 2017

even though the date might not be 100% abided to on the server

I think this bit from the Sunset header proposal may be relevant

Even though the Sunset header information is made available by the
resource itself, there is no guarantee that the resource indeed will
become unavailable, and if so, how the response will look like for
requests made after that timestamp.

I'd also say there's no guarantee that the field (in our case) won't disappear sooner either, although that would certainly annoy clients/customers and be, um... ill-advised.

@dret

This comment has been minimized.

Copy link
Collaborator

dret commented Sep 19, 2017

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 19, 2017

@dret agreed. In the rewrite I'm doing for #377 (just reworking for clarity, no implementation-impacting changes), I plan to put a section about this in the Target Hints section. Part of that reorganization is to facilitate making this patterns clear rather than just tacking them onto certain individual fields.

@philsturgeon

This comment has been minimized.

Copy link
Collaborator

philsturgeon commented Sep 19, 2017

@dret just so everyone is all clear, this issue is not about link hints, only field deprecations. Let's make sure from now on we're all only talking about field deprecation. :)

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 19, 2017

@philsturgeon @dret sorry that I was my fault for bringing in the Sunset header. These things are very separate but analogous in my head so I tend to reference both without thinking that it might be confusing.

@Relequestual

This comment has been minimized.

Copy link
Member

Relequestual commented Sep 20, 2017

In agreement with @philsturgeon on all counts for this one.
We should make sure the date is in an unambigious format!

@adamvoss

This comment has been minimized.

Copy link
Contributor

adamvoss commented Sep 21, 2017

Trying to get messages involved is overkill, as the field is going away and the why doesn't really matter

It is important to know why in order to know how to update a document to remove the deprecated member.

I think this issue may be running into of intention stemming from different usages of JSON Schema. Many of the vocal users here seem to be ones who just JSON Schema in APIs and I have even seen comments (not in this item) that JSON is always used as part of a larger system. This is not true in my usage where I will use manually maintained JSON in-lieu of a database or spreadsheet for storing information (often to be displayed by Jekyll but always) and I will have a schema for it. Similarly, many tools/applications have user-edited JSON configuration.

Regarding deprecation specifically, Microsoft even added a custom extension for JSON Schema to capture a deprecation message. See: https://github.com/Microsoft/vscode-json-languageservice/blob/a89e995877162b698e338e8301403a4ea49d5d92/src/test/parser.test.ts#L1155-L1171

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

@adamvoss this is going in the validation spec rather than the hyper-schema spec specifically because it applies to more than just APIs/hypermedia.

@adamvoss

This comment has been minimized.

Copy link
Contributor

adamvoss commented Sep 21, 2017

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

I think this issue may be running into of intention stemming from different usages of JSON Schema. Many of the vocal users here seem to be ones who just JSON Schema in APIs and I have even seen comments (not in this item) that JSON is always used as part of a larger system.

I was replying to this part.

I don't have an opinion on the messages at the moment.

@philsturgeon

This comment has been minimized.

Copy link
Collaborator

philsturgeon commented Sep 21, 2017

@adamvoss fair enough on this:

It is important to know why in order to know how to update a document to remove the deprecated member.

That's a valid point. Would you be ok with deprecationHref or deprecationInfo or something, which would contain a URL to a place where there is more information? It could be a JSON Schema reference which lets folks know they can replace it, or a link to human readable documentation, or a blog post, etc.

This is how sunset is avoiding the need for messages, and maybe we could do the same.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

@philsturgeon @adamvoss if people want to include URIs they should just do so using Hyper-Schema keywords (even if the system is not otherwise hypermedia)

{
    "deprecated": ["foo"],
    "properties": {"foo": true, "bar": true},
    "links": [{"rel": "deprecation", "href": "https://example.com/deprecations/foo"}]
}

or something like that. We already have a generic syntax for adding links so I'd rather not just toss in more keywords for specific links.

@philsturgeon

This comment has been minimized.

Copy link
Collaborator

philsturgeon commented Sep 21, 2017

LGTM. So we dont need message or link. Keeping it easy.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

Also, more reasons to use and implement Hyper-Schema :-)
For this use case, you wouldn't even need a full implementation, but it would hopefully encourage people to think of what else you could do with hyper-schema.

With this approach, a schema author could choose to link to a single resource that explains all deprecations for the object, or individual resources for each deprecated field, whichever makes more sense for the resource's users.

@dret

This comment has been minimized.

Copy link
Collaborator

dret commented Sep 21, 2017

@adamvoss

This comment has been minimized.

Copy link
Contributor

adamvoss commented Sep 21, 2017

It is important to know why in order to know how to update a document to remove the deprecated > member.

That's a valid point. Would you be ok with deprecationHref or deprecationInfo or something, which would contain a URL to a place where there is more information? It could be a JSON Schema reference which lets folks know they can replace it, or a link to human readable documentation, or a blog post, etc.

I don't think this would allow for a very good editor experience and it creates an additional burden on a schema creator to have a place to host such information. Most of the schemas at http://schemastore.org/json/ are for things that would be manually edited by a human rather than tooling and many of the schema there are not hosted by the projects that describe them (where requiring a url/hosting would either more coordination from the target project or create a problem for external schema authors).

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

@adamvoss it's possible for the link to be an internal one that simply points to a different place within the schema (since any implementation may add extension keywords, and contentMediaType would allow storing, for instance, HTML). That allows for a great deal of flexibility in proving arbitrary additional deprecation information without needing to include every possibility directly in the spec.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 21, 2017

I'm not dead-set on the link approach, but I think it is a good extension mechanism, and we will no doubt continue to find areas where some bit of data is important for a specific use case, but not broadly important enough to be worth complicating the primary specification.

It would be good to have an approach for handling these long-tail features.

@adamvoss

This comment has been minimized.

Copy link
Contributor

adamvoss commented Sep 21, 2017

The process you specified would only be useful to an editor if it were standardized. Otherwise the link leads allows for essentially infinite variability which cannot be codified into an algorithm. Even then, it sounds like a complicated and not very discoverable process.

This post is too long for me to just mention him directly, but if you have a concise specific wonder about the usage, it could be valuable to mentions aeschli who might be able to comment on MS's decision to add a custom property. Here is the issue that implemented that feature: Microsoft/vscode#14260

Edit: also possibly worth linking, discussion on VSCode's use of JSON Schema.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 22, 2017

@adamvoss if we're making up the link relation we can make up requirements for its target.

But I'm going to step back and let you and @philsturgeon sort this out while I focus on the getting hyper-schema over the finish line.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 25, 2017

Earlier, in response to @levbishop, I said:

In terms of implementation, how can I tell if a property is deprecated? Looking in the immediate schema for properties is not sufficient. I need to look down through any *Of keywords that are present. Your oneOf example shows this, but consider a very complex schema with multiple levels of *Of keywords. It gets more and more difficult. This, on its own, is not a fatal problem: figuring out if a link applies to an instance has the same problem, and there is a section in Hyper-Schema addressing the more complex cases. But it's a factor.

Having thought about this in several contexts a lot more, I no longer think it's a factor, as there is inevitably a need for this sort of search to support annotation keywords in general. See #423 for my full thoughts on the matter. I'm not sure any of it solves the disagreements here, but for people who like consistent underlying theoretical frameworks, well... it's a thing :-P

@philsturgeon

This comment has been minimized.

Copy link
Collaborator

philsturgeon commented Sep 27, 2017

Because some men want to watch the world burn, here's how PayPal handle this with their own vendor prefixed deprecated OpenAPI extensions: https://github.com/paypal/api-standards/blob/master/api-style-guide.md#annotation-x-deprecated

I'd like anyone interested in this deprecated functionality to have a bit of a think about these keywords, and see if they think that the current suggestions fit.

We don't need to copy PayPal, and they're a company very focused on "API Version" as a planned and published thing, many are not that. They also get into the nitty gritty of deprecating enum values, and I'm not sure I want to live in that world - even though I've 100% had to do that in the past.

Let's all have a little think on these usages. If others could post with thoughts that'd be cool.

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Sep 27, 2017

Thanks, @philsturgeon, I'll take a look at that.

Also, I invite you, @adamvoss, @levbishop and anyone else interested to take a look at PR #424 which lays out a way of approaching annotations and extended vocabularies, which will include whatever we end up doing for field deprecation. It might help us all think about how this should work a little more thoroughly. This is the PR resulting from issue #423 that I mentioned above. It's probably actually easier to just read the PR and ignore the issue :-P

@handrews

This comment has been minimized.

Copy link
Member

handrews commented Dec 29, 2017

We've done a lot of work defining what annotations are and how they get collected and used during the later parts of draft-07 and into the main work of draft-08. Which has also made some underlying JSON Schema principles more clear to me. With that in mind, I would now support adopting the single boolean form for a number of reasons:

  1. See #364 for why I'm dropping some other proposals to use arrays instead of booleans.
  2. OpenAPI has a substantial user base setting a precedent. I have no intention of adopting everything OpenAPI adopts (e.g. nullable and discriminator do not fit in JSON Schema for a number of reasons), but this was an option under serious consideration as a proposal anyway, and the one @awwright favored as the person who filed the issue. And it seems nice to keep compatibility if we can.
  3. JSON Schema in general favors annotations that can be interpreted broadly, and allows applications to narrow that interpretation. Having a simple boolean that can be backed up by application-specific behavior or extensions seems to fit that philosophy.
  4. JSON Schema in general prefers to allow harmless nonsense (and if applications come up with a use, great!) rather than trying to prevent keyword combinations that seem useless. This is relevant further down.
  5. If we really need other data (dates, links, etc.) we can add those as more keywords later.
  6. I have answers for the previously unanswered questions:

In a root schema with a "self" link is means that the resource itself is deprecated? But it's still there currently?
In a root schema without a "self" link it means....?
In the schema for "schema" "submissionSchema" or "targetSchema" it means...?

In all cases it means that the entire representation is somehow deprecated. Applications / APIs can provide a more specific meaning, but I would read it to mean "everything about this document is going away". That may or may not mean the resource itself goes away- we have the Sunset header for that anyway.

We are now much more clear about target and context representations and how Hyper-Schema works in general. If you write a "self" link with a target schema (in targetSchema) that is incompatible with or out of sync with your context schema (the one containing the "self" link), then that's your problem.

In general, if there's no sensible interpretation, it can and should be ignored (JSON Schema allows harmless weird things).

In a property subschema (in any of the property keywords) it means that all matching properties are deprecated but will still be present?

Um... yeah... I'm not sure why I thought this was questionable.

In a schema applying to a single position in an array/tuple ("items" is an array) it means...?
In a schema applying to multiple array positions ("items" as one schema, or "additionalItems") it means...?

It's application-specific. I would guess it means something about the array elements (or certain positions of it) being deprecated without the concept of an array being deprecated. What that means I'm not sure, but if anyone actually wants to use it presumably they'll provide some guidance on what they mean. This definitely falls under the "allow harmless apparent nonsense that someone might find a good unanticipated use for anyway" approach.

There are far too many cases that need discussing and a coherent view of hypermedia resources that need to be developed before we can just toss this in.

...and we have now spent a great deal of effort developing that view. I'd say it's safe to just toss it in now :-)

Objections? @awwright ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment