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

Resolve compile issues from core changes and update CIs #1100

Merged
merged 9 commits into from
Aug 23, 2023

Conversation

lezzago
Copy link
Member

@lezzago lezzago commented Aug 21, 2023

Issue #, if available:
N/A

Description of changes:
Resolve compile issues from core changes that backported to 2.x from main and disable BWC tests for non-bwc CIs

Backports following PRs:
#1062, #1090, #1025

CheckList:

  • Commits are signed per the DCO using --signoff

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.

lezzago and others added 8 commits August 21, 2023 13:51
…#1062)

* Resolve core Xcontent refactor

Signed-off-by: Ashish Agrawal <[email protected]>

* Resolve core CircuitBreaker refactor

Signed-off-by: Ashish Agrawal <[email protected]>

* Resolve integ test issues with adding test dependency

Signed-off-by: Ashish Agrawal <[email protected]>

---------

Signed-off-by: Ashish Agrawal <[email protected]>
Signed-off-by: Ashish Agrawal <[email protected]>
Signed-off-by: Ashish Agrawal <[email protected]>
Signed-off-by: Ashish Agrawal <[email protected]>
@lezzago lezzago changed the title Resolve compile issues from core changes Resolve compile issues from core changes and update CIs Aug 22, 2023
@lezzago
Copy link
Member Author

lezzago commented Aug 22, 2023

Test are failing due to issues in MonitorDataSourcesIT. Will resolve the integ tests in a future PR.

Tests with failures:
 - org.opensearch.alerting.MonitorDataSourcesIT.test execute with custom alerts and finding index with bucket and doc monitor when doc monitor  is used in chained finding
 - org.opensearch.alerting.MonitorDataSourcesIT.classMethod
 - org.opensearch.alerting.MonitorRunnerServiceIT.classMethod
 - org.opensearch.alerting.alerts.AlertIndicesIT.classMethod
 - org.opensearch.alerting.resthandler.FindingsRestApiIT.test find Finding by tag
 - org.opensearch.alerting.util.clusterMetricsMonitorHelpers.CatIndicesWrappersIT.classMethod

@lezzago lezzago marked this pull request as ready for review August 22, 2023 23:15
@@ -44,7 +42,7 @@ jobs:
java-version: ${{ matrix.java }}
- name: Build and run with Gradle
working-directory: ${{ env.WORKING_DIR }}
run: ./gradlew build ${{ env.BUILD_ARGS }}
run: ./gradlew build ${{ env.BUILD_ARGS }} -x alertingBwcCluster#fullRestartClusterTask -x alertingBwcCluster#mixedClusterTask -x alertingBwcCluster#oldVersionClusterTask0 -x alertingBwcCluster#oldVersionClusterTask1 -x alertingBwcCluster#rollingUpgradeClusterTask -x alertingBwcCluster#twoThirdsUpgradedClusterTask
Copy link
Member

@bowenlan-amzn bowenlan-amzn Aug 22, 2023

Choose a reason for hiding this comment

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

I prefer to specify what to run ./gradlew assemble integTest instead of specify bunch of tests to not run

@lezzago
Copy link
Member Author

lezzago commented Aug 23, 2023

Test are failing due to issues in MonitorDataSourcesIT. Will resolve the integ tests in a future PR.

Tests with failures:
 - org.opensearch.alerting.MonitorDataSourcesIT.test execute with custom alerts and finding index with bucket and doc monitor when doc monitor  is used in chained finding
 - org.opensearch.alerting.MonitorDataSourcesIT.classMethod
 - org.opensearch.alerting.MonitorRunnerServiceIT.classMethod
 - org.opensearch.alerting.alerts.AlertIndicesIT.classMethod
 - org.opensearch.alerting.resthandler.FindingsRestApiIT.test find Finding by tag
 - org.opensearch.alerting.util.clusterMetricsMonitorHelpers.CatIndicesWrappersIT.classMethod

These issues have been resolved

@@ -44,7 +42,7 @@ jobs:
java-version: ${{ matrix.java }}
- name: Build and run with Gradle
working-directory: ${{ env.WORKING_DIR }}
run: ./gradlew build ${{ env.BUILD_ARGS }}
run: ./gradlew assemble integTest ${{ env.BUILD_ARGS }}
Copy link
Member

Choose a reason for hiding this comment

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

is this change present in main?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had used the remove argument before, so I will update main with this change as well.

* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.alerting.action
Copy link
Member

Choose a reason for hiding this comment

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

NIT: This test class is present in common utils along with the request class. We shouldn't have this here as these tests would break if anything is changed in common utils

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh this test was present in Alerting main here

@lezzago lezzago merged commit f5fe1dc into opensearch-project:2.x Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants