Skip to content

[Remote Clusters] Update callout and move server_name field#62352

Merged
alisonelizabeth merged 4 commits intoelastic:masterfrom
alisonelizabeth:remote_clusters/polish
Apr 7, 2020
Merged

[Remote Clusters] Update callout and move server_name field#62352
alisonelizabeth merged 4 commits intoelastic:masterfrom
alisonelizabeth:remote_clusters/polish

Conversation

@alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Apr 2, 2020 •
edited
Loading

Fixes #62328

Changes include:

  • Text improvements to cloud callout
  • The "Server name" input was moved above "Socket connections"
    • I'm interested in feedback on this. Right now, this change is applied for any environment, but I also considered if this should be a cloud-specific change since the server name field is only required on cloud.

Screen Shot 2020-04-02 at 1 46 14 PM

How to test

To simulate a cloud environment, enable the cloud plugin in your kibana.dev.yml file.

xpack.cloud.enabled: true
xpack.cloud.id: 'foobarbaz'

@alisonelizabeth alisonelizabeth added Feature:CCR and Remote Clusters v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.8.0 labels Apr 2, 2020
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner April 2, 2020 18:04
@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 2, 2020

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Apr 2, 2020

/cc @cristina-eleonora

@cristina-eleonora
Copy link

cristina-eleonora commented Apr 3, 2020

Thanks Alison!

@zanbel had some comments about changing single proxy address to proxy mode or just proxy, see #62328 (comment)

If we were to change it, the text would look like this:

Screenshot 2020-04-03 at 10 09 43

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Apr 3, 2020

@gchaps what are your thoughts on this?

@zanbel had some comments about changing single proxy address to proxy mode or just proxy, see #62328 (comment)

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Apr 6, 2020

@elasticmachine merge upstream

@gchaps
Copy link
Contributor

gchaps commented Apr 6, 2020

I checked with the Cloud writers, who suggested changing the title to this:

Use proxy mode for Elasticsearch Service deployment

Also, there should be a period at the end of this sentence:

Use seed nodes by default or switch to proxy mode.

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Apr 6, 2020

Thanks @cristina-eleonora and @gchaps for your feedback! Here is the updated screenshot:

Screen Shot 2020-04-06 at 5 02 35 PM

@zanbel
Copy link

zanbel commented Apr 7, 2020

@gchaps @alisonelizabeth this should in the same way for both ECE and ESS, so I would suggest a more generic term like "Elastic Cloud" instead of "Elasticsearch Service".

@cristina-eleonora
Copy link

cristina-eleonora commented Apr 7, 2020

@gchaps @alisonelizabeth this should in the same way for both ECE and ESS, so I would suggest a more generic term like "Elastic Cloud" instead of "Elasticsearch Service".

++++

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Apr 7, 2020

👍 I made the change from Elasticsearch Service --> Elastic Cloud.

jloleysens
jloleysens approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Just tested the code and it looks great 🎉

Regarding the move of server name to above socket connections; I think it is OK to keep the order consistent for Cloud and non-Cloud cases. Especially because socket connections already defaults to 18.

Non-cloud UI

Screenshot 2020-04-07 at 17 15 15

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 7, 2020

💚 Build Succeeded

History

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

@alisonelizabeth alisonelizabeth changed the title [Remote Clusters] Update cloud callout message + move server_name input [Remote Clusters] Update callout and move server_name field Apr 7, 2020
@alisonelizabeth alisonelizabeth merged commit 406d755 into elastic:master Apr 7, 2020
@alisonelizabeth alisonelizabeth deleted the remote_clusters/polish branch April 7, 2020 16:36
This was referenced Apr 7, 2020
Merged
Merged
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Apr 7, 2020
@alisonelizabeth
[Remote Clusters] Update callout and move server_name field (elastic#…
c0f4f3b 
…62352)
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Apr 7, 2020
@alisonelizabeth
[Remote Clusters] Update callout and move server_name field (elastic#…
d498469 
…62352)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 7, 2020
@gmmorris
Merge branch 'master' into alerting/tls-disable
184fde1 
* master:
  [APM] Change custom link from EuiListGroupItem to EuiLink (elastic#62742)
  [Remote Clusters] Update callout and move server_name field (elastic#62352)
  Removes Pitch Presentation Template from Canvas (elastic#62688)
  FTR: Enable w3c for chromedriver (elastic#62542)
  [ML] Disable functional tests
  [ILM] Skip failing API integration test (elastic#62779)
  [SIEM] Update beat doc (elastic#61902)
  [Search] Properly add slash preceding path in async search (elastic#62722)
  [APM] make sure environment query is correct for service maps… (elastic#62764)
  Add service map icon for rum-js agent type (elastic#62721)
  [APM] Service map - fixes irrelevant services on data refresh (elastic#62750)
  [APM] Service map - Fix taxi edge arrow orientation (elastic#62741)
  [APM] Prevent error rate alert trigger from rendering NaN (elastic#62754)
  [EPM] Store map visualizations from the package registry and use saved object ID (elastic#62059)
  [Alerting] for email action, set tls.rejectUnauthorized: false when secure: false (elastic#62380)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jloleysens jloleysens jloleysens approved these changes

Assignees

No one assigned

Labels

Feature:CCR and Remote Clusters release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.7.0 v7.8.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update the Add remote cluster page on hosted Kibana

7 participants

@alisonelizabeth @elasticmachine @cristina-eleonora @gchaps @zanbel @kibanamachine @jloleysens