feat: Add cache layer for the datastore adapter#1514
Conversation
ArneGudermann
added
feature
phorward
left a comment
phorward
left a comment
There was a problem hiding this comment.
Thanks for facing this.
I'm a little bit astonished how bloated this code is, especially regarding this multiple-key feature. Is this anywhere used in ViUR, except in legacy code?
Anyway. I think this code might be improved in the cache-module, too.
My recommendation: When code that either handles one or a list of keys is written, write the code as it always handles a list of keys, so that the len(keys) == 1-case becomes the special case. This eliminates lots of bloating regarding isinstance()-checks and iterable-checking.
Can you please test my suggestions, and please don't forget to import utils to use ensure_iterable.
Co-authored-by: Jan Max Meyer <jmm@phorward.de>
|
@phorward we can not use |
The previous solution was because SortIndex was in viur-datastore previously.
I've fixed that and also a a bug and update the code to use |
| if not isinstance(keys, (tuple, list, set)): | ||
| keys = (keys,) |
There was a problem hiding this comment.
| if not isinstance(keys, (tuple, list, set)): | |
| keys = (keys,) | |
| keys = utils.ensure_iterable(keys) |
| keys = keys[MEMCACHE_MAX_BATCH_SIZE:] | ||
| except Exception as e: | ||
| logging.error(f"""Failed to get keys form the memcache with {e=}""") | ||
| for key, value in cached_data.items(): |
There was a problem hiding this comment.
Cached data might be uninizialized!
| logging.error(f"""Failed to get keys form the memcache with {e=}""") | ||
| for key, value in cached_data.items(): | ||
| entity = Entity(Key(key)) | ||
| entity = Entity(Key.from_legacy_urlsafe(key)) |
| if not isinstance(keys, (tuple, list, set)): | ||
| keys = (keys,) |
There was a problem hiding this comment.
| if not isinstance(keys, (tuple, list, set)): | |
| keys = (keys,) | |
| keys = utils.ensure_iterable(keys) |
| if not isinstance(keys, (tuple, list, set)): | ||
| keys = (keys,) |
There was a problem hiding this comment.
| if not isinstance(keys, (tuple, list, set)): | |
| keys = (keys,) | |
| keys = utils.ensure_iterable(keys) |
| if not keys: | ||
| return |
There was a problem hiding this comment.
This is tested twice, the keys list cannot be empty when coming to this line
| if not keys: | |
| return |
phorward
added
Priority: High
phorward
added
Priority: Medium
2 participants
No description provided.