Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove unused elasticsearch.preserverHost setting #78608

Merged
merged 2 commits into from
Sep 30, 2020
Merged

Remove unused elasticsearch.preserverHost setting #78608

merged 2 commits into from
Sep 30, 2020

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Sep 28, 2020 •
edited
Loading

Summary

Fixes #17242

I've also removed elasticsearch.startupTimeout which was unused. Since Kibana will keep on trying to connect to Elasticsearch until it manages to connect, I don't think there's any value in having such a setting.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 28, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

page load bundle size

id value diff baseline
core 1.1MB -17.0B 1.1MB

History

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

@rudolf rudolf marked this pull request as ready for review September 29, 2020 07:36
@rudolf rudolf requested review from a team as code owners September 29, 2020 07:36
@rudolf rudolf requested a review from a team September 29, 2020 07:36
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

spalger
spalger approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM, those are some truly ancient settings.

pgayvallet
pgayvallet approved these changes Sep 29, 2020
View reviewed changes
src/core/server/config/deprecation/core_deprecations.ts
Comment on lines +139 to +140
unusedFromRoot('elasticsearch.preserveHost'),
unusedFromRoot('elasticsearch.startupTimeout'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to have it in the ES config deprecations instead:

kibana/src/core/server/elasticsearch/elasticsearch_config.ts

Line 126 in bf04235

const deprecations: ConfigDeprecationProvider = () => [

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering about that, but my thinking was that deprecations are essential legacy or technical debt. They once belonged to a specific domain, but now they're just there to tell users they're doing something wrong until we make these config values invalid in 8.x. Instead of these distracting from the other domain code, having them isolated to one place is similar to how we isolated the legacy folder.

These other ES config deprecations require access to the ES config so they can't be centralised this way.

chrisronline
chrisronline approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM

@rudolf rudolf merged commit f8d796a into elastic:master Sep 30, 2020
@rudolf rudolf deleted the remove-preservehost-config branch September 30, 2020 08:39
@rudolf rudolf mentioned this pull request Sep 30, 2020
Merged
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 30, 2020
@phillipb
Merge branch 'master' of github.com:elastic/kibana into override-anom…
a3da4bc 
…aly-detection-partition-field

* 'master' of github.com:elastic/kibana: (37 commits)
  [UiActions] Don't throw an error if there are no compatible actions to execute (elastic#78917)
  [Observability] Kibana nav when docked overlaps the content of the pages. (elastic#78593)
  Invalid `searchSourceJSON` causes saved object migration to fail (elastic#78535)
  update vega version (elastic#78390)
  Fix warning text doesn't get displayed on filters with custom filter name (elastic#78617)
  [ILM] Data tier notices should reflect tier preferences (elastic#78398)
  [APM] Service Map: `Not Defined` option doesn't work properly (elastic#77757)
  Improve invalid field error message at search.aggs.param_types.field (elastic#78587)
  Remove isDeprecated flag on visType (elastic#78820)
  Remove unused elasticsearch.preserverHost setting (elastic#78608)
  Fix condition and adjust tests (elastic#78898)
  [UX] Add percentile selector (elastic#78562)
  [ML] Replace use of rest_total_hits_as_int with track_total_hits (elastic#78423)
  expression service docs (elastic#78774)
  [Functional] Wait for the page to load and then click the new vis button (elastic#78725)
  [TSVB] No data in visualizations with annotations (elastic#78794)
  [kbn/ui-shared-deps] track asset sizes (elastic#78718)
  delete target before building (elastic#78665)
  Move indexPattern.popularizeField into discover (elastic#77668)
  [Security Solution][Resolver]Add backdrop to pills (elastic#78625)
  ...
rudolf added a commit that referenced this pull request Oct 1, 2020
@rudolf
Remove unused elasticsearch.preserverHost setting (#78608) (#78921)
e51a87a 
* Remove unused elasticsearch.preserverHost setting

* deprecate and remove elasticsearch.startupTimeout config
# Conflicts:
#	src/core/server/config/deprecation/core_deprecations.ts
@TinaHeiligers TinaHeiligers mentioned this pull request Sep 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisronline chrisronline chrisronline approved these changes

@spalger spalger spalger approved these changes

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

elasticsearch.preserveHost setting
6 participants
@rudolf @kibanamachine @elasticmachine @chrisronline @spalger @pgayvallet