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

YAML JSON Schema is not enough for roundtrip-safe #2951

Open
ioggstream opened this issue Jun 22, 2022 · 10 comments
Open

YAML JSON Schema is not enough for roundtrip-safe #2951

ioggstream opened this issue Jun 22, 2022 · 10 comments
Labels
metadata tags, info, license, contact, markdown usage, etc. review
Milestone

Comments

@ioggstream
Copy link
Contributor

I suggest

Because

  • The "YAML JSON Schema" has some issues with inf/nan that do not map to JSON datatypes.
  • DRY

cc: @darrelmiller ietf-wg-httpapi/mediatypes#8 (comment)

@darrelmiller
Copy link
Member

@webron Can you get an authoritative opinion on this?

@handrews handrews added this to the v3.2.0 milestone May 24, 2024
@handrews handrews added the metadata tags, info, license, contact, markdown usage, etc. label May 24, 2024
@LasneF
Copy link
Member

LasneF commented Jun 20, 2024

@handrews; @darrelmiller
should this be updated to leverage https://www.rfc-editor.org/rfc/rfc9512.html as it is now published

@handrews
Copy link
Member

@LasneF I think it should definitely be updated in 3.2. I hesitate to update it in the patch releases, in part because a.) I'm uncertain of the exact impact, and b.) I doubt anyone actually depends on this guidance for round-trip safety. I don't even know how to enforce it- if I want round-trip safety, I'll test it.

I am open to other opinions on this, although if we want it in 3.0.4 we need to decide that ASAP.

@LasneF
Copy link
Member

LasneF commented Jun 21, 2024

i would rather use the RFC as even for the 3.04

  • our patch is pushing should be leveraging the latest precision of RFC if there is no breaking change but just precision
  • it s better to mention a stable RFC than a draft
  • in our case the content of the paragraph is similar

we need to introduce a sentence about it as currenlty there is no such precision , and it should be an advise only

my opinion would be here to have a consistent update for all the version

that said if we push it only to 3.2 it would be ok as well

@handrews
Copy link
Member

@LasneF

our patch is pushing should be leveraging the latest precision of RFC if there is no breaking change but just precision

We already decided not to update RFCs in patch releases (search issues / PRs for 9110).

it s better to mention a stable RFC than a draft

Yes, and we have made those updates, but the YAML 1.2 spec is not a draft. It just was not a formal media type registration for IANA purposes.

my opinion would be here to have a consistent update for all the version

Yeah I'd prefer that but if we do that we need to update all RFCs and revisit the decision made about RFC9110 replacing RFC7231 (which we decided to deal with in 3.2). Feel free to lobby the TSC for it or pick up this work, but I don't have the time/energy for it. You could bring it up in the Style Guide / release checklist issue as it could arguably be part of the checklist if we want to update.

@ioggstream
Copy link
Contributor Author

Hi folks. Feel free to ping me...

I think it's ok to reference yaml interoperability considerations from the IANA media types registry.

It is ok to remove references to "YAML JSON SCHEMA", since it is misleading.

Pax, R

@handrews
Copy link
Member

@OAI/tsc review request: Please decide if this needs doing in 3.0.4 and 3.1.1, and if so, what about updating all other references? The decision should be recorded in the style guide / release checklist (#3785) for future releases.

@mikekistler
Copy link
Contributor

I'm struggling to understand what change is proposed here. I think the proposal is to replace this paragraph and the two subsequent bullets in the Format section with a reference to a newer document about the YAML media type.

If this is correct, I don't have a strong opinion on this but I don't think the arguments for making this change in patch releases are strong enough to override our general philosophy of minimizing changes in patch releases to avoid the potential for subtle compatibility issues.

@handrews
Copy link
Member

@mikekistler

subtle compatibility issues.

Yeah this is exactly what I'm concerned about.

I dug into the YAML spec some more to try to understand this "inf/nan" issue- it seems to be related to the floating point tag, which is actually problematic in general: neither the JSON nor JSON Schema data models can distinguish floats from integers based on their textual representation. That's why we have all of those numeric format values.

So I'd say there's a bigger problem than just "inf/nan" but I'm also hesitant to change things. And while we should point to that section in the YAML RFC in 3.2, I'm not sure that actually solves everything because of the int vs float issues.

I'm also pretty sure that anyone who tries to use "inf" or "nan" in YAML with OpenAPI is going to get an error very quickly.

@LasneF
Copy link
Member

LasneF commented Jun 27, 2024

reading back the thread , i would roll back the fact to put it in on patch considering the "low value" it provides

we can mention for 3.2 instead

In order to preserve the ability to round-trip between YAML and JSON formats, YAML version 1.2 is RECOMMENDED along with some additional constraints: etc

instead of leveraging the 2 bullets point we can may be just mentionned
https://www.rfc-editor.org/rfc/rfc9512.html#section-3.4 that is may be more precise , and in term of responsability more "their" job that OAS to try to settle the list of feature to do ,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metadata tags, info, license, contact, markdown usage, etc. review
Projects
None yet
Development

No branches or pull requests

5 participants