Use removeBlobs() for bulk-delete. #39
Conversation
There was a problem hiding this comment.
We really shouldn't have printStackTrace anywhere in our code.
There was a problem hiding this comment.
Done -- was following the other code in the project (e.g. AuthResource, ExceptionLogger, and IdentityResource).
timuralp
force-pushed
the
bug/use-remove-blobs
branch
from
c96cff5 to
52aa43b
Compare
|
|
timuralp
force-pushed
the
bug/use-remove-blobs
branch
from
52aa43b to
7c9fe56
Compare
|
👍 |
1 similar comment
|
👍 |
There was a problem hiding this comment.
Do you want to use a Multimap instead?
There was a problem hiding this comment.
That would work too. I'll change the commit.
We should use removeBlobs() for bulk-delete. The change deletes blobs en-masse. Further, we no longer will perform a HEAD on every blob, as that's fairly expensive. Fixes #38
timuralp
force-pushed
the
bug/use-remove-blobs
branch
from
7c9fe56 to
7ff978e
Compare
There was a problem hiding this comment.
I think you're right, because we already do substring(1) on line 161. I'm not sure how this works right now for containers that start with "/"!
There was a problem hiding this comment.
We are probably deleting a non-existent container which does not generate an error. We should enable/add some swift-tests to check for these behaviors.
|
@timuralp Do we we a path forward on this PR? |
|
@gaul I'll need to reload this into my brain. Will try to check it out over the weekend. |
We should use removeBlobs() for bulk-delete. The change deletes blobs
en-masse. Further, we no longer will perform a HEAD on every blob, as
that's fairly expensive.
Fixes #38