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

Update alerting with qualifier support in releases #366

Conversation

peterzhuamazon
Copy link
Member

Signed-off-by: Peter Zhu [email protected]

Issue #, if available:
#348

Description of changes:
Update alerting with qualifier support in releases

CheckList:
[x] 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.

Signed-off-by: Peter Zhu <[email protected]>
@peterzhuamazon
Copy link
Member Author

@lezzago seems like your test is failing I am not sure if it is compatible with 2.0.0-alpha1 as of yet. Can you guys take a look on this?

Thanks.

@lezzago
Copy link
Member

lezzago commented Mar 29, 2022

We will submit a new pr later to fix those test issues since there are more 2.0 breaking changes

@qreshi
Copy link
Contributor

qreshi commented Mar 29, 2022

This doesn't just look like test failures, it's failing to build with OpenSearch 2.0.0-alpha1. We should make the appropriate changes in this PR to fix compile issues, etc.

@peterzhuamazon If you'd like to avoid that, you can just make the build qualifier changes here but keep using 2.0.0 for OpenSearch instead of 2.0.0-alpha1. Then I can look into the errors of bumping to alpha1. I want to avoid breaking main.

@peterzhuamazon
Copy link
Member Author

After talking to alerting team I will keep 2.0.0-SNAPSHOT for the time being to merge this PR.

They will fix the compiling issues on alpha1 and add back.

Thanks.

@codecov-commenter
Copy link

Codecov Report

Merging #366 (c327add) into main (0c5090f) will decrease coverage by 0.13%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #366      +/-   ##
============================================
- Coverage     79.15%   79.01%   -0.14%     
  Complexity      268      268              
============================================
  Files           176      176              
  Lines          7286     7286              
  Branches       1016     1016              
============================================
- Hits           5767     5757      -10     
- Misses          994     1003       +9     
- Partials        525      526       +1     
Impacted Files Coverage Δ
...lin/org/opensearch/alerting/alerts/AlertIndices.kt 66.44% <0.00%> (-7.24%) ⬇️
...ing/model/destination/DestinationContextFactory.kt 78.57% <0.00%> (+3.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c5090f...c327add. Read the comment docs.

@peterzhuamazon
Copy link
Member Author

Merge this as the security plugin is not available yet, so security checks expected to fail here.

@peterzhuamazon peterzhuamazon merged commit 9528fb6 into opensearch-project:main Mar 29, 2022
@peterzhuamazon peterzhuamazon deleted the opensearch-2.0.0-qualifier branch March 29, 2022 22:21
Angie-Zhang pushed a commit to Angie-Zhang/alerting that referenced this pull request Jun 29, 2022
…t#366)

* Update alerting with qualifier support in releases

Signed-off-by: Peter Zhu <[email protected]>

* Move snapshot above

Signed-off-by: Peter Zhu <[email protected]>

* Change to 2.0.0-SNAPSHOT until alerting fixed to compile on 2.0.0-alpha1-SNAPSHOT

Signed-off-by: Peter Zhu <[email protected]>

* Support qualifier for docker file pulling

Signed-off-by: Peter Zhu <[email protected]>
Signed-off-by: Angie Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants