[Console] Support suggesting index templates v2 #124655

mibragimov · 2022-02-04T07:17:12Z

Release note

Console in Dev Tools now supports autocompletion for index templates and component templates introduced in Elasticsearch 7.8.

Kuznietsov

Solution LGTM. Thanks, @mibragimov.
Left some comments to be addressed.

Kuznietsov · 2022-02-07T07:47:31Z

src/plugins/console/public/lib/autocomplete/components/index_template_autocomplete_component.js

+  }
+
+  getDefaultTermMeta() {
+    return 'name';


I'm curious about the reason for not using a pattern of TypeAutocompleteComponent or FieldAutocompleteComponent classes here.

According to that pattern, getDefaultTermMeta should return index_template, and getContextKey - index_templates.

On the other hand, TemplateAutocompleteComponent is using only getContextKey.

And then, at src/plugins/console/public/lib/kb/kb.js, it is provided a template for every key.

So, my suggestion is to follow the style of TypeAutocompleteComponent or FieldAutocompleteComponent classes and add support of both meaningful keywords at the src/plugins/console/public/lib/kb/kb.js factories, or go by the path of TemplateAutocompleteComponent, add support only for the context key and add its support to the parametrizedComponentFactories.

I prefer to follow the way of TemplateAutocompleteComponent, as they are close by the idea with IndexTemplateAutocompleteComponent. WDYT?

There is bit of a problem with long index template names. See screenshots below:

Perhaps we should leave name as default here. WDYT?

The main idea of the context key is to identify the element, which has been shown at the prompt. Let us imagine, one day we'll receive the task to add templates to _index_template's autocompletion results.

And you'll have:

.ml-state-temp template .ml-state name

But should see:

.ml-state-temp template .ml-state index_template

What about the long name of the index, possibly, we should pick some shorter meaningful name for the index_template context key. The other option is resizing the .ace-editor.ace-autocomplete. Or, as this problem has been before, leave it as it is.

@cjcenizal could you please review the changes and advise regarding the comments above?

Thanks for the ping! Someone from @elastic/platform-deployment-management will provide input.

I agree with @mibragimov and would prefer to use index_template.

I see the issue with the spacing, though. Perhaps we should address this as a separate issue, as the same could happen in other scenarios, e.g., if you have a really long index name. Thoughts?

Created a separate issue for this #125186. I'll work on that

src/plugins/console/public/lib/kb/kb.js

src/plugins/console/public/lib/mappings/mappings.js

Kuznietsov · 2022-02-07T08:37:10Z

@elasticmachine merge upstream

Kuznietsov

Code LGTM 👍

What about the width of the autocomplete prompt window, let us listen to the opinion of the @elastic/platform-deployment-management.

elasticmachine · 2022-02-08T13:03:13Z

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

alisonelizabeth · 2022-02-09T14:30:38Z

@elasticmachine merge upstream

alisonelizabeth

Great work @mibragimov!

Tested locally and works as expected. I went ahead and updated the PR description to include a release note. I also updated the labels to include 8.2. I believe main is currently representative of 8.2, and then this will need to be backported to the 8.1 branch.

Also, a couple questions -

1 - Is it possible to add any tests around this new functionality?
2 - What are your thoughts about renaming areas of the code for the legacy template autocomplete to be prefixed with legacy? For example, TemplateAutocompleteComponent would become LegacyTemplateAutocompleteComponent. Otherwise, I could see it potentially confusing if someone else comes into the code and needs to differentiate between the template autocomplete logic vs. the index_template autocomplete logic. WDYT?

mibragimov · 2022-02-10T05:24:43Z

Great work @mibragimov!

Tested locally and works as expected. I went ahead and updated the PR description to include a release note. I also updated the labels to include 8.2. I believe main is currently representative of 8.2, and then this will need to be backported to the 8.1 branch.

Also, a couple questions -

1 - Is it possible to add any tests around this new functionality? 2 - What are your thoughts about renaming areas of the code for the legacy template autocomplete to be prefixed with legacy? For example, TemplateAutocompleteComponent would become LegacyTemplateAutocompleteComponent. Otherwise, I could see it potentially confusing if someone else comes into the code and needs to differentiate between the template autocomplete logic vs. the index_template autocomplete logic. WDYT?

Thank you for the review @alisonelizabeth! I completely agree with the suggested changes. Working on these

mibragimov · 2022-02-10T08:05:17Z

@alisonelizabeth @Kunzetsov I haven't noticed this point from #75967

Note that we also want to suggest the names of existing component templates for the _component_template API.

So I added support for _component_template as well. Working on the tests.

alisonelizabeth · 2022-02-10T13:56:18Z

Thanks for making those changes @mibragimov! Let me know when you add tests and I will review again.

mibragimov · 2022-02-14T04:41:37Z

@elasticmachine merge upstream

kibana-ci · 2022-02-14T06:15:09Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`console`	165	168	+3

Async chunks

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

id	before	after	diff
`console`	378.3KB	379.6KB	+1.2KB

History

💚 Build #23364 succeeded 754d225
💚 Build #23039 succeeded f9d6bd4
💚 Build #22790 succeeded 028f05d
💚 Build #22347 succeeded a4d47cf
💚 Build #22030 succeeded 03d7199
💚 Build #22016 succeeded 8bb606f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mibragimov

alisonelizabeth

Latest changes LGTM. Thanks @mibragimov!

* Add autocomplete suggestions for index template api * Add support for component templates * Add unit tests Co-authored-by: Muhammad Ibragimov <[email protected]> Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 75ac8e5)

kibanamachine · 2022-02-15T09:02:05Z

💚 All backports created successfully

Status	Branch	Result
✅	8.1

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

Questions ?

Please refer to the Backport tool documentation

* Add autocomplete suggestions for index template api * Add support for component templates * Add unit tests Co-authored-by: Muhammad Ibragimov <[email protected]> Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 75ac8e5) Co-authored-by: Muhammad Ibragimov <[email protected]>

…pdf-generation-in-worker-thread * 'main' of github.com:elastic/kibana: (40 commits) Service overview e2e test (elastic#125359) [Graph] Make graph edges easier to click (elastic#124053) [Reporting] Add additional PNG and PDF metrics (elastic#125450) [APM] Lint rule for explicit return types (elastic#124771) [SecuritySolution] Enrich threshold data from correct fields (elastic#125376) [Uptime monitor management] Make index template installation retry (elastic#125537) [Console] Support suggesting index templates v2 (elastic#124655) [Logs UI] Support switching between log source modes (elastic#124929) SavedObjects management: change index patterns labels to data views (elastic#125356) [ci-stats] add Client class for accessing test group stats (elastic#125164) [ML] Use Discover locator in Data Visualizer instead of URL Generator (elastic#124283) [Fleet] Add retries when starting docker registry in integration tests (elastic#125530) Update osquery.asciidoc to address doc issue (elastic#125425) synthetics e2e - use custom location when defined (elastic#125560) [ci] Retry failed steps in on-merge pipeline (elastic#125550) [babel] Bump babel packages (elastic#125446) [DOCS] Updates Upgrade Assistant API status (elastic#125410) [DOCS] Minor tweaks to upgrade docs (elastic#125538) [Uptime] handle null duration on ping list (elastic#125438) [TSVB][Lens] Navigate to Lens with your current configuration (elastic#114794) ... # Conflicts: # x-pack/plugins/reporting/server/export_types/common/pdf/pdfmaker.ts # x-pack/plugins/reporting/server/export_types/printable_pdf/lib/generate_pdf.ts # x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts

mibragimov added the WIP Work in progress label Feb 4, 2022

mibragimov requested review from alexwizp, Kuznietsov and vladpro25 February 4, 2022 07:17

mibragimov self-assigned this Feb 4, 2022

mibragimov force-pushed the index-template branch from 071ddb7 to 8bb606f Compare February 7, 2022 03:43

Kuznietsov reviewed Feb 7, 2022

View reviewed changes

Kuznietsov approved these changes Feb 8, 2022

View reviewed changes

mibragimov marked this pull request as ready for review February 8, 2022 13:03

mibragimov requested a review from a team as a code owner February 8, 2022 13:03

alisonelizabeth self-requested a review February 8, 2022 18:52

alisonelizabeth added v8.2.0 and removed backport:skip This commit does not require backporting labels Feb 9, 2022

alisonelizabeth reviewed Feb 9, 2022

View reviewed changes

mibragimov mentioned this pull request Feb 10, 2022

[Console] Autocomplete window doesn't have enough space for long index names #125186

Closed

mibragimov force-pushed the index-template branch 2 times, most recently from 234bff3 to f9d6bd4 Compare February 10, 2022 08:01

Kuznietsov approved these changes Feb 10, 2022

View reviewed changes

Muhammad Ibragimov added 4 commits February 11, 2022 12:20

Add autocomplete suggestions for index template api

6085f72

Address comments

e206204

Add support for component templates

a8b9a8a

Add unit tests

754d225

mibragimov force-pushed the index-template branch from f9d6bd4 to 754d225 Compare February 11, 2022 07:21

Merge branch 'main' into index-template

0f1d9a8

mibragimov requested a review from alisonelizabeth February 14, 2022 05:47

alisonelizabeth approved these changes Feb 14, 2022

View reviewed changes

mibragimov added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 15, 2022

mibragimov merged commit 75ac8e5 into elastic:main Feb 15, 2022

mibragimov deleted the index-template branch February 15, 2022 08:58

kibanamachine mentioned this pull request Feb 15, 2022

[8.1] [Console] Support suggesting index templates v2 (#124655) #125612

Merged

alisonelizabeth mentioned this pull request Aug 3, 2023

[Console] Update dynamic parameters for the new script #162917

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Support suggesting index templates v2 #124655

[Console] Support suggesting index templates v2 #124655

mibragimov commented Feb 4, 2022 •

edited

Loading

Kuznietsov left a comment

Kuznietsov Feb 7, 2022

mibragimov Feb 8, 2022

Kuznietsov Feb 8, 2022 •

edited

Loading

mibragimov Feb 8, 2022

cjcenizal Feb 8, 2022

alisonelizabeth Feb 9, 2022 •

edited

Loading

mibragimov Feb 10, 2022

Kuznietsov commented Feb 7, 2022

Kuznietsov left a comment

elasticmachine commented Feb 8, 2022

alisonelizabeth commented Feb 9, 2022

alisonelizabeth left a comment

mibragimov commented Feb 10, 2022

mibragimov commented Feb 10, 2022

alisonelizabeth commented Feb 10, 2022

mibragimov commented Feb 14, 2022

kibana-ci commented Feb 14, 2022

alisonelizabeth left a comment

kibanamachine commented Feb 15, 2022

[Console] Support suggesting index templates v2 #124655

[Console] Support suggesting index templates v2 #124655

Conversation

mibragimov commented Feb 4, 2022 • edited Loading

Release note

Kuznietsov left a comment

Choose a reason for hiding this comment

Kuznietsov Feb 7, 2022

Choose a reason for hiding this comment

mibragimov Feb 8, 2022

Choose a reason for hiding this comment

Kuznietsov Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

mibragimov Feb 8, 2022

Choose a reason for hiding this comment

cjcenizal Feb 8, 2022

Choose a reason for hiding this comment

alisonelizabeth Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

mibragimov Feb 10, 2022

Choose a reason for hiding this comment

Kuznietsov commented Feb 7, 2022

Kuznietsov left a comment

Choose a reason for hiding this comment

elasticmachine commented Feb 8, 2022

alisonelizabeth commented Feb 9, 2022

alisonelizabeth left a comment

Choose a reason for hiding this comment

mibragimov commented Feb 10, 2022

mibragimov commented Feb 10, 2022

alisonelizabeth commented Feb 10, 2022

mibragimov commented Feb 14, 2022

kibana-ci commented Feb 14, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

alisonelizabeth left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 15, 2022

💚 All backports created successfully

Questions ?

mibragimov commented Feb 4, 2022 •

edited

Loading

Kuznietsov Feb 8, 2022 •

edited

Loading

alisonelizabeth Feb 9, 2022 •

edited

Loading