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

Split discriminator out of default OAS 3.1 dialect? #2436

Closed
MikeRalphson opened this issue Jan 7, 2021 · 3 comments
Closed

Split discriminator out of default OAS 3.1 dialect? #2436

MikeRalphson opened this issue Jan 7, 2021 · 3 comments
Assignees

Comments

@MikeRalphson
Copy link
Member

Split off from PR #2399

@handrews said (#2399 (comment)):

Something that just occurred to me due to the ongoing discussion about discriminator, particularly if the selectBy idea is adopted: It might be better to put discriminator in its own vocabulary since it is complex to implement and (particularly if selectBy were added) not strictly necessary.

Putting it in its own vocabulary allows implementations to choose whether to implement it separately from the other OAS extension keywords, and could allow for dialect configurations that require the other OAS extension keywords but leave discriminator as optional. idk if that's really a great idea or not but thought I would mention it. We moved unevaluatedProperties and unevaluatedItems into their own vocabulary for similar reasons- they are complex and potentially expensive so some use cases might prefer not to deal with them (streaming implementations, for example, are much more costly with these).

@OAI/tsc decided this warranted further discussion but didn't want it to hold up merging the above PR.

@MikeRalphson MikeRalphson self-assigned this Jan 7, 2021
@sm-Fifteen
Copy link

Given it was already established in issue #2143 that discriminator...

  • is not strictly required for validation (though it can be a huge performance boost in large unions)
  • is not supported by most OpenAPI doc generators (to my knowledge, only ReDoc supports it; SwaggerUI, Rapidoc and StopLight don't)
  • has all the implementation issues raised by handrews
  • does things that don't really align with the rest of the specification with its use of refs in its mapping, and also how said mapping is always partly implicit

...it may be a wise thing to do, if only so the base specification matches what is actually supported in the wild.

Though I say that not being all that aware of the implications of splitting some part of the specification into its own JSON schema vocabulary.

@handrews
Copy link
Member

At this point, it's clear that most users aren't making deliberate use of JSON Schema vocabularies to exercise fine-grained control over keywords. I wish it were otherwise, but I'd suggest we close this as I think it would add complexity without much benefit.

@lornajane
Copy link
Contributor

I'll second that suggestion and close. And we can re-open if it turns out we do want to revisit.

@lornajane lornajane closed this as not planned Won't fix, can't repro, duplicate, stale Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants