Skip to content

Add esql version #2512

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

Merged
merged 4 commits into from
Apr 17, 2024
Merged

Add esql version #2512

merged 4 commits into from
Apr 17, 2024

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 15, 2024

Adds a version parameter to the ESQL _query request endpoint. We'll do some work to give this a default value in the generated clients soon, but for 8.14+ this'll be a required parameter.

Adds a `version` parameter to the ESQL `_query` request endpoint. We'll
do some work to give this a default value in the generated clients soon,
but for 8.14+ this'll be a required parameter.
@nik9000
Copy link
Member Author

nik9000 commented Apr 15, 2024

You can validate these APIs yourself by using the make validate target.

I'm running:

make validate api=esql.query type=request stack-version=8.14.0-SNAPSHOT

and it's been half an hour or so installing a flight recorder.

Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

Can't comment on how this affects clients etc., but I checked/proofread the contained docs.

Thanks a lot @nik9000 !

Only reservation I have is whether official specs should already document "version": "snapshot", which is intended for testing purposes only.

"name": "2024.04.01"
},
{
"description": "Run against the unreleased version of the ESQL language. This will\ncontain any changes staged for release that have yet to be included\nin a version.",
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO we should mention that this only works on SNAPSHOT builds. ES on non-SNAPSHOT builds will return a 400 if it sees "version": "snapshot".

I wonder if we should even make snapshot a part of the official specs - it's not like users miss out if they cannot use language feature xyz, given that existing language versions are going to still be updated with new features.

Can we maybe add snapshot here later, if it turns out that there's an important enough use case for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'll drop it.

query: string
query: string,
/**
* The version of the ES|QL language in which the "query" field uses was written.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The version of the ES|QL language in which the "query" field uses was written.
* The version of the ES|QL language in which the "query" field was written.

Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
esql.query 🔴 -1/177 6/177

You can validate these APIs yourself by using the make validate target.

Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

I committed the updated jsons obtained from make generate.

Looks good to me!

@flobernd
Copy link
Member

fyi: I created this workflow a while ago which automatically takes care of generating- and pushing the output json, if it is not part of the PR. This makes including the output json optional 🙂 It was primarily done to allow users to contribute without setting up Node etc., but might as well save some time for everybody else.

@nik9000 nik9000 merged commit 947a1ea into main Apr 17, 2024
6 checks passed
@nik9000 nik9000 deleted the esql_version branch April 17, 2024 12:27
@nik9000
Copy link
Member Author

nik9000 commented Apr 17, 2024

Cool! I've just merged. Let me know if we need anything else around version~

github-actions bot pushed a commit that referenced this pull request Apr 22, 2024
Adds a `version` parameter to the ESQL `_query` request endpoint. We'll
do some work to give this a default value in the generated clients soon,
but for 8.14+ this'll be a required parameter.

---------

Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
(cherry picked from commit 947a1ea)
l-trotta pushed a commit that referenced this pull request Apr 24, 2024
Adds a `version` parameter to the ESQL `_query` request endpoint. We'll
do some work to give this a default value in the generated clients soon,
but for 8.14+ this'll be a required parameter.

---------

Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
(cherry picked from commit 947a1ea)

Co-authored-by: Nik Everett <nik9000@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants