-
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
Enhance provenance sweeper with more graceful failure when registry contains zero documents #25
Comments
Is there a stack trace or something to go this this? I am not seeing any division problems. |
@al-niessner if you've run sweepers against a completely-empty registry and aren't able to replicate, possibly this was fixed and the issue not closed, or fixed as a side-effect of something else, in which case this issue can be closed without further action. |
On a completely empty opensearch (no registry) get a bunch of SSL messages because of self signed open cert (who cares it is local testing) and then a good error telling you that there is no registry:
Did we want to make that error more ambiguous somehow? |
Then with an opensearch that has no data (created with registry/docker docker compose --profile=dev-api up):
Same unimportant self-signed warnings but no other errors. If this is sufficient for you two, then I will let you close it without further adieu. |
@al-niessner looks like you're just running provenance there, not the entire set of sweepers? Should be this'n: https://github.com/NASA-PDS/registry-sweepers/blob/main/docker/sweepers_driver.py |
Yes, because the subject specifically says provenance sweeper. No need to run/test others. |
@al-niessner my mistake - I'm so used to mentally translating between the two because people still refer to the sweepers suite as "provenance". Probably I wasn't doing that in the ticket title, but I can't be absolutely certain, so it may be best to run the full suite for completeness' sake given that it's not replicable via just the provenance script. Your call though. |
Not my call. I am just working on what was stated in the ticket. If the requirements need to be changed, then change them (fix the subject). You can also just state that the subject is wrong and they all need to be done (may be best for completeness sake is not stating it). I am used to requirements changing but it costs money when they change; so, whoever changes it has to take clear responsibility for added costs. |
@al-niessner this ticket was basically a quick note-to-self from a few months ago that I'd forgotten the empty-registry corner case, so that I'd remember to loop back to it (hence the flippant original text). Something something good intentions... Because of that, I can only speculate whether I made a mistake in the initial ticket subject. I'll take a look now and see whether I can replicate it. |
I couldn't replicate it with the full-suite driver, so safe to say it's no longer a valid issue. |
Update to safeguard against
DivideByZero
The text was updated successfully, but these errors were encountered: