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

Use common-utils 1.1 branch for OpenSearch 1.1 #430

Merged
merged 2 commits into from
Sep 10, 2021
Merged

Use common-utils 1.1 branch for OpenSearch 1.1 #430

merged 2 commits into from
Sep 10, 2021

Conversation

adityaj1107
Copy link
Contributor

@adityaj1107 adityaj1107 commented Sep 9, 2021

Description

Change to use the 1.1 branch of Common Utils in the 1.1 manifest file.

Issues Resolved

opensearch-project/common-utils#64

PR (opensearch-project/common-utils#65) with all the commits cherry picked from the main branch. We can keep this PR as an artifact.

Check List

  • 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.

@adityaj1107 adityaj1107 marked this pull request as draft September 9, 2021 20:43
skkosuri-amzn
skkosuri-amzn previously approved these changes Sep 9, 2021
@adityaj1107 adityaj1107 marked this pull request as ready for review September 9, 2021 23:05
@adityaj1107
Copy link
Contributor Author

PR Merged: opensearch-project/common-utils#67

VachaShah
VachaShah previously approved these changes Sep 9, 2021
@adityaj1107
Copy link
Contributor Author

adityaj1107 commented Sep 9, 2021

@VachaShah If possible, Can you also provide me with more context on the failure so that I can double check and be completely sure, if the build failure is fully addressed by this opensearch-project/common-utils#67 PR.

@VachaShah
Copy link
Contributor

VachaShah commented Sep 10, 2021

@aditjind I think #410 was the issue. May be @peternied can elaborate here?

@peternied
Copy link
Member

I've updated the repro steps on #410, copied the reproduction steps into this PR for validation

To Reproduce

  1. Flush maven local with rm -rf ~/.m2/
  2. ./bundle-workflow/build.sh manifests/opensearch-1.1.0.yml --component common-utils --snapshot
  3. ls .m2/repository/org/opensearch/common-utils/
  4. Verify that the directory that is created is named X.X.X.X-SNAPSHOT and not X.X.X.X

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

This needs to wait on opensearch-project/common-utils#65 to be before it can be merged

@dblock
Copy link
Member

dblock commented Sep 10, 2021

I'd like to wait for #414 to get merged too so we can verify that this properly fails rn.

@dblock dblock dismissed stale reviews from VachaShah and skkosuri-amzn via e50636f September 10, 2021 17:53
@dblock
Copy link
Member

dblock commented Sep 10, 2021

Well, CI did catch a bug!

opensearch-project/common-utils#69 should fix it

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

CI is managing final approve now, great work @dblock

@dblock
Copy link
Member

dblock commented Sep 10, 2021

I re-ran the checks after opensearch-project/common-utils#69 and we're good to go.

@dblock dblock merged commit e4b6348 into opensearch-project:main Sep 10, 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