-
Notifications
You must be signed in to change notification settings - Fork 369
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
feat: add includeFoldersAsPrefixes for managed folders #2413
Conversation
There are some assumptions baked into the Fixing this in paginator is not a straightforward process and would likely break other libraries. |
@@ -3129,6 +3129,20 @@ describe('storage', () => { | |||
assert.strictEqual(files!.length, NEW_FILES.length); | |||
}); | |||
|
|||
it('returns folders as prefixes when includeFoldersAsPrefixes is set', async () => { | |||
const expected = [`${DIRECTORY_NAME}/`]; | |||
const [, , result] = await bucket.getFiles({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't confirm a specific managed folder is in the list of prefixes; It might be the best we can do right now though because you'd need to make a one-off request to create a managed folder through Apiary Node.js client to verify functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By apiary you mean storage control?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there's support in Storage v1 json apiary that has CreateManagedFolder RPC which would help with this test until Storage Control is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably beyond the scope of this change to start pulling in other dependencies just for a single test case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When do we plan on adding the create functionality in this library?
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #2384 🦕