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

Implement reindexing sweeper #155

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

Conversation

alexdunnjpl
Copy link
Contributor

🗒️ Summary

Implements reindexing sweeper/script
Implements support for aliased indices in all sweepers code

⚙️ Test Data and/or Report

One of the following should be included here:

  • Reference to regression test included in code (preferred wherever reasonable)
  • Attach test data here + outputs of tests

♻️ Related Issues

Related to NASA-PDS/registry#339 (comment)
Related to NASA-PDS/registry#329
Related to NASA-PDS/registry#338

this currently yields strange results which may indicate that OS/python are inconsistent in their lexical sorting
…dest

This avoids issues where ES/python have inconsistent lexical sort approaches
TODO: implement argparse CLI
without this in place prior to migration, the reindexer sweeper will fail
… documents

this is a bandaid, but should be replaced by detection/handling of "RequestError(400, 'illegal_argument_exception', 'ReleasableBytesStreamOutput cannot hold more than 2GB of data'" such that
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.

The pull request still has boilerplate:

One of the following should be included here:

Reference to regression test included in code (preferred wherever reasonable)
Attach test data here + outputs of tests

Any of those outputs available to replace the boilerplate?

I tried running the tests myself with tox -e py39 but got 4 failed, 39 passed, 120 warnings, 8 errors so I might be running them wrong 🤷

@alexdunnjpl
Copy link
Contributor Author

The pull request still has boilerplate:

One of the following should be included here:
Reference to regression test included in code (preferred wherever reasonable)
Attach test data here + outputs of tests

Any of those outputs available to replace the boilerplate?

I tried running the tests myself with tox -e py39 but got 4 failed, 39 passed, 120 warnings, 8 errors so I might be running them wrong 🤷

Sorry, meant to open this as a draft PR - will update and re-request review later

@nutjob4life nutjob4life marked this pull request as draft January 30, 2025 22:10
@nutjob4life
Copy link
Member

@alexdunnjpl no worries! Converted to "draft". Thanks!

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@alexdunnjpl alexdunnjpl marked this pull request as ready for review January 31, 2025 20:25
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.

Pull request description … still has boilerplate text 🤷
Visual inspection of code … looks fine ✓
Running tox -e py39 … get 51 passed, 120 warnings 🎉
Approval: ✅

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.

2 participants