Skip to content

Conversation

@jmorph99
Copy link
Contributor

@jmorph99 jmorph99 commented Jun 6, 2017

Those plugins don't replace the discovery logic but rather only provide a custom unicast host provider for their respective platforms. in 5.1 we introduced the discovery.zen.hosts_provider setting to better reflect it. This PR removes BWC code in those plugins as it is not needed anymore

Fixes #24543

The EC2 Test did not need to be modified like the other two.
@nik9000
Copy link
Member

nik9000 commented Jun 6, 2017

Can you explain what you did in the PR's title? It is super useful for those of us that use our email to decide which PRs to review.

Copy link
Contributor Author

@jmorph99 jmorph99 left a comment

Choose a reason for hiding this comment

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

I fixed the plugins
used DISCOVERY_HOSTS_PROVIDER_SETTING
instead of DISCOVERY_TYPE_SETTING

removed backward compatibility support for DISCOVERY_TYPE.
Updated unit tests as well to use the new method.

@jmorph99
Copy link
Contributor Author

jmorph99 commented Jun 6, 2017 via email

@rjernst
Copy link
Member

rjernst commented Jun 7, 2017

@jmorph99 I think he means the issue title and the commit message. Right now it is "Issue #24543 Fix", which will be the first line of the commit message when this is merged. That title is not useful. Instead, I would expect something like "Remove discovery plugins legacy support for discovery.type".

@jmorph99
Copy link
Contributor Author

jmorph99 commented Jun 7, 2017 via email

@bleskes bleskes changed the title Issue #24543 Fix Remove discovery.type BWC layer from the EC2/Azure/GCE plugins Jun 7, 2017
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

Thx @jmorph99 for all the iterations. The code looks good! My only ask is that you add a note to the migration guide, here. I think something like adding a new section called GCE/Azure/EC2 discovery setting and note that discovery.type: ec2/azure/gce has been removed in favor of the discovery host provider variants.

jmorph99 added a commit to jmorph99/elasticsearch that referenced this pull request Jun 8, 2017
@jmorph99
Copy link
Contributor Author

jmorph99 commented Jun 8, 2017

Ok - I think I did it successfully.

jmorph99 added a commit to jmorph99/elasticsearch that referenced this pull request Jun 8, 2017
@jmorph99
Copy link
Contributor Author

Ever going to close this?

@bleskes
Copy link
Contributor

bleskes commented Jun 13, 2017

@jmorph99 I think you pushed your doc change to another branch of your repo and therefore they don't appear here. I suggest we just pull this PR as is and I can work on docs on as a follow up. Is that OK with you? Thanks for all the hard work. Git can be difficult.

@jmorph99
Copy link
Contributor Author

jmorph99 commented Jun 13, 2017 via email

@bleskes bleskes added the v6.0.0 label Jun 14, 2017
@bleskes bleskes merged commit c652b58 into elastic:master Jun 14, 2017
@bleskes
Copy link
Contributor

bleskes commented Jun 14, 2017

Thx @jmorph99 !

bleskes added a commit that referenced this pull request Jun 14, 2017
bleskes added a commit that referenced this pull request Jun 14, 2017
@jmorph99 jmorph99 deleted the jmorph99 branch June 14, 2017 14:52
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 14, 2017
* master: (27 commits)
  Refactor TransportShardBulkAction.executeUpdateRequest and add tests
  Make sure range queries are correctly profiled. (elastic#25108)
  Test: allow setting socket timeout for rest client (elastic#25221)
  Migration docs for elastic#25080 (elastic#25218)
  Remove `discovery.type` BWC layer from the EC2/Azure/GCE plugins elastic#25080
  When stopping via systemd only kill the JVM, not its control group (elastic#25195)
  Remove PrefixAnalyzer, because it is no longer used.
  Internal: Remove Strings.cleanPath (elastic#25209)
  Docs: Add note about which secure settings are valid (elastic#25212)
  Indices.rollover/10_basic should refresh to make the doc visible in lucene stats
  Port support for commercial GeoIP2 databases from Logstash. (elastic#24889)
  [DOCS] Add ML node to node.asciidoc (elastic#24495)
  expose simple pattern tokenizers (elastic#25159)
  Test: add setting to change request timeout for rest client (elastic#25201)
  Fix secure repository-hdfs tests on JDK 9
  Add target_field parameter to gsub, join, lowercase, sort, split, trim, uppercase (elastic#24133)
  Add Cross Cluster Search support for scroll searches (elastic#25094)
  Adapt skip version in rest-api-spec/test/indices.rollover/20_max_doc_condition.yml
  Rollover max docs should only count primaries (elastic#24977)
  Add remote cluster infrastructure to fetch discovery nodes. (elastic#25123)
  ...
jasontedor added a commit to glefloch/elasticsearch that referenced this pull request Jun 15, 2017
* master: (44 commits)
  Upgrade icu4j for the ICU analysis plugin to 59.1 (elastic#25243)
  move assertBusy to use CheckException (elastic#25246)
  Use SPI in High Level Rest Client to load XContent parsers (elastic#25098)
  [TEST] test that low level REST client leaves path untouched (elastic#25193)
  Speed up PK lookups at index time. (elastic#19856)
  [Docs] Fix documentation for percentiles bucket aggregation (elastic#25229)
  Upgrade to lucene-7.0.0-snapshot-92b1783. (elastic#25222)
  Build: Add master flag for disabling bwc tests (elastic#25230)
  Scripting: Rename SearchScript.needsScores to needs_score (elastic#25235)
  Support script context stateful factory in Painless. (elastic#25233)
  FastVectorHighlighter should not cache the field query globally (elastic#25197)
  Remove QUERY_AND_FETCH BWC for pre-5.3.0 nodes (elastic#25223)
  Add more missing AggregationBuilder getters (elastic#25198)
  Extract the snapshot/restore full cluster restart tests from the translog full cluster restart tests (elastic#25204)
  Refactor TransportShardBulkAction.executeUpdateRequest and add tests
  Make sure range queries are correctly profiled. (elastic#25108)
  Test: allow setting socket timeout for rest client (elastic#25221)
  Migration docs for elastic#25080 (elastic#25218)
  Remove `discovery.type` BWC layer from the EC2/Azure/GCE plugins elastic#25080
  When stopping via systemd only kill the JVM, not its control group (elastic#25195)
  ...
@clintongormley clintongormley added :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure and removed :Plugin Discovery Azure Classic labels Feb 13, 2018
tlrx pushed a commit that referenced this pull request Mar 12, 2018
The docs state that `_gce_` is recommended but the code sample states
that `_gce:hostname_` is recommended. This aligns the code sample with
the documentation. Also replace `type` with `zen.hosts_provider` as 
discovery.type was removed in #25080.
tlrx pushed a commit that referenced this pull request Mar 12, 2018
The docs state that `_gce_` is recommended but the code sample states
that `_gce:hostname_` is recommended. This aligns the code sample with
the documentation. Also replace `type` with `zen.hosts_provider` as 
discovery.type was removed in #25080.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>breaking :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants