Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Elasticsearch 7 rest client #58

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

purplesword
Copy link

No description provided.

@joschi
Copy link
Member

joschi commented May 26, 2019

@purplesword Thanks for your contribution!

Could you please resolve the conflicts with the master branch and remove Gradle from the PR? We're not planning to change the build system from Maven to Gradle.

@coveralls
Copy link

coveralls commented May 26, 2019

Coverage Status

Coverage increased (+8.6%) to 87.923% when pulling 4ce4c2e on purplesword:elasticsearch_7_rest_client into e2088de on dropwizard:master.

.gitignore Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved

@JsonProperty
@NotNull
private Map<String, String> settings = Collections.emptyMap();
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

added a bunch more.. Not sure if did it properly.. Tests will be added soon.

Copy link
Author

Choose a reason for hiding this comment

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

I added more tests now the coverage is higher and 100% on health checks.

Could you comment more about what do you think is necessary to be covered in configuration?

I added most of them listed in the documentation (but feel a bit hard to write test unless using testcontainer). Also I sort of feel it's a bit too complicate, maybe it's better to cover some simple common configs and then let sophisticated users do what they want with the ManagedClient(client,sniffer) constructor with their own client and sniffer..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants