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

Refactored validate-refs to use new OpenSearch Serverless and registry-common library #906

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

al-niessner
Copy link
Contributor

@al-niessner al-niessner commented May 23, 2024

🗒️ Summary

Use registry-common for access to opensearch via Java SDK v2

⚙️ Test Data and/or Report

Had to remove the automated RI checks that did not seem to work anyway (they were causing errors but ignored by checkers) because need a mock for registry-common ConnectionFactory and RestClient. Loaded a DB by hand using ref data repository then ran RI. It ran to completion with no unexpected errors.

♻️ Related Issues

Closes #895
Depends on NASA-PDS/registry-common#57

@al-niessner al-niessner requested a review from a team as a code owner May 23, 2024 16:18
@al-niessner al-niessner marked this pull request as draft May 23, 2024 16:18
@al-niessner al-niessner self-assigned this May 23, 2024
@al-niessner
Copy link
Contributor Author

@jasonmlkang @tloubrieu-jpl

Stuck at a bug that requires validate pom include jakarta. If we are moving validate-refs to harvest (my recommendation because they share a large amount of code via config) then this would be the time to do it.

@jordanpadams
Copy link
Member

@al-niessner let's maybe move forward with this adding this JAR, and then we can create a ticket down the road to refactor, as needed. I like this validate-refs being in the same package as validate because users don't want to download or care to download 2 packages to do "validation". But we can climb that tree when we get to it.

@jordanpadams
Copy link
Member

@al-niessner also looks like the tests are failing here.

@al-niessner
Copy link
Contributor Author

@jordanpadams

This cannot build until we update registry-common and harvest. Even if it is complete, it cannot work here or in the wild until those repos are updated. Currently, it only works in my deve environment.

@jordanpadams
Copy link
Member

@al-niessner I think we have registry-common merged and some data should be in the AWS OpenSearch Serverless. Do we want to get back onto this task?

@al-niessner
Copy link
Contributor Author

Sure. Still stuck with the big question of do we move reference checker to harvest since it shares the configuration or make validate artifact dependent on harvest artifact. I still suggest moving reference checker to harvest since it has zero dependencies on validate.

I understand the grouping issue. I suggest solving that with a a pseudo artifact that installs many tools in some grouping rather than actually trying to group stuff by use rather than software dependency.

Al Niessner added 3 commits August 6, 2024 09:37
Moved to using registry-common and harvest for the tools they provide. harvest provides the configuration reader while the registry-common provides the interface to any opensearch.

Updated the logic to use the new tools which made the unit tests invalid. Removed all of the unit test code for RI since it is no longer applicable.

The logic of RI has remained. What has changed is how the interface to opensearch is used.
@al-niessner al-niessner marked this pull request as ready for review August 8, 2024 15:49
@jordanpadams jordanpadams changed the title 895: refactored for Java SDK v2 Refactored validate-refs to use new OpenSearch Serverless and registry-common Aug 9, 2024
@jordanpadams jordanpadams changed the title Refactored validate-refs to use new OpenSearch Serverless and registry-common Refactored validate-refs to use new OpenSearch Serverless and registry-common library Aug 9, 2024
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.

Switchover to using registry-common library to communicate with OpenSearch Serverless Registry
2 participants