Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport] [2.x] Update to Gradle 8.5 (#1131) #1132

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

reta
Copy link
Contributor

@reta reta commented Jan 17, 2024

(cherry picked from commit 7c0ce4c)

Description

Backport of #1131 to 2.x

Issues Resolved

Part of opensearch-project/OpenSearch#10334

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Andriy Redko <[email protected]>
(cherry picked from commit 7c0ce4c)
Signed-off-by: Andriy Redko <[email protected]>
@@ -265,7 +257,10 @@ public void getLatestDataTime_returnExpectedToListener() {
return null;
}).when(client).search(eq(searchRequest), any(ActionListener.class));

when(ParseUtils.getLatestDataTime(eq(searchResponse))).thenReturn(Optional.of(epochTime));
InternalMax maxAgg = new InternalMax(CommonName.AGG_NAME_MAX_TIME, epochTime, DocValueFormat.RAW, emptyMap());
Copy link
Member

Choose a reason for hiding this comment

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

just wondering why does there need to be this new code added here

Copy link
Contributor Author

@reta reta Jan 17, 2024

Choose a reason for hiding this comment

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

This is part of phasing out PowerMock , the reference is #985 (which was never backported sadly), thanks @amitgalitz for looking

Copy link
Member

Choose a reason for hiding this comment

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

I see, thank you for making this change!

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 77 lines in your changes are missing coverage. Please review.

Comparison is base (871b1dd) 79.84% compared to head (c914d10) 79.78%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #1132      +/-   ##
============================================
- Coverage     79.84%   79.78%   -0.06%     
+ Complexity     4366     4365       -1     
============================================
  Files           309      309              
  Lines         18207    18170      -37     
  Branches       1909     1909              
============================================
- Hits          14537    14497      -40     
+ Misses         2765     2763       -2     
- Partials        905      910       +5     
Flag Coverage Δ
plugin 79.78% <57.45%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ensearch/ad/client/AnomalyDetectionNodeClient.java 100.00% <100.00%> (ø)
.../opensearch/ad/cluster/ADClusterEventListener.java 87.50% <100.00%> (+1.38%) ⬆️
...main/java/org/opensearch/ad/cluster/DailyCron.java 100.00% <100.00%> (ø)
...va/org/opensearch/ad/feature/SearchFeatureDao.java 86.68% <100.00%> (ø)
...n/java/org/opensearch/ad/ml/EntityColdStarter.java 82.07% <100.00%> (-0.08%) ⬇️
.../handler/AbstractAnomalyDetectorActionHandler.java 61.99% <100.00%> (+0.09%) ⬆️
...arch/ad/transport/handler/AnomalyIndexHandler.java 76.38% <100.00%> (-0.64%) ⬇️
...ain/java/org/opensearch/ad/util/ExceptionUtil.java 72.09% <100.00%> (+1.63%) ⬆️
...ter/diskcleanup/ModelCheckpointIndexRetention.java 83.33% <75.00%> (+0.40%) ⬆️
...va/org/opensearch/ad/AnomalyDetectorJobRunner.java 80.30% <75.00%> (-0.20%) ⬇️
... and 14 more

... and 3 files with indirect coverage changes

amitgalitz
amitgalitz previously approved these changes Jan 17, 2024
@jackiehanyang
Copy link
Collaborator

Seeing this jackson-core error which is causing 2.x branch build failure. Should we force jackson-core version to be 2.16.1?
Reference: https://github.com/opensearch-project/flow-framework/blob/main/build.gradle#L172

@reta
Copy link
Contributor Author

reta commented Jan 22, 2024

Seeing this jackson-core error which is causing 2.x branch build failure. Should we force jackson-core version to be 2.16.1?

Do you use explicit version ? Or it is transitive dependency (sorry, I didn't check but I will if you don't know).

@owaiskazi19
Copy link
Member

owaiskazi19 commented Jan 22, 2024

Seeing this jackson-core error which is causing 2.x branch build failure. Should we force jackson-core version to be 2.16.1?

Do you use explicit version ? Or it is transitive dependency (sorry, I didn't check but I will if you don't know).

Looks like it's coming from core as there's no occurrence of 2.16.1 in 2.x of AD . I tried to get the tree on main branch and got the idea that it's a transitive dependency from core

+--- org.opensearch.test:framework:3.0.0-SNAPSHOT
|    +--- org.opensearch.client:opensearch-rest-client:3.0.0-SNAPSHOT (*)
|    +--- org.opensearch.client:opensearch-rest-client-sniffer:3.0.0-SNAPSHOT
|    |    +--- org.opensearch.client:opensearch-rest-client:3.0.0-SNAPSHOT (*)
|    |    +--- org.apache.httpcomponents.client5:httpclient5:5.2.1
|    |    +--- org.apache.httpcomponents.core5:httpcore5:5.2.2
|    |    +--- commons-codec:commons-codec:1.15
|    |    +--- commons-logging:commons-logging:1.2
|    |    \--- com.fasterxml.jackson.core:jackson-core:2.16.0

Note: The above is on the main branch.

@jackiehanyang
Copy link
Collaborator

jackiehanyang commented Jan 22, 2024

Seeing this jackson-core error which is causing 2.x branch build failure. Should we force jackson-core version to be 2.16.1?

Do you use explicit version ? Or it is transitive dependency (sorry, I didn't check but I will if you don't know).

nvm, we can take care of the jackson-core version in a separate pr since it's not causing build failure in this pr build. We just need to verify if it's a flaky test that is causing this build to fail - https://github.com/opensearch-project/anomaly-detection/actions/runs/7559131041/job/20750086870?pr=1132. After that we can go ahead merging this pr :)

Or you can just change the jackson-core version here to 2.16.1 to align with core jackson-core version that's defined here - https://github.com/opensearch-project/OpenSearch/blob/2.x/buildSrc/version.properties#L10-L11

@reta
Copy link
Contributor Author

reta commented Jan 23, 2024

Or you can just change the jackson-core version here to 2.16.1 to align with core jackson-core version that's defined here - https://github.com/opensearch-project/OpenSearch/blob/2.x/buildSrc/version.properties#L10-L11

So there is explicit version management, makes sense to change it here. I will update the pull request shortly, thanks @owaiskazi19 @jackiehanyang

Signed-off-by: Andriy Redko <[email protected]>
@reta
Copy link
Contributor Author

reta commented Jan 23, 2024

Checks are super unstable folks :(

@owaiskazi19
Copy link
Member

owaiskazi19 commented Jan 23, 2024

@reta do we need to make the dependency upgrades on main first and then backport to 2.x? Otherwise, there will be discrepancies between versions.

@reta
Copy link
Contributor Author

reta commented Jan 23, 2024

@reta do we need to make the dependency upgrades on main first and then backport to 2.x? Otherwise, there will be discrepancies between versions.

Ah, I will do the PR in a sec for main, thanks @owaiskazi19

@reta reta mentioned this pull request Jan 23, 2024
@owaiskazi19 owaiskazi19 merged commit 72b7d53 into opensearch-project:2.x Jan 24, 2024
19 of 23 checks passed
@owaiskazi19 owaiskazi19 added the v2.12.0 Issues targeting release v2.12.0 label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.12.0 Issues targeting release v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants