Skip to content

Comments

Console update autocomplete definitions#39508

Merged
sebelga merged 11 commits intoelastic:masterfrom
sebelga:update-console-autocomplete-definitions
Jul 2, 2019
Merged

Console update autocomplete definitions#39508
sebelga merged 11 commits intoelastic:masterfrom
sebelga:update-console-autocomplete-definitions

Conversation

@sebelga
Copy link
Contributor

@sebelga sebelga commented Jun 24, 2019

This PR updates the console autocomplete definitions generated with the yarn spec_to_console.

@dimitris-athanasiou Can you please test the payloads for data frame analytics and confirm if the property feature_influence_threshold is correct?

I also made a small modification to the README.md file and updated the yarn script to make sure that the folder exists before trying to write a file into it.

@sebelga sebelga requested a review from cjcenizal June 24, 2019 12:10
@sebelga sebelga added Feature:Console Dev Tools Console Feature release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.3.0 v8.0.0 labels Jun 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cjcenizal cjcenizal requested review from alisonelizabeth and jen-huang and removed request for cjcenizal June 24, 2019 15:46
@cjcenizal
Copy link
Contributor

@sebelga I took a quick look and I just wanted to double-check that these files all belong in x-pack? Many of these endpoints (e.g. cat health) look like they belong in OSS: https://github.com/elastic/kibana/tree/master/src/legacy/core_plugins/console/api_server/spec/generated.

I also wonder how we define the generated spec files. Should we sync with Bill or re-watch his walkthrough video?

@sebelga
Copy link
Contributor Author

sebelga commented Jun 25, 2019

@cjcenizal I never thought we'd spread the generated definition across folders... I will re-watch the walkthrough video to see how to fix this.

EDIT: I figured it out, we indeed need to call the script twice, once for oss and once for xpack. I will now add manually the new APIs body payload

@sebelga sebelga changed the title [Console] Update autocomplete definitions [WIP] Console update autocomplete definitions Jun 25, 2019
@sebelga sebelga added the WIP Work in progress label Jun 25, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sebelga sebelga changed the title [WIP] Console update autocomplete definitions Console update autocomplete definitions Jun 26, 2019
@sebelga
Copy link
Contributor Author

sebelga commented Jun 26, 2019

@jen-huang @alisonelizabeth the PR is ready for review when you have time 😊

@cjcenizal cjcenizal self-requested a review June 26, 2019 16:37
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

LGTM. Tried out the yarn script and tested a couple of the autocomplete changes but not all.

@sebelga sebelga removed the WIP Work in progress label Jun 27, 2019
@cjcenizal
Copy link
Contributor

@sebelga Take a look at #19928 to refer to the value of manually reviewing and testing these endpoints. It looks like most of the things we caught were either a) errors in the overrides which we could fix in the PR or b) mistakes on the Elasticsearch side which we could forward on to the appropriate owner.

@sebelga
Copy link
Contributor Author

sebelga commented Jul 1, 2019

@cjcenizal I see. Do you think it would be valuable to work on a better (less error-prone and no engineering time involved) system? It seems to me that we should have a single source of truth with its tests (probably in the Elasticsearch repo) and all the dependent parties would pull from it without having to retest everything.

@cjcenizal
Copy link
Contributor

Do you think it would be valuable to work on a better (less error-prone and no engineering time involved) system? It seems to me that we should have a single source of truth with its tests (probably in the Elasticsearch repo) and all the dependent parties would pull from it without having to retest everything.

100% yes! The vision of what this system would look like isn't clear to me. I'd love to discuss any ideas you have.

@@ -0,0 +1,24 @@
{
Copy link
Contributor

@cjcenizal cjcenizal Jul 2, 2019

Choose a reason for hiding this comment

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

@dimitris-athanasiou I couldn't find docs for this API online. Do they live somewhere?

EDIT: Same goes for the put_data_frame_analytics API below.

Copy link
Contributor

Choose a reason for hiding this comment

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

No. We're working on the docs, they should be added this week.

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Code LGTM. I tested the overrides, some documentation links, and a few random endpoints in the browser and everything worked as expected. Thanks for doing this @sebelga!

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

The data frame analytics bits look good to me!

@sebelga
Copy link
Contributor Author

sebelga commented Jul 2, 2019

Thanks for the review @dimitris-athanasiou, @alisonelizabeth @cjcenizal!

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sebelga
Copy link
Contributor Author

sebelga commented Jul 2, 2019

retest

1 similar comment
@sebelga
Copy link
Contributor Author

sebelga commented Jul 2, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alisonelizabeth
Copy link
Contributor

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sebelga sebelga merged commit 40089f9 into elastic:master Jul 2, 2019
@sebelga sebelga deleted the update-console-autocomplete-definitions branch July 2, 2019 16:33
sebelga added a commit to sebelga/kibana that referenced this pull request Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Console Dev Tools Console Feature release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.3.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants