-
Notifications
You must be signed in to change notification settings - Fork 625
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
[Bug Fix] Stop accessing underlying storage for checking encoder existence #1417
Conversation
c9f5af4
to
273726c
Compare
Codecov Report
@@ Coverage Diff @@
## main #1417 +/- ##
==========================================
- Coverage 92.33% 92.24% -0.09%
==========================================
Files 176 176
Lines 14204 14227 +23
==========================================
+ Hits 13115 13124 +9
- Misses 1089 1103 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
try: | ||
self.meta_cache[key] = enc | ||
except ReadOnlyModeError: | ||
pass |
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.
If tile encoder does not exist and the storage is in read only mode, I am not sure we should be setting / returning an empty encoder that is not persistent. Should probably return None (and the calling method should check for None).
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.
checking for None won't work as that is the default value. To prevent multiple reads from storage we're checking self._tile_encoder to not be None. Persistence shouldn't be an issue as this only happens for read only.
If the user switches mode and tries to write, we will synchronize when the user does his first write in add_cachables_to_cache_dirty_keys
a98ffd1
to
924aafc
Compare
🚀 🚀 Pull Request
Checklist:
coverage-rate
upChanges