Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix most esql.query request types #2671

Merged
merged 6 commits into from
Jul 10, 2024
Merged

Fix most esql.query request types #2671

merged 6 commits into from
Jul 10, 2024

Conversation

pquentin
Copy link
Member

@pquentin pquentin commented Jul 2, 2024

No description provided.

pquentin
pquentin commented Jul 2, 2024
View reviewed changes
specification/esql/query/QueryRequest.ts
locale?: string
/**
* To avoid any attempts of hacking or code injection, extract the values in a separate list of parameters. Use question mark placeholders (?) in the query string for each of the parameters.
* @doc_id esql-query-params
*/
params?: Array<ScalarValue>
params?: Array<EsqlParam>
profile?: boolean
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is undocumented but used in the YAML tests, which is why I'm adding without details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@pquentin
Copy link
Member Author

pquentin commented Jul 2, 2024 •
edited
Loading

With make validate, I get ✔ 174 out of 174 test request cases are passing.. I'll need to understand what is going on. Even if 223 is still an improvement over 218.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

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

API Status Request Response
esql.query 🔴 224/227 12/227

You can validate this API yourself by using the make validate target.

@pquentin
Copy link
Member Author

So I get 224/227 now (both locally and in CI thanks to #2690!), and all the remaining failures are related to LOOKUP and its new tables parameter that I'd like to tackle later.

@pquentin pquentin changed the title Fix esql.query request types Fix most esql.query request types Jul 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

API Status Request Response
esql.query 🔴 224/227 12/227

You can validate this API yourself by using the make validate target.

@pquentin pquentin merged commit 6180ad4 into main Jul 10, 2024
6 checks passed
@pquentin pquentin deleted the esql.query branch July 10, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@l-trotta l-trotta l-trotta approved these changes

@Anaethelion Anaethelion Awaiting requested review from Anaethelion

@flobernd flobernd Awaiting requested review from flobernd

Assignees
No one assigned
Labels
specification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pquentin @Anaethelion @l-trotta