GH-79 Copy and delete performance #92
Conversation
* Copy and delete now share a listCommand - DRY * Copy and delete now pass down a continuationToken * Delete will now look for formData Resolves: GH-79
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
+ Coverage 53.64% 55.61% +1.96%
==========================================
Files 38 39 +1
Lines 1795 1836 +41
==========================================
+ Hits 963 1021 +58
+ Misses 832 815 -17 ☔ View full report in Codecov by Sentry. |
| return /* await */ deleteObjects(env, daCtx); | ||
| export async function deleteSource({ req, env, daCtx }) { | ||
| const details = await deleteHelper(req); | ||
| return /* await */ deleteObjects(env, daCtx, details); |
There was a problem hiding this comment.
Why is the await commented out instead of deleted?
There was a problem hiding this comment.
I do not know. I left it there as it was from the original variation of this work.
There was a problem hiding this comment.
I did that 😄
The reason is that I find it problematic that all of a sudden the linter wants you to lose the await on an async function if its called on return. If you change the function later and want to add some statements before returning you might forget to add the await at that point.
So I just added the await in a comment as a reminder.
But if you don't like it feel free to remove 😄
| }), | ||
| ); | ||
| } | ||
| await Promise.all(sourceKeys.map(async (key) => { |
There was a problem hiding this comment.
I know for the client-side operations @auniverseaway is now using a queue to limit the number of concurrent operations. Is that something that could be needed if a very large copy operation is happening?
There was a problem hiding this comment.
As long as we stay under 1000 sub-requests, we should be fine. The Queue class on client-side is for when we have 2000+, want to send all immediately, and don't want the browser to choke with too many requests.
|
Just wondering is there anything stopping us from merging this? |
Fix key passed to postObjectVersionWithLabel when deleting
Description
Copy & Delete now return a specific status code based on resulting state:
Copy operation remaining keys are stored in KV, subsequent calls for that operation w/ continuation token will use the remaining list, updating KV as necessary if more remain.
Delete operation can simply be called again and again until a 204 response is returned.
Related Issue
Fixes #79
How Has This Been Tested?
Added unit tests, also added functional/integration test as expected by browser.
Screenshots (if appropriate):
Types of changes
Checklist: