-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[OAS] Remove elastic-api-version
from stateful spec
#197968
[OAS] Remove elastic-api-version
from stateful spec
#197968
Conversation
…cumented commands and update the readme
Pinging @elastic/kibana-core (Team:Core) |
oas_docs/makefile
Outdated
@npx @redocly/cli bundle output/kibana.tmp4.yaml --ext yaml -o output/kibana.new.yaml | ||
@npx @redocly/cli bundle output/kibana.serverless.tmp4.yaml --ext yaml -o output/kibana.serverless.yaml | ||
@npx @redocly/cli bundle output/kibana.tmp4.yaml --ext yaml -o output/kibana.yaml | ||
@sed -i '' -e 's/; Elastic-Api-Version=[0-9]\{4\}-[0-9]\{2\}-[0-9]\{2\}//g' output/kibana.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukeelmers this is where we remove the Elastic-Api-Version
from the stateful bundles. For now I think this should be OK as, in future, we might choose to introduce them there more formally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! does it need to be there in serverless too... it feels like we could remove it in both places, right? (since it looks like this is the one that's in the content-type
which afaik nobody needs to actually include)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my concern for stateful is that it will be confusing as we have not formally/publicly explained how the dual date/stack versioning works there.
my concern for serverless is that it is just redundant and also a bit confusing since content-type does not dictate api version (unless I missed something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my concern for serverless is that it is just redundant and also a bit confusing since content-type does not dictate api version (unless I missed something)
My understanding is it is intended to be used for grouping our serverless versions. Until that strategy changes from docs side I don't think we should remove it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relates to elastic/docs-content#142 As soon as we start having multiple valid in-support elastic-api-version values in stateful or serverless, we'll need to be able to differentiate their impact on the requests and responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... we have not formally/publicly explained how the dual date/stack versioning works there.
Whenever we're ready to document that, it can be added to the API docs in a topic similar to https://www.elastic.co/docs/api/doc/kibana/topic/topic-kibana-spaces, which we add via
x-topics: |
@elasticmachine merge upstream |
💔 Build Failed
Failed CI StepsTest Failures
Metrics [docs]
History
|
I think this also has some overlap with the info in elastic/docs-content#141 |
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
Superseded by #202923 |
Summary
Remove mention of
elastic-api-version
from the stateful spec bundle (for now just a simplesed
command).Additionally:
make api-docs
now does everything including applying overlays to get the final bundles*.new.*
workflows, happy to bring them back but they seemed a bit redundant and we can always look at the contents ofmakefile
to recreate them on demandThese also fix the current
.buildkite/scripts/steps/openapi_bundling/final_merge.sh
running in CI that assumed everything was being done to create the "final" merge.