Skip to content

Comments

[Remote clusters] Adopt changes to remote info API#60795

Merged
alisonelizabeth merged 2 commits intoelastic:masterfrom
alisonelizabeth:bugfix/remote_clusters/api_changes
Mar 23, 2020
Merged

[Remote clusters] Adopt changes to remote info API#60795
alisonelizabeth merged 2 commits intoelastic:masterfrom
alisonelizabeth:bugfix/remote_clusters/api_changes

Conversation

@alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Mar 20, 2020

This PR adopts changes made recently to the ES remote cluster info API.

Related ES changes: elastic/elasticsearch#53634, elastic/elasticsearch#53441

@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 labels Mar 20, 2020
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 20, 2020

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

@jloleysens
Copy link
Contributor

jloleysens commented Mar 23, 2020

@elasticmachine merge upstream

@jloleysens jloleysens self-requested a review March 23, 2020 10:22
jloleysens
jloleysens approved these changes Mar 23, 2020 •
edited
Loading
View reviewed changes
@jloleysens jloleysens self-requested a review March 23, 2020 10:29
jloleysens
jloleysens reviewed Mar 23, 2020
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@alisonelizabeth Thanks for working on this fix!

I tested locally and noticed that after creating a remote cluster with proxy settings and editing it the proxy address and server name fields are empty (even though I provided both when creating the remote cluster).

Screenshot 2020-03-23 at 11 25 45

My questions are:

Is this expected?
Could it be related to these changes?
If both are "no" we can address this in a separate PR, just wanted to clarify before approving (even though I already approved 🤦‍♂

@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 23, 2020

💚 Build Succeeded

History

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

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Mar 23, 2020

@jloleysens thanks for the review!

I tested locally and noticed that after creating a remote cluster with proxy settings and editing it the proxy address and server name fields are empty (even though I provided both when creating the remote cluster).

I'm having a hard time reproducing this. Do you think test again? I did find a bug when trying to remove the server name field in edit mode and opened #60902.

@jloleysens
Copy link
Contributor

jloleysens commented Mar 23, 2020

@alisonelizabeth Yeah it is strange, I am not seeing the same behaviour on master, so it must be something in my local setup on that branch or in the code changes. This is what I am seeing:

create-and-edit-remote-cluster

@jloleysens
Copy link
Contributor

jloleysens commented Mar 23, 2020

opened #60902

Nice catch btw!

@jloleysens
Copy link
Contributor

jloleysens commented Mar 23, 2020

@alisonelizabeth Running with yarn es snapshot with your code fix the issue I was seeing!

Happy for this to be merged 👍

@alisonelizabeth alisonelizabeth merged commit 8572e3f into elastic:master Mar 23, 2020
@alisonelizabeth alisonelizabeth deleted the bugfix/remote_clusters/api_changes branch March 23, 2020 14:42
@alisonelizabeth alisonelizabeth mentioned this pull request Mar 23, 2020
Merged
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Mar 23, 2020
@alisonelizabeth
[Remote clustersadopt changes to remote info API (elastic#60795)
b76dc4e
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 23, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
4122b2e 
* master:
  [Remote clustersadopt changes to remote info API (elastic#60795)
  Only run xpack siem cypress in PRs when there are siem changes (elastic#60661)
  [CI] Add error steps and help links to PR comments (elastic#60772)
  skip flaky functional test (elastic#60898)
  [Alerting] Fixes mistake in empty list assertion (elastic#60896)
  a11y tests for login and logout (elastic#60799)
  removed boom errors from AlertNavigationRegistry (elastic#60887)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 23, 2020
@gmmorris
Merge branch 'master' into alerting/details-cleanup
5897883 
* master: (26 commits)
  [Alerting] Fixes flaky test in Alert Instances Details page (elastic#60893)
  cleanup visualizations api (elastic#59958)
  Inline timezoneProvider function, remove ui/vis/lib/timezone  (elastic#60475)
  [SIEM] Adds 'Open one signal' Cypress test (elastic#60484)
  [UA] Upgrade assistant migration meta data can become stale (elastic#60789)
  [Metrics Alerts] Remove metric field from doc count on backend (elastic#60679)
  [Uptime] Skip failing location test temporarily (elastic#60938)
  [ML] Disabling datafeed editing when job is running (elastic#60751)
  Adding `authc.invalidateAPIKeyAsInternalUser` (elastic#60717)
  [SIEM] Add license check to ML Rule form (elastic#60691)
  Adding `authc.grantAPIKeyAsInternalUser`  (elastic#60423)
  Support Histogram Data Type (elastic#59387)
  [Upgrade Assistant] Fix edge case where reindex op can falsely be seen as stale (elastic#60770)
  [SIEM] [Cases] Update case icons (elastic#60812)
  [TSVB] Fix percentiles band mode (elastic#60741)
  Fix formatter on range aggregation (elastic#58651)
  Goodbye, legacy data plugin 👋 (elastic#60449)
  [Metrics UI] Alerting for metrics explorer and inventory (elastic#58779)
  [Remote clustersadopt changes to remote info API (elastic#60795)
  Only run xpack siem cypress in PRs when there are siem changes (elastic#60661)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 24, 2020
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 24, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 24, 2020
alisonelizabeth added a commit that referenced this pull request Mar 24, 2020
@alisonelizabeth
[Remote clusters] Adopt changes to remote info API (#60795) (#60912)
6ed72d9
@sebgl sebgl mentioned this pull request May 13, 2020
Closed
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 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alisonelizabeth @elasticmachine @jloleysens @kibanamachine