search

OAI / Overlay-Specification

The OAI Overlay Specification
Apache License 2.0
52 stars 14 forks source link

Clarify the behavior of YAML comments in Overlays #83

Open earth2marsh opened 1 week ago

earth2marsh commented 1 week ago

The Overlay Specification should clarify how YAML comments work. Some things to consider:

  1. That YAML comments may be used within an overlay document
  2. That any comments in any original YAML description document (i.e., the document that an overlay modifies) should be preserved in the merge
  3. ... or alternatively that you may expect that any comments are nuked after applying an overlay.
  4. Other?
earth2marsh commented 1 week ago

Thomas's comment in a slack thread caused me to check the OpenAPI Specification to see what we say about YAML comments there, and it turns out we don't say anything about them. That makes some sense to me, as I wouldn't expect converting YAML > JSON > YAML would preserve a comment.

However, Overlays feels a little bit different in that you could apply YAML to YAML and end up losing your YAML comments. Isn't that a small surprise?

handrews commented 1 week ago

My feelings are that:

  1. Anything that can't be used across both JSON and YAML should be outside of our specification(s)
  2. YAML comments in an overlay are comments about the overlay, not the OAS content (because the OAS does not make any mention of YAML comments at all)
handrews commented 1 week ago

It's worth noting that many YAML packages do not preserve comments, or require great contortions to access them. Here is a thread on comments in PyYAML, the de-facto Python standard for working with YAML (and fastest by a large margin despite the claims of other libraries- I tested a bunch of them).

Therefore, I don't think it's reasonable for any of our specifications to mandate YAML comment-preservation behavior. It's simply not going to be interoperable.

I think this strengthens the argument that, even when using a comment-preserving parser, comments from the overlay should be treated as commenting on the overlay and not as commenting on the target. That makes the behavior between comment-preserving and non-comment-preserving parsers more similar.

I see this as analogous to preservation of line numbers: That's not something any OpenAPI specification (little-s, generic, not just OAS) should mandate. Not only do many parsers not track them, but the ones that do are generally orders of magnitude slower.

handrews commented 1 week ago

Also, while that thread about PyYAML mentions an alternative package, keep in mind that in many environments, incorporating a new package into code is an extremely heavy-weight process requiring both legal and security review. Anything we mandate needs to work with the most common libraries and not require something unusual.