-
Notifications
You must be signed in to change notification settings - Fork 148
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(azure-storage-blob): add raw support #565
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
- Coverage 65.05% 59.63% -5.43%
==========================================
Files 39 42 +3
Lines 4055 3676 -379
Branches 487 590 +103
==========================================
- Hits 2638 2192 -446
- Misses 1408 1481 +73
+ Partials 9 3 -6 ☔ View full report in Codecov by Sentry. |
Thanks for bringing this up again, I unfortunately missed the PR. |
Hi @itpropro , is there any update on your review please? |
Hey, @pi0 do you thinks it's better to check for the item first and only then delete or to just catch a failed delete to nowhere? I would prefer to silently catch for performance reasons. |
I did not know that there is a default raw support test. I have seen that the fs driver has a raw items test, and I assumed this had to be added for drivers which implement these additional methods. If not needed, then let's remove this test! |
No problem, all drivers use the unstorage/test/drivers/utils.ts Line 136 in 67b09a1
|
I removed the redundant test. Should I extract the additional added tests to a separate PR @pi0 ? |
This PR adds raw item get/set support, similar to abandoned #442, but also adds test for that new procedures.
Also I have noticed that the current
removeItem
test fails, asdelete
operation with non-existent keys leads to an error. I assume the intention is not to fail in such cases, so I have not changed theremoveItem
test, but added a check before attempting to delete the blob at the Azure Blob driver.