-
-
Notifications
You must be signed in to change notification settings - Fork 372
fix: ensure ZipStore is open before acquiring lock
#3593
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3593 +/- ##
==========================================
+ Coverage 60.90% 60.92% +0.02%
==========================================
Files 86 86
Lines 10174 10180 +6
==========================================
+ Hits 6196 6202 +6
Misses 3978 3978
🚀 New features to boost your workflow:
|
|
how did the bug you are fixing here get past the store tests? Maybe we need to make those tests stronger? |
|
You're right, I didn't want to appear presumptuous but the code could do with some refactoring as well, having to repeat all those |
|
if you can find a clean way to reduce the repetition (e.g., a decorator) we'd appreciate it! |
|
@d-v-b just for info, I'm currently on holiday but I'm planning to implement the proposed improvements shortly |
|
@d-v-b I've finally had time to revisit this! I've added tests and cleanup up the open check, the proposed decorator approach turned out not very clean imo because you'd need to test for the various types of functions (async, iterator, etc.) |
Fix a bug where
ZipStorecomplains that it has no attribute_lockwhen some methods are called. Any method using the lock should make sure the store is opened because that's when the_lockattribute gets set.