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.

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

Remove DiscoveryPlugin#getDiscoveryTypes #38414

Merged

Conversation

DaveCTurner
Copy link
Contributor

With this change we no longer support pluggable discovery implementations. No
known implementations of DiscoveryPlugin actually override this method, so in
practice this should have no effect on the wider world. However, we were using
this rather extensively in tests to provide the test-zen discovery type. We
no longer need a separate discovery type for tests as we no longer need to
customise its behaviour.

Relates #38410

With this change we no longer support pluggable discovery implementations. No
known implementations of `DiscoveryPlugin` actually override this method, so in
practice this should have no effect on the wider world. However, we were using
this rather extensively in tests to provide the `test-zen` discovery type. We
no longer need a separate discovery type for tests as we no longer need to
customise its behaviour.

Relates elastic#38410
@DaveCTurner DaveCTurner added >enhancement >breaking v7.0.0 :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Feb 5, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@DaveCTurner
Copy link
Contributor Author

@elasticmachine please run elasticsearch-ci/default-distro

.put(TestZenDiscovery.USE_ZEN2.getKey(), false)
.put(TestZenDiscovery.USE_MOCK_PINGS.getKey(), false)) // Zen2 does not know about mock pings
.build();
.put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.ZEN_DISCOVERY_TYPE)).build();

private static Settings ZEN2_SETTINGS = Settings.builder()
Copy link
Contributor

Choose a reason for hiding this comment

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

should we rename the "zen2" discovery type to "default"? This means no need for the "zen" name anymore

@DaveCTurner
Copy link
Contributor Author

DaveCTurner commented Feb 5, 2019

@elasticmachine retest this please now that 766f7b5 is in 6.x.

DaveCTurner added a commit that referenced this pull request Feb 5, 2019
Marks `DiscoveryPlugin#getDiscoveryTypes` as deprecated since we intend to
remove the ability for plugins to provide their own discovery type in a future
version.

Relates #38414
@DaveCTurner
Copy link
Contributor Author

CI failure due to #37807 - @elasticmachine please run elasticsearch-ci/2.

Copy link
Contributor

@andrershov andrershov left a comment

Choose a reason for hiding this comment

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

Nice, LGTM2

@DaveCTurner
Copy link
Contributor Author

@elasticmachine please run elasticsearch-ci/packaging-sample

@DaveCTurner DaveCTurner merged commit f2dd5dd into elastic:master Feb 5, 2019
@DaveCTurner DaveCTurner deleted the 2019-02-05-remove-pluggable-discovery branch February 5, 2019 17:42
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 5, 2019
* master: (23 commits)
  Lift retention lease expiration to index shard (elastic#38380)
  Make Ccr recovery file chunk size configurable (elastic#38370)
  Prevent CCR recovery from missing documents (elastic#38237)
  re-enables awaitsfixed datemath tests (elastic#38376)
  Types removal fix FullClusterRestartIT warnings (elastic#38445)
  Make sure to reject mappings with type _doc when include_type_name is false. (elastic#38270)
  Updates the grok patterns to be consistent with logstash (elastic#27181)
  Ignore type-removal warnings in XPackRestTestHelper (elastic#38431)
  testHlrcFromXContent() should respect assertToXContentEquivalence() (elastic#38232)
  add basic REST test for geohash_grid (elastic#37996)
  Remove DiscoveryPlugin#getDiscoveryTypes (elastic#38414)
  Fix the clock resolution to millis in GetWatchResponseTests (elastic#38405)
  Throw AssertionError when no master (elastic#38432)
  `if_seq_no` and `if_primary_term` parameters aren't wired correctly in REST Client's CRUD API (elastic#38411)
  Enable CronEvalToolTest.testEnsureDateIsShownInRootLocale (elastic#38394)
  Fix failures in BulkProcessorIT#testGlobalParametersAndBulkProcessor. (elastic#38129)
  SQL: Implement CURRENT_DATE (elastic#38175)
  Mute testReadRequestsReturnLatestMappingVersion (elastic#38438)
  [ML] Report index unavailable instead of waiting for lazy node (elastic#38423)
  Update Rollup Caps to allow unknown fields (elastic#38339)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking >breaking-java :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >enhancement v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants