Skip to content

Openapi: adding more info in x-state, adding x-state info to fields and params #4431

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

l-trotta
Copy link
Contributor

@l-trotta l-trotta commented May 30, 2025

Closes #4386, Closes #4356.
Couple of doubts:

  • This would replace "x-beta" with "x-state" in the Beta case, and change the field type from a boolean to a string, is this okay?
  • Does this also have to be configurable, or can it be the standard output?

@l-trotta l-trotta requested a review from a team as a code owner May 30, 2025 14:01
@l-trotta l-trotta requested review from lcawl and removed request for a team May 30, 2025 14:01
@l-trotta l-trotta marked this pull request as draft May 30, 2025 14:01
@l-trotta l-trotta added the skip-backport This pull request should not be backported label May 30, 2025
@lcawl
Copy link
Contributor

lcawl commented May 30, 2025

  • This would replace "x-beta" with "x-state" in the Beta case, and change the field type from a boolean to a string, is this okay?

There is info about the differences between these extensions in https://docs.bump.sh/help/specification-support/doc-badges/. The benefit of using x-beta was that it affected what was deemed a "breaking change" in the automated changelog. However, given that we didn't have the same consideration for tech preview / experimental states, IMO it's not worth trying to have both the x-beta and x-state applied. Rather, I think the latter is the richer and more consistent option.

  • Does this also have to be configurable, or can it be the standard output?

Since this PR affects only a specification extension, in my opinion it should be something that's ignorable by any other tools or processes that consume these OpenAPI documents. That is to say, IMO it can be standard output.

@lcawl
Copy link
Contributor

lcawl commented May 30, 2025

I did a quick skim of the output and the elasticsearch-openapi.json changes LGTM. The elasticsearch-serverless-openai.json, however, seem to inherit some of the non-serverless details. If this ought to be tracked in a separate issue, lmk, but here's what's in the specification, for example:

 * @rest_spec_name async_search.delete
 * @availability stack since=7.7.0 stability=stable
 * @availability serverless stability=stable visibility=public

And here's what I see in this PR's serverless-specific output:

 "summary": "Delete an async search",
 ...
 WAS:       "x-state": "Added in 7.7.0"
 NOW:       "x-state": "Generally available; Added in 7.7.0"

IMO the output should match the @availability serverless definition and just say "Generally available".

@l-trotta l-trotta force-pushed the detailed-availability branch from 63774cd to 0c62637 Compare June 3, 2025 15:27
@l-trotta l-trotta marked this pull request as ready for review June 3, 2025 15:28
@l-trotta l-trotta requested a review from swallez June 3, 2025 15:28
@l-trotta
Copy link
Contributor Author

l-trotta commented Jun 3, 2025

@lcawl fixed the serverless output!
@swallez right now what happens if there's no flavor set in the configuration is that availability gets completely skipped, because if it does both stack end serverless we'll have the same bug as before where the first info gets overwritten. Should we have "stack" as default flavor if none is set?

@l-trotta l-trotta changed the title Openapi: adding more info in x-state Openapi: adding more info in x-state, adding x-state info to fields and params Jun 4, 2025
@l-trotta
Copy link
Contributor Author

l-trotta commented Jun 4, 2025

@lcawl @swallez ready for review!

Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler skip-backport This pull request should not be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenAPI] Publish availability.since in more contexts Update OpenAPI generator to include parameter- and property-level availability
2 participants