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

Remove legacy OODT dependencies #78

Merged
merged 10 commits into from
Jul 22, 2024
Merged

Remove legacy OODT dependencies #78

merged 10 commits into from
Jul 22, 2024

Conversation

jordanpadams
Copy link
Member

@jordanpadams jordanpadams commented Jul 21, 2024

🗒️ Summary

Refactor to remove legacy OODT dependencies.

NOTE: Merge #79 into this branch before merging to main

⚙️ Test Data and/or Report

Still working on branch testing workflow, but with everything deployed locally, I was able to run the smoke tests successfully:

Summary:

5 of 5 file(s) processed, 0 other file(s) skipped
0 error(s), 0 warning(s)

Product Labels:
5          Successfully registered
0          Failed to register

Registry Search Solr Documents:
5          Successfully created
0          Failed to get created

Product Types Handled:
2 Product_Collection
2 Product_Context
1 Product_Bundle


Registry Package Id: e3ca72b0-09b5-4ce9-909b-483ea5160867

End of Log

[SUCCESS] Harvest Successful
[INFO] Check solr docs match expected
[SUCCESS] Solr Doc Diff Successful
[INFO] Registry Manager Load
[SUCCESS] Registry Manager Load Successful

♻️ Related Issues

Resolves #75

* Copied numerous classes from OODT v1.9, which led to incompatibilities with some aspects of the code
* Additional refactoring to support upgraded OODT components
* New smoke tests for integration testing

Resolves #75
@jordanpadams jordanpadams requested a review from a team as a code owner July 21, 2024 17:00
Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

Whew! What a herculean effort … just a couple minor issues, take a look?

i.hasNext();) {
Map.Entry entry = (Map.Entry) i.next();
String key = entry.getKey().toString();
// for (Iterator i = sourceMet.getHashTable().entrySet().iterator();
Copy link
Member

Choose a reason for hiding this comment

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

When considering commenting-out code, either:

  • Include a comment explaining why it's commented out … or …  🤔
  • If you can't come up with one, then just delete the code completely 😏

@@ -1,191 +1,185 @@
package gov.nasa.pds.harvest.search.crawler.actions;
Copy link
Member

Choose a reason for hiding this comment

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

Was this entire file commented-out?

Could the file just be deleted?

@@ -0,0 +1,80 @@
//// Copyright 2006-2017, by the California Institute of Technology.
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a new file but is entirely commented-out … could this just not be part of the commit?

@jordanpadams
Copy link
Member Author

@nutjob4life thanks for the review? making these updates on #80 to avoid potential merge conflicts.

Refactor search-core dependencies
@jordanpadams jordanpadams merged commit eba5c0a into main Jul 22, 2024
0 of 2 checks passed
@jordanpadams jordanpadams deleted the i75 branch July 22, 2024 17:42
jordanpadams added a commit that referenced this pull request Jul 22, 2024
@jordanpadams
Copy link
Member Author

Cleanup completed here: 2295786

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.

Refactor code to remove OODT dependencies
2 participants