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

Odfe 1.4.0 #43

Merged
merged 15 commits into from
Feb 20, 2020
Merged

Conversation

jngz-es
Copy link
Contributor

@jngz-es jngz-es commented Feb 13, 2020

#42

Build against ES 7.4.2.

@jngz-es jngz-es added the enhancement New feature or request label Feb 13, 2020
@jngz-es jngz-es self-assigned this Feb 13, 2020
@jngz-es jngz-es requested a review from ylwu-amzn February 13, 2020 23:30
isSnapshot = "true" == System.getProperty("build.snapshot", "true")
}

version = "${opendistroVersion}.0-alpha"
version = "${opendistroVersion}.0"
Copy link
Member

Choose a reason for hiding this comment

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

We are still doing an alpha release, not official release, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is for 7.4 release.

@@ -258,6 +258,9 @@ List<String> jacocoExclusions = [
'com.amazon.opendistroforelasticsearch.ad.transport.StopDetectorRequest',
'com.amazon.opendistroforelasticsearch.ad.transport.StopDetectorResponse',
'com.amazon.opendistroforelasticsearch.ad.transport.StopDetectorTransportAction',
'com.amazon.opendistroforelasticsearch.ad.transport.ADStatsAction',
Copy link
Member

Choose a reason for hiding this comment

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

Why do we add those exclusions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To speed up 7.4 release, I excluded them since UT coverage is below the threshold.

@@ -54,9 +57,7 @@ public ADStatsRequest getADStatsRequest() {
return request;
}

@Override
public void readFrom(StreamInput in) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this method since you have added a new constructor taking StreamInput as input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw there is still reference to this function, so I kept it.

@@ -79,9 +85,7 @@ public void clear() {
return statsToBeRetrieved;
}

@Override
public void readFrom(StreamInput in) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this method since you have added a new constructor taking StreamInput as input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still see a reference to this function. I don't want to touch too much code which is not related to build against 7.4.

Copy link
Contributor

@ylwu-amzn ylwu-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. Thanks for the change!

@jngz-es jngz-es merged commit 040b2f3 into opendistro-for-elasticsearch:development Feb 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants