search

OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
29.11k stars 9.08k forks source link

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

Open ioggstream opened 2 years ago

ioggstream commented 2 years ago

I suggest

Because

cc: @darrelmiller https://github.com/ietf-wg-httpapi/mediatypes/issues/8#issuecomment-1073116351

darrelmiller commented 2 years ago

@webron Can you get an authoritative opinion on this?

LasneF commented 5 months ago

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

handrews commented 5 months ago

@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 commented 5 months ago

i would rather use the RFC as even for the 3.04

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 ,