-
Notifications
You must be signed in to change notification settings - Fork 283
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
[RPM M1] Add rpm assemble/bundle code for opensearch using rpmbuild #1726
[RPM M1] Add rpm assemble/bundle code for opensearch using rpmbuild #1726
Conversation
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1726 +/- ##
============================================
- Coverage 94.76% 94.59% -0.17%
- Complexity 14 17 +3
============================================
Files 168 170 +2
Lines 3512 3592 +80
Branches 26 26
============================================
+ Hits 3328 3398 +70
- Misses 181 191 +10
Partials 3 3 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Hi @dblock please review the code and I will add test cases later. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs passing tests and documentation updates.
…ings Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cd
worries me. Can we merge without that?
@spotrh would you mind checking the spec file in this PR? Thanks. Note: as of now we are using bundled JDK per #1682 (comment) And we are also thinking about using Adopt per community suggestions: opensearch-project/OpenSearch#2312 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Block this PR for now until Spot review the spec files.
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good to me. I don't understand the tests well enough here to be sure, but I don't think there is any CI integration to confirm that the RPM was properly generated (or generated at all). Not a blocker for this, but probably something you want to do.
Yes @spotrh we have plans to add tests and verifications here: #27 Thanks. |
@spotrh has approved the PR now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Block this one from merging since we haven't generated the artifacts candidates for 1.3.0 release.
1.3.0 is released we can resume this now.
Description
Add rpm assemble/bundle code for opensearch using rpmbuild.
Adding rpm specific block so that cpio is used during extraction, and rpmbuild is used during build.
A specific opensearch spec file is provided, dashboards one will be committed in another PR.
Issues Resolved
Part of #1545
Check List
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.