Support for Elasticsearch 6.0.0 #20

gajicvedrana · 2018-01-19T11:10:32Z

Refs #19

Add ES5 string to version number for local builds.

- Change default nodeClient setting to false; - Disabled unit tests for node client. Elastic recommend using a local coordinating client in conjunction with the TransportClient instead of NodeClient.

…asticsearch into elasticsearch-5.0

# Conflicts: # README.md # pom.xml

# Conflicts: # README.md # pom.xml # src/main/java/io/dropwizard/elasticsearch/util/TransportAddressHelper.java # src/test/java/io/dropwizard/elasticsearch/util/TransportAddressHelperTest.java

coveralls · 2018-01-19T11:11:59Z

Coverage decreased (-7.2%) to 72.131% when pulling 2ab0f40 on engagetech:elasticsearch-6.0.0 into 4cc065c on dropwizard:master.

joschi

@gajicvedrana Thanks for your contribution! 😃

There are a few things we have to address before we can merge this PR.

joschi · 2018-01-19T12:02:30Z

README.md

@@ -43,7 +43,7 @@ Configuration

 The following configuration settings are supported by `EsConfiguration`:

-* `nodeClient`: When `true`, `ManagedEsClient` will create a `NodeClient`, otherwise a `TransportClient`; default: `true`
+* `nodeClient`: **DEPRECATED** Will throw an exception if `true`. Default: `false`


Why keep this setting at all when the node client isn't supported any more?

joschi · 2018-01-19T12:03:14Z

README.md

@@ -59,6 +59,17 @@ The order of precedence is: `nodeClient`/`servers`/`clusterName` > `settings` >
 any setting in `settingsFile` can be overwritten with `settings` which in turn get overwritten by the specific settings
 like `clusterName`.

+### Notes for Elasticsearch 5.x


Elasticsearch 5.x or 6.x? 😉

joschi · 2018-01-19T12:03:38Z

pom.xml

@@ -8,7 +8,7 @@

    <groupId>io.dropwizard.modules</groupId>
    <artifactId>dropwizard-elasticsearch</artifactId>
-    <version>1.2.0-2-SNAPSHOT</version>
+    <version>1.2.0-2-ES6.0.0-SNAPSHOT</version>


Please restore the old version.

joschi · 2018-01-19T12:04:15Z

pom.xml

-        <elasticsearch.version>2.4.6</elasticsearch.version>
+        <elasticsearch.version>6.0.0</elasticsearch.version>
+        <!-- github server corresponds to entry in ~/.m2/settings.xml -->
+        <github.global.server>github</github.global.server>


I don't think that's property is necessary.

joschi · 2018-01-19T12:05:09Z

pom.xml

            <version>${elasticsearch.version}</version>
        </dependency>
+        <!-- Required for ES 5.x -->
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>


Instead of pulling in a second logging framework, this project should use the Log4j to SLF4J adapter.

joschi · 2018-01-19T12:06:05Z

src/main/java/io/dropwizard/elasticsearch/config/EsConfiguration.java

@@ -24,7 +24,7 @@
    private String clusterName = "elasticsearch";

    @JsonProperty
-    private boolean nodeClient = true;
+    private boolean nodeClient = false;


Since the node client isn't supported anymore, this setting should be removed.

Instead, we could add support for the Elasticsearch High Level REST Client:

https://www.elastic.co/guide/en/elasticsearch/client/java-rest/6.x/java-rest-high.html

https://www.elastic.co/blog/the-elasticsearch-java-high-level-rest-client-is-out

joschi · 2018-01-19T12:07:51Z

src/main/java/io/dropwizard/elasticsearch/managed/ManagedEsClient.java

-                    .settings(settings)
-                    .build();
-            this.client = this.node.client();
+            throw new UnsupportedOperationException("Node client is not allowed for Elasticsearch 6. Use a local coordinating node, and the transport client.");


Please remove that completely.

joschi · 2018-01-19T12:08:32Z

src/test/java/io/dropwizard/elasticsearch/managed/ManagedEsClientTest.java

-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;


Please don't use wildcard imports.

joschi · 2018-01-19T12:09:58Z

src/test/java/io/dropwizard/elasticsearch/managed/ManagedEsClientTest.java

@@ -131,13 +160,13 @@ public void transportClientShouldBeCreatedFromConfig() throws URISyntaxException
                transportClient.transportAddresses().get(2));
    }

-    @Test
+    @Test @Ignore


Why is this test ignored?

joschi · 2018-01-19T12:10:04Z

src/test/java/io/dropwizard/elasticsearch/managed/ManagedEsClientTest.java

@@ -148,13 +177,13 @@ public void managedClientShouldUseCustomElasticsearchConfig() throws URISyntaxEx
        assertEquals("19300-19400", nodeClient.settings().get("transport.tcp.port"));
    }

-    @Test
+    @Test @Ignore


Why is this test ignored?

mattflax · 2018-01-22T14:00:43Z

I have a version for ES 5.6 which has most of these changes in place. Do you have a preferred version numbering scheme? I doubt the ES6 version is likely to be compatible with ES5, but both are still in common use among clients I visit, so it would be good to have both available.

If you let me know, I'll create a separate PR for the ES5 version.

joschi · 2018-01-25T14:01:30Z

@mattflax Since Elasticsearch 5.x is already legacy, I'd rather not add support for it but jump straight to Elasticsearch 6.x.

joschi · 2018-03-16T11:15:22Z

@gajicvedrana Are you going to address the review comments or should I close this PR?

joschi · 2019-03-03T15:46:49Z

Closed due to inactivity.

mattflax and others added 19 commits December 12, 2016 13:55

Changes for ES 5.0

abad736

Add log4j2 config.

e9f15c4

Make log4j version explicit.

2e1212c

Add ES5 string to version number for local builds.

Disabled NodeClient completely.

7127bf4

- Change default nodeClient setting to false; - Disabled unit tests for node client. Elastic recommend using a local coordinating client in conjunction with the TransportClient instead of NodeClient.

Modify dependencies to work better(ish) with SLF4J.

aee0167

dropwizard#19: Update README.

e7c3a51

dropwizard#19: Update README, update config unit test.

1f61708

Merge branch 'elasticsearch-5.0' of github.com:mattflax/dropwizard-el…

3488c07

…asticsearch into elasticsearch-5.0

Update Javadocs.

80a208a

Fix tests.

a6c896c

Update documentation.

499df1e

Add details for pushing to github Maven repo.

57db7fc

Skip adding project version to repo path.

0057c54

Merge branch 'master' into elasticsearch-5.0

5df5e83

# Conflicts: # README.md # pom.xml

Update to Dropwizard 1.1.0, as per upstream.

17f8948

Merge remote-tracking branch 'upstream/master' into elasticsearch-5.0

234dceb

# Conflicts: # README.md # pom.xml

Update to ES 5.6.4.

c23f0b5

Added support for Elasticsearch 6.0.0

fa31389

Merge branch '1.2.0-2-ES6.0.0' into elasticsearch-6.0.0

2ab0f40

# Conflicts: # README.md # pom.xml # src/main/java/io/dropwizard/elasticsearch/util/TransportAddressHelper.java # src/test/java/io/dropwizard/elasticsearch/util/TransportAddressHelperTest.java

joschi suggested changes Jan 19, 2018

View reviewed changes

joschi closed this Mar 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Elasticsearch 6.0.0 #20

Support for Elasticsearch 6.0.0 #20

gajicvedrana commented Jan 19, 2018 •

edited by joschi

Loading

coveralls commented Jan 19, 2018

joschi left a comment

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

joschi Jan 19, 2018

mattflax commented Jan 22, 2018

joschi commented Jan 25, 2018

joschi commented Mar 16, 2018

joschi commented Mar 3, 2019

Support for Elasticsearch 6.0.0 #20

Support for Elasticsearch 6.0.0 #20

Conversation

gajicvedrana commented Jan 19, 2018 • edited by joschi Loading

coveralls commented Jan 19, 2018

joschi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattflax commented Jan 22, 2018

joschi commented Jan 25, 2018

joschi commented Mar 16, 2018

joschi commented Mar 3, 2019

gajicvedrana commented Jan 19, 2018 •

edited by joschi

Loading