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

Build OpenSearch in CD workflow in order to build security plugin #1364

Merged
merged 3 commits into from
Aug 7, 2021

Conversation

cliu123
Copy link
Member

@cliu123 cliu123 commented Jul 30, 2021

opensearch-security pull request intake form

Please provide as much details as possible to get feedback/acceptance on your PR quickly

  1. Category: (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

  2. Github Issue # or road-map entry, if available:

  3. Description of changes:

  4. Why these changes are required?

  5. What is the old behavior before changes and new behavior after changes? (Please add any example/logs/screen-shot if available)

  6. Testing done: (Please provide details of testing done: Unit testing, integration testing and manual testing)

  7. TO-DOs, if any: (Please describe pending items and provide Github issues# for each of them)

  8. Is it backport from main branch? (If yes, please add backport PR # and commits #)

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

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.

@cliu123 cliu123 requested a review from a team July 30, 2021 20:05
@cliu123 cliu123 force-pushed the build_opensearch_in_cd branch from 26a1ea1 to bfc1d2b Compare July 30, 2021 20:16
@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2021

Codecov Report

Merging #1364 (bfc1d2b) into main (960d420) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1364   +/-   ##
=========================================
  Coverage     64.78%   64.78%           
- Complexity     3204     3205    +1     
=========================================
  Files           247      247           
  Lines         17252    17252           
  Branches       3053     3053           
=========================================
+ Hits          11176    11177    +1     
+ Misses         4526     4525    -1     
  Partials       1550     1550           
Impacted Files Coverage Δ
.../dlic/auth/ldap2/LDAPConnectionFactoryFactory.java 57.25% <0.00%> (+0.76%) ⬆️

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 960d420...bfc1d2b. Read the comment docs.

.github/workflows/cd.yml Outdated Show resolved Hide resolved
.github/workflows/cd.yml Outdated Show resolved Hide resolved
path: OpenSearch
ref: '1.0'

- name: Build OpenSearch
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this step be after Cache Maven packages step?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't OpenSearch artifact built be cached?
If moving Build OpenSearch step and putting it after Cache Maven packages step, both of Build OpenSearch and Build steps will be after Cache Maven packages step. Shouldn't the build outputs be cached?

Copy link
Contributor

@vrozov vrozov Aug 3, 2021

Choose a reason for hiding this comment

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

What is the point of caching OpenSearch maven packages when they are rebuilt anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. In this case, only downloaded dependencies should be cached. There is no point of caching build outputs.
Moved. Thanks!

@vrozov vrozov merged commit 9f04635 into opensearch-project:main Aug 7, 2021
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