Skip to content

[ES|QL] separate KEEP, DROP, and SORT autocomplete routines#197744

Merged
drewdaemon merged 9 commits intoelastic:mainfrom
drewdaemon:195418/separate-autocomplete-routines-1
Oct 29, 2024
Merged

[ES|QL] separate KEEP, DROP, and SORT autocomplete routines#197744
drewdaemon merged 9 commits intoelastic:mainfrom
drewdaemon:195418/separate-autocomplete-routines-1

Conversation

@drewdaemon
Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon commented Oct 24, 2024

Summary

This PR begins the refactor described in #195418.

The autocomplete engine now delegates to command-specific routines attached to the command definitions for KEEP, DROP, and SORT.

The naming of getFieldsFor has been broadened to getColumnsFor because the response from Elasticsearch can contain variables as well as fields, depending on the query that is used to fetch the columns.

No user-facing behavior should have changed.

Checklist

@drewdaemon drewdaemon force-pushed the 195418/separate-autocomplete-routines-1 branch from f33357a to d4dfe34 Compare October 28, 2024 20:51
@drewdaemon drewdaemon changed the title [ES|QL] move sort routine to command definition [ES|QL] separate KEEP, DROP, and SORT autocomplete routines Oct 28, 2024
@drewdaemon drewdaemon added Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:version Backport to applied version labels v8.17.0 labels Oct 29, 2024
@drewdaemon drewdaemon marked this pull request as ready for review October 29, 2024 00:32
@drewdaemon drewdaemon requested a review from a team as a code owner October 29, 2024 00:32
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@drewdaemon
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@drewdaemon
Copy link
Copy Markdown
Contributor Author

@stratoula I'm aware that these commands aren't the top priority, but I'm cutting my teeth on the simplest commands. Now that a pattern is established, I will tackle the more pressing ones.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6132 6135 +3
unifiedSearch 356 359 +3
total +6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
esql 180.5KB 180.5KB +1.0B
securitySolution 20.5MB 21.0MB ⚠️ +511.9KB
total +511.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.4MB 3.4MB +1.1KB
Unknown metric groups

References to deprecated APIs

id before after diff
@kbn/monaco 9 10 +1

Unreferenced deprecated APIs

id before after diff
@kbn/esql-validation-autocomplete 0 1 +1

History

@stratoula
Copy link
Copy Markdown
Contributor

It is ok Drew! Whatever makes your work easier 🙌

Copy link
Copy Markdown
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This is very nice Drew, very clean, very readable.

I also tested the 3 commands, work as expected 💯

}>
) {
const finalFields = customFields || fields;
const finalColumnsSinceLastCommand = customColumnsSinceLastCommand || fields;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what a lovely variable name

}

/**
* @deprecated — this generic logic will be replaced with the command-specific suggest functions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome thanx for the comment 👍

validate?: (option: ESQLCommand) => ESQLMessage[];
modes: CommandModeDefinition[];
hasRecommendedQueries?: boolean;
/** @deprecated this property will disappear in the future */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like it 👍

@drewdaemon
Copy link
Copy Markdown
Contributor Author

@stratoula thank you for the comments! Did you mean to leave an approval?

@stratoula
Copy link
Copy Markdown
Contributor

@drewdaemon I approved but it must be due to the kibana repo being set to private.

@drewdaemon drewdaemon merged commit 11ae6a5 into elastic:main Oct 29, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11577596927

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 29, 2024
…astic#197744)

## Summary

This PR begins the refactor described in
elastic#195418.

The autocomplete engine now delegates to command-specific routines
attached to the command definitions for `KEEP`, `DROP`, and `SORT`.

The naming of `getFieldsFor` has been broadened to `getColumnsFor`
because the response from Elasticsearch can contain variables as well as
fields, depending on the query that is used to fetch the columns.

No user-facing behavior should have changed.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 11ae6a5)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x #198159

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 30, 2024
…SORT&#x60; autocomplete routines (#197744) (#198159)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] separate &#x60;KEEP&#x60;, &#x60;DROP&#x60;, and
&#x60;SORT&#x60; autocomplete routines
(#197744)](#197744)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2024-10-29T15:50:30Z","message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.17.0"],"title":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete
routines","number":197744,"url":"https://github.com/elastic/kibana/pull/197744","mergeCommit":{"message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197744","number":197744,"mergeCommit":{"message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Drew Tate <drew.tate@elastic.co>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:ESQL ES|QL related features in Kibana t// v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants