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

Support ES version 7.7.0 #58

Merged
merged 20 commits into from
May 20, 2020
Merged

Support ES version 7.7.0 #58

merged 20 commits into from
May 20, 2020

Conversation

ftianli-amzn
Copy link

Issue #, if available:
#57

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ftianli-amzn
Copy link
Author

Thanks zengyan-amazon and qreshi for the tricky fix for the testClusters issue.

// The job-scheduler zip is added explicitly above but the sample-extension-plugin is added implicitly at some time during evaluation.
// Will need to do a deep dive to find out exactly what task adds the sample-extension-plugin and add job-scheduler there but a temporary hack is to
// reorder the plugins list after evaluation but prior to task execution when the plugins are installed.
afterEvaluate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this a bit more reliable by checking if the sample plugin comes before job scheduler plugin and if so then removing/adding? Otherwise if it changes again next version this will break again.

Copy link
Author

Choose a reason for hiding this comment

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

I think I can make this change in this version. There is no doubt it will make the plugin installation order much more reliable, but is not that important. The whole afterEvaluate block is a temporary workaround, and I believe the plugins should not be installed in this way forever, so the afterEvaluate block won't last long. An ideal case is in next version, the testClusters Gradle plugin get changed and this problem get fixed, and this gradle file get restored.

Copy link
Author

Choose a reason for hiding this comment

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

Looking forward to hear opinions from other reviewers as well.

@bowenlan-amzn bowenlan-amzn self-requested a review May 20, 2020 00:30
Copy link
Contributor

@bowenlan-amzn bowenlan-amzn left a comment

Choose a reason for hiding this comment

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

LGTM

@ftianli-amzn ftianli-amzn merged commit d16b08b into opendistro-for-elasticsearch:master May 20, 2020
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.

3 participants