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

[JPMS Support] Refactor :libs module bootstrap package to eliminate top level split packages #17117

Merged
merged 1 commit into from
Jan 25, 2025

Conversation

prudhvigodithi
Copy link
Member

@prudhvigodithi prudhvigodithi commented Jan 24, 2025

Description

For Phase 0 - JPMS Support, refactor :libs module bootstrap package to eliminate top level split packages.

The :libs:opensearch-common and :server module both has org.opensearch.bootstrap. Updated the :libs org.opensearch.bootstrap to org.opensearch.common.bootstrap.

Related Issues

Part of #8110

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

@prudhvigodithi
Copy link
Member Author

@reta @andrross can you please check and provide your thoughts ?
Thanks
@getsaurabh02 @cwperks

Copy link
Contributor

❌ Gradle check result for 6c17381: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 70b105a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 4047be3: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@andrross
Copy link
Member

From #8110:

o.o.bootstrap - refactor to o.o.bootstrap in :libs:opensearch-common to new o.o.common.bootstrap package

Why did you move the classes in :server as opposed to moving the 2 classes in the :libs:opensearch-common to o.o.common.bootstrap like was suggested in #8110? It seems like it would be nice to have everything in the common lib to be under the o.o.common namespace.

Copy link
Contributor

❌ Gradle check result for 7c38379: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for faa0ee7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@prudhvigodithi
Copy link
Member Author

Why did you move the classes in :server as opposed to moving the 2 classes

Initially I thought of parking bootstrap, cli and client under server, but its messing up lot of things, had an offline discussion with @reta and saw lot of tests failing.

server
├── cli
├── client
└── bootstrap

I have updated this PR back to o.o.common.bootstrap.

Copy link
Contributor

❌ Gradle check result for bb5c763: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@prudhvigodithi prudhvigodithi changed the title [JPMS Support] Refactor :server module bootstrap package to eliminate top level split packages [JPMS Support] Refactor :libs module bootstrap package to eliminate top level split packages Jan 25, 2025
Copy link
Contributor

❌ Gradle check result for 90a1d17: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@reta
Copy link
Collaborator

reta commented Jan 25, 2025

I think it looks better this way @prudhvigodithi , thank you!

Copy link
Contributor

❕ Gradle check result for 90a1d17: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.repositories.azure.AzureBlobStoreRepositoryTests.testDeleteBlobs

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Jan 25, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.36%. Comparing base (931c1aa) to head (31fb88c).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...a/org/opensearch/gradle/precommit/JarHellTask.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17117      +/-   ##
============================================
+ Coverage     72.30%   72.36%   +0.05%     
- Complexity    65482    65486       +4     
============================================
  Files          5309     5309              
  Lines        304324   304347      +23     
  Branches      44132    44141       +9     
============================================
+ Hits         220056   220241     +185     
+ Misses        66259    66081     -178     
- Partials      18009    18025      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

❕ Gradle check result for 31fb88c: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.cluster.MinimumClusterManagerNodesIT.testThreeNodesNoClusterManagerBlock

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@reta reta merged commit 46f852a into opensearch-project:main Jan 25, 2025
29 of 30 checks passed
expani pushed a commit to expani/OpenSearch that referenced this pull request Jan 27, 2025
etolbakov pushed a commit to etolbakov/OpenSearch that referenced this pull request Jan 29, 2025
Signed-off-by: Prudhvi Godithi <[email protected]>
Signed-off-by: Eugene Tolbakov <[email protected]>
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.

3 participants