-
Notifications
You must be signed in to change notification settings - Fork 5
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
Issues with manifests after multiple slash update fix (#162) #178
Comments
@nutjob4life while you are waiting on the devops blocker, here is a bug I found. this is still backseat to NASA-PDS/devops#69 but next up. |
@jordanpadams superb, thanks! Will work on this |
On hold while https://pds.nasa.gov/api/search/1.0/ is reporting HTTP 502 Update from Slack: the new URL is https://pds.nasa.gov/api/search/1/ However the server is returning 501 when querying for Update: merging this PR fixes the invocation |
Okay this isn't actually a problem with the Deep Archive so much as the data is "bad". For
These URLs have For
This I can have Deep Archive work around the double-slash in |
@nutjob4life the Scenario 1: Contains >1 slash
Scenario 2: Does Not Contain extra slashes
Agreed deep-archive should not care about what the actual path is, just about encountering >1 slash and replacing that with 1 slash |
Should be done by the end of week. |
@jordanpadams thanks for spelling it out. Extra slashes in the bundle URL are easy enough to fix and affects just one line of code. However, upon digging deeper into the Deep Archive, I see a deeper problem that's more deeply concerning. Slicing and Dicing URLsIn the past, Deep Archive would find the right-most
Then the prefix
Works great! Until it doesn't. Record-Scratch MomentEnter
As you can see, the bundle has this extra The checksum manifest should only contain suffixes after the common prefix, but there's not really a common prefix we can compute for the bundle URL if it doesn't have to have anything in common with file_refs. I'm not sure what to do about this. One option is to make the checksum manifests much longer; instead of
have
Is that okay? Another option would be to write a little function to find a common prefix not based on
The common prefix in this case is Or what if we get this? 😱
Do we add a command-line argument like |
@nutjob4life let's just go with this:
|
Checked for duplicates
No - I haven't checked
🐛 Describe the bug
in cassini_uvis_solarocc_beckerjarmak2023_v1.0_20240903_checksum_manifest_v1.0.tab, lines like this:
should really be:
another example:
in cassini_uvis_solarocc_beckerjarmak2023_v1.1_20240903_checksum_manifest_v1.0.tab, lines like this:
should really be:
🕵️ Expected behavior
The results to be correct
📜 To Reproduce
See above
🖥 Environment Info
Mac OSx
📚 Version of Software Used
Latest -dev version as of today
🩺 Test Data / Additional context
No response
🦄 Related requirements
🦄 #162
The text was updated successfully, but these errors were encountered: