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

yuliacech · 2023-08-01T16:07:07Z

Summary

Fixes #160539

This PR updates the logic for dynamic parameters in the Console autocomplete engine in preparation for the new script.

Changes

The old script packages/spec-to-console contained some logic that would rename parameters in the json specs: {index} -> {indices}, {node_id} -> {nodes}, {metric} -> {metrics}. I don't think we need this anymore because such logic only introduces more hidden mechanics to the script. There is no significant improvements to autocomplete suggestions due to these replacements.
The dynamic parameters defined in the Console autocomplete engine:
- indices - is deleted and only index is used instead. Originally, there was a minor difference between indices and index: the former should accept several index names, the latter only 1. But this logic is not working for urls currently anyways. I don't think there is a significant improvement for UX to keep this distinction. The main useful feature of this parameter is to display a list of indices in the deployment.
- type and types - deleted because index mapping types is a removed ES feature (see this doc)
- id, transform_id, ids, task_id - deleted as not working. Also definitions using these parameters don't intend them to be autofilled. For example, in the url /_async_search/{id} the parameter {id} can be any string.
- user and username - deleted as incorrectly being resolved to a list of indices. Also definitions using these parameters don't intend them to be autofilled, for example _security/user/{username} when creating a new user.
- node - delete because it was just an empty list
- nodes - deleted because the suggestions of the form ['_local', '_master', 'data:true', 'data:false', 'master:true', 'master:false'] are not correct for definitions where a node ID is expected.
Renamed variables to dynamic parameters in the Console README file: I first used the word variables in readme for the values defined in the file kb.js. But then variables support was added to Console and there were several sections in the readme using the word variables for different concepts. Since in the autocomplete engine code, the function is called parametrizedComponentFactories I think calling these values dynamic parameters makes more sense and avoids confusion.

yuliacech · 2023-08-02T16:36:53Z

@elasticmachine merge upstream

yuliacech · 2023-08-02T18:42:15Z

src/plugins/console/public/lib/autocomplete/body_completer.js

-    throw new Error("no factory found for '" + value + "'");
+    console.warn("[Console] no factory found for '" + value + "'");
+    // using upper case as an indication that this value is a parameter
+    return new ConstantComponent(value.toUpperCase());


Instead of throwing an error and failing all definitions loading, we log a warning and use a constant with an upper case string.

yuliacech · 2023-08-02T18:43:28Z

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

-    names.unshift(name);
-    _.each(apiJson.globals || {}, function (globalJson, globalName) {
-      api.addGlobalAutocompleteRules(globalName, globalJson);
+  try {


We need a try-catch clause here, otherwise if there is an error while loading definitions, the process fails silently.

elasticmachine · 2023-08-03T06:56:58Z

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

alisonelizabeth

Nice work @yuliacech! Thanks for updating the readme as well - super helpful. I left one question about one of your code comments, but I don't think it's directly related to this PR. Also, found a small typo in the readme. Otherwise, LGTM.

src/plugins/console/README.md

alisonelizabeth · 2023-08-03T16:54:15Z

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

  },
+  // composable index templates
+  // currently seems to be unused, but that is a useful functionality


Can you elaborate on this? I believe this functionality was added via #124655. I don't remember it being removed, so I wonder if there was a regression somewhere 🤔

Good catch, thanks @alisonelizabeth! I checked the PR and the dynamic parameters were added there and they indeed still work correctly. But their usage was in the generated definitions file. The next time we used the script (#153098), we reverted the manual changes made to those definitions. So instead of _component_template/{component_template} we have _component_template/{name} because name is how it's defined in the specs. I'll open an issue to use those dynamic parameters, but in the overrides files.
I think the goal is to not edit generated files and not even have to review them when they are re-generated using the script. On the other hand, we should spend more time on updating the overrides.

Sounds good. Thanks for the explanation and looking into this!

Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>

yuliacech · 2023-08-04T12:44:30Z

Thanks a lot for the review, @alisonelizabeth!

kibana-ci · 2023-08-04T13:03:06Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`console`	205	201	-4

Async chunks

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

id	before	after	diff
`console`	418.6KB	416.6KB	-2.0KB

Page load bundle

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

id	before	after	diff
`console`	27.4KB	26.7KB	-703.0B

History

💚 Build #147238 succeeded 25c597a
💚 Build #147068 succeeded 5420d6f
💔 Build #147013 failed 013c8f7
💚 Build #146902 succeeded c4ac947
💔 Build #146875 failed 98a9aa7

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

## Summary While working on #162917, I noticed that there is a property `url_components` that is being generated with the old script. This PR adds the same functionality to the new script. The purpose of this property is to provide autocomplete suggestions for dynamic parameters used in `patterns`. For example, the URL defined as `_cluster/state/{metrics}` has a list of options allowed for the parameter `metrics`. These options are defined in the json specs, but unfortunately they are not always translated into concrete types in the specification repo. In this example, `metrics` is described as `string | string[]` in the specification repo. Because of that I added some override files with `url_components` that will not be available when we switch to the new script and re-generate the definitions with it. The logic for generating the url components is the same as for query parameters. Most changes in this PR is extracting the code so that both functions `generateQueryParams` and `generateUrlComponents` can share it. The only difference between query parameters and url components: with query parameters we keep all properties so that all parameters are displayed in the autocomplete suggestions, for example `{ param1: '', param2: '' }` provide information about 2 query parameters, specifically their names. For url components, the name of the parameter is not important, because only the values will be displayed for autocompletion. That is why, we ignore all parameters without known values and only keep properties like `{ param: ['value1', 'value2'] }`.

## Summary Fixes elastic#160539 This PR updates the logic for dynamic parameters in the Console autocomplete engine in preparation for the new script. #### Changes - The old script `packages/spec-to-console` contained some logic that would rename parameters in the json specs: `{index} -> {indices`}, `{node_id} -> {nodes}, {metric} -> {metrics}`. I don't think we need this anymore because such logic only introduces more hidden mechanics to the script. There is no significant improvements to autocomplete suggestions due to these replacements. - The dynamic parameters defined in the Console autocomplete engine: - `indices` - is deleted and only `index` is used instead. Originally, there was a minor difference between `indices` and `index`: the former should accept several index names, the latter only 1. [But this logic is not working for urls currently anyways](elastic#163000). I don't think there is a significant improvement for UX to keep this distinction. The main useful feature of this parameter is to display a list of indices in the deployment. - `type` and `types` - deleted because `index mapping types` is a removed ES feature (see this [doc](https://www.elastic.co/guide/en/elasticsearch/reference/6.0/removal-of-types.html)) - `id`, `transform_id`, `ids`, `task_id` - deleted as not working. Also definitions using these parameters don't intend them to be autofilled. For example, in the url `/_async_search/{id}` the parameter `{id}` can be any string. - `user` and `username` - deleted as incorrectly being resolved to a list of indices. Also definitions using these parameters don't intend them to be autofilled, for example `_security/user/{username}` when creating a new user. - `node` - delete because it was just an empty list - `nodes` - deleted because the suggestions of the form `['_local', '_master', 'data:true', 'data:false', 'master:true', 'master:false']` are not correct for definitions where a node ID is expected. - Renamed `variables` to `dynamic parameters` in the Console README file: I first used the word `variables` in readme for the values defined in the file [`kb.js`](https://github.com/elastic/kibana/blob/6e1445b66aae27a9a762ed5632549b3f6a6dfa51/src/plugins/console/public/lib/kb/kb.js). But then variables support was added to Console and there were several sections in the readme using the word `variables` for different concepts. Since in the autocomplete engine code, the function is called `parametrizedComponentFactories` I think calling these values `dynamic parameters` makes more sense and avoids confusion. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>

## Summary While working on elastic#162917, I noticed that there is a property `url_components` that is being generated with the old script. This PR adds the same functionality to the new script. The purpose of this property is to provide autocomplete suggestions for dynamic parameters used in `patterns`. For example, the URL defined as `_cluster/state/{metrics}` has a list of options allowed for the parameter `metrics`. These options are defined in the json specs, but unfortunately they are not always translated into concrete types in the specification repo. In this example, `metrics` is described as `string | string[]` in the specification repo. Because of that I added some override files with `url_components` that will not be available when we switch to the new script and re-generate the definitions with it. The logic for generating the url components is the same as for query parameters. Most changes in this PR is extracting the code so that both functions `generateQueryParams` and `generateUrlComponents` can share it. The only difference between query parameters and url components: with query parameters we keep all properties so that all parameters are displayed in the autocomplete suggestions, for example `{ param1: '', param2: '' }` provide information about 2 query parameters, specifically their names. For url components, the name of the parameter is not important, because only the values will be displayed for autocompletion. That is why, we ignore all parameters without known values and only keep properties like `{ param: ['value1', 'value2'] }`.

yuliacech added 3 commits August 1, 2023 17:24

[Console] Update variables for the new script

96dc776

[Console] Fix unit tests

39e9e0f

[Console] Update unit tests

6e1445b

yuliacech changed the title ~~[Console] Update variables for the new script~~ [Console] Update dynamic parameters for the new script Aug 2, 2023

Merge branch 'main' into console/update_vars

d0496bb

kibanamachine and others added 3 commits August 2, 2023 12:36

Merge branch 'main' into console/update_vars

98a9aa7

[Console] Update README.md

6241688

Merge branch 'main' into console/update_vars

c4ac947

yuliacech commented Aug 2, 2023

View reviewed changes

yuliacech marked this pull request as ready for review August 3, 2023 06:55

yuliacech requested a review from a team as a code owner August 3, 2023 06:55

Merge branch 'main' into console/update_vars

013c8f7

yuliacech added Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Aug 3, 2023

Merge branch 'main' into console/update_vars

5420d6f

yuliacech added the ci:cloud-deploy Create or update a Cloud deployment label Aug 3, 2023

yuliacech mentioned this pull request Aug 3, 2023

[Console] Generate url components with the new script #163096

Merged

Merge branch 'main' into console/update_vars

25c597a

alisonelizabeth approved these changes Aug 3, 2023

View reviewed changes

Update src/plugins/console/README.md

86c86aa

Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>

yuliacech enabled auto-merge (squash) August 4, 2023 12:43

yuliacech merged commit 04b3d17 into elastic:main Aug 4, 2023
21 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Aug 4, 2023

ElenaStoeva mentioned this pull request Aug 10, 2023

[Console] Update autocomplete variables #162156

Closed

yuliacech deleted the console/update_vars branch February 15, 2024 12:05

yuliacech mentioned this pull request May 31, 2024

[Console] Fields autocomplete suggestions are shown for all indices #184584

Closed

yuliacech mentioned this pull request Jun 21, 2024

[Console Monaco migration] Fixes #186479

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

yuliacech commented Aug 1, 2023 •

edited

Loading

yuliacech commented Aug 2, 2023

yuliacech Aug 2, 2023

yuliacech Aug 2, 2023

elasticmachine commented Aug 3, 2023

alisonelizabeth left a comment

alisonelizabeth Aug 3, 2023 •

edited

Loading

yuliacech Aug 4, 2023

alisonelizabeth Aug 4, 2023

yuliacech commented Aug 4, 2023

kibana-ci commented Aug 4, 2023 •

edited

Loading

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

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

Conversation

yuliacech commented Aug 1, 2023 • edited Loading

Summary

Changes

yuliacech commented Aug 2, 2023

yuliacech Aug 2, 2023

Choose a reason for hiding this comment

yuliacech Aug 2, 2023

Choose a reason for hiding this comment

elasticmachine commented Aug 3, 2023

alisonelizabeth left a comment

Choose a reason for hiding this comment

alisonelizabeth Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

yuliacech Aug 4, 2023

Choose a reason for hiding this comment

alisonelizabeth Aug 4, 2023

Choose a reason for hiding this comment

yuliacech commented Aug 4, 2023

kibana-ci commented Aug 4, 2023 • edited Loading

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

yuliacech commented Aug 1, 2023 •

edited

Loading

alisonelizabeth Aug 3, 2023 •

edited

Loading

kibana-ci commented Aug 4, 2023 •

edited

Loading