fix: Update S3 prefix listing to work with ListObjectsV2 API #946
fix: Update S3 prefix listing to work with ListObjectsV2 API #946chrisj merged 2 commits intogoogle:masterfrom
Conversation
|
Can you add a test of the parsing to demonstrate the failure with the old code (which gets fixed with the new code)? |
Added as a browser test (passes with this branch, fails in master). Running the test as a spec tests passes in the original implementation because node's Tested via |
| namespaceURI, | ||
| "Prefix", | ||
| ); | ||
| for (let j = 0; j < prefixNodes.length; ++j) { |
There was a problem hiding this comment.
these for loops would be a bit cleaner as for..of
| ); | ||
| for (let j = 0; j < prefixNodes.length; ++j) { | ||
| let name = prefixNodes.item(j)!.textContent; | ||
| if (name === null) continue; |
There was a problem hiding this comment.
I don't think null is a possibility because these are Element not Node if we trust lib.dom.d.ts
4 participants
Got a fix here for the s3 autocomplete. If this has been fixed or addressed in another PR or issue, please disregard. Thanks for your review!
Summary of issue
I noticed for the past couple months that S3 URIs have not been completing in the source input box in Chrome/Safari/Firefox. Should be reproducible in any mainline Neuroglancer distribution. (Try
s3://bossdb-open-data/)Google cloud bucket URIs were autocompleting. I assumed it was an issue with the listing operation itself but the XML response looked correct to me (properly showing all prefixes delimited by a
/), so I think I tracked the issue with the XML parser in the S3 listing script here.What Changed
Technically we might not need
Keysor individual objects.Impact
S3-backed datasource URL autocomplete now surfaces prefixes/files as expected for namespace-qualified ListObjectsV2 XML responses.
Testing
npm test -- tests/kvstore/s3.spec.tsThis change was implemented with assistance from Codex (AI coding assistant). CLA has been signed.