-
Notifications
You must be signed in to change notification settings - Fork 81
Support elastic search7 travis fix - support platforms. #98
base: master
Are you sure you want to change the base?
Conversation
…earch. This includes choosing the right URL for elastic 7 and the ability to install from tar.gz (besides zip)
…not maintaining exec permissions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added just two remarks on the code.
return false; | ||
} | ||
String esV = jsonNode.get("version").get("number").asText(); | ||
return Integer.parseInt(esV.substring(0,1)) >= 7; //if version is 7 and above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens with elasticsearch version above 9? Can we use split with '.' sign to use the structure of the version number?
|
||
EmbeddedElastic.Builder baseEmbeddedElastic() { | ||
return EmbeddedElastic.builder() | ||
.withElasticVersion("6.3.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use here an older elasticsearch version for the tests?
Applied both suggestions @bekihm , thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@pablotdl Had travis temporarily problems to run the build? |
Looks like a problem with an ES plugin |
Indeed, the discovery-file plugin is removed in 7.3, let's see if another one makes the trick. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that the alternative plugin has done the job 👍
Yep, hope it gets merged now. |
1 similar comment
Yep, hope it gets merged now. |
This PR is broken for
|
Added support for downloading the platform specific versions of elasticsearch listed at https://www.elastic.co/downloads/elasticsearch.
This should complement the work by @eynand and @bekihm. Let's see what travis says.