-
Notifications
You must be signed in to change notification settings - Fork 1
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
Baseline refactoring to support OpenSearch Serverless and OpenSearch Java JDK #47
Conversation
Created ConnectionFactory for building the HTTP connection that is currently being done in a more direct use class. The facade should allow cognito go out and get the signed URL without any other part of the code caring. The big question is will the signed URL really work as well as amazon promises.
Did the first round of hacking things apart and it all compiles again. Installed a facade for controlling the connection to search. If you either or both of you have some spare cycles, can you take a gander and see if it aligns with what you had in mind. I removed RegistryCfg then started fixing from there. There may be items that somehow still do rather direct connections but I think I found them all by removing RegistryCfg. Still... Anyway, I am just looking for a sanity check. Did you want me to change all of the elasticsearch dependencies to opensearch? I did it for registry way back when so have some experience at it. It will add a day or so but not much. |
Had to extend the ConnectionFactory just a bit to support ESClient authentication. In do so, it seems like creatng the ECClient may become the ConnectionFactory job as we move into cognito. For now, it sufficed to keep it where is. Probably need a common.connection.es, common.connection.os, common.connection.cognito to complete the facade an move code around some more. Really should junk es since it cannot live side-by-side with os (opensearch).
The first step of getting past cognito is working. The second step of retrieving a self signed URL is not working. Backing the changes up to this branch.
Moved to an XML file for registry connection information. Also added some support for various URL protocols to shorten the names (app). Internally, always use the jar: protcol because it is naturally supported by Java. Did an operation test then removed code as it is good for development but not regression.
Here are the changes for the registry connection. Right now I just have a localhost connection but will be adding cognito soon. It should give you a good idea through. For tools that current use --registry https://localhost:9200 it would not be --registry app:/connections/direct/localhost.xml. Here is what that URL now translates into: Should also extend the KnownRegistryConnections tool to extract it as well. Maybe I will do that next while I wait for @sjoshi-jpl When extracted, it could be edited and the used with --registry file:/path/to/localhost.xml |
Really broke the ES knowledge from out of the existing Java code. Left some because it is as good as a configuration language as anything else. It does mean it will have to be parsed in the serverless half of the house. There are now interfaces that separate the commands and what needs to be done with those commands. Moved all of the turning of business data into ES JSON into its own utility and buried. Then fixed all of the broken bits. It is now more cleanly divided and ready for Java v2 implementation for serverless.
Not sure how it was tested, but added the handler to the system property so that it can be found by harvest. Not sure where all this should go or will go, but definitely needs to be here.
For testing, going to use a local opensearch with java v2 to simplify things. However this means adding the ability to do it and change the differentiator between the two connection types (now three): direct (managed) and multitenancy (serverless) and SDK 1 or SDK 2. The refactoring results in establishing dire connections using SDK 1 or SDK 2 or multitenancy with sdk 2.
@tloubrieu-jpl @alexdunnjpl can we do a couple quick smoke tests on this PR along with NASA-PDS/harvest#152 and NASA-PDS/registry-mgr#73 |
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.
I've been able to test it, and that works.
🗒️ Summary
Refactored this repo to handle multiple ways of connecting to an open/elastic search service.
⚙️ Test Data and/or Report
TBD
♻️ Related Issues
Closes #36
Closes NASA-PDS/harvest#158
Closes NASA-PDS/harvest#127