-
Notifications
You must be signed in to change notification settings - Fork 12
feat: Implement Dataset, KeyValueStore classes, create storage management logic #21
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: Implement Dataset, KeyValueStore classes, create storage management logic #21
Conversation
…t and request queue
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.
Wow, looks great! I just had a couple of smaller comments, but otherwise I think it is good to go.
src/apify/storages/_utils.py
Outdated
self._cache = OrderedDict() | ||
self._max_length = max_length | ||
|
||
def get(self, key: str) -> Optional[T]: # TODO: Consider making this __getitem__ to be more Pythonic |
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.
Wow, that would be really cool, if this had __getitem__
, __setitem__
and __delitem__
, then it would basically support the Mapping
protocol and would be usable as a dictionary.
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.
Found a cool SO post about it https://stackoverflow.com/questions/19775685/how-to-correctly-implement-the-mapping-protocol-in-python
I'll look into it when implementing request queue, but it would be quite nice
also found this: https://gist.github.com/davesteele/44793cd0348f59f8fadd49d7799bd306
it directly inherits an OrderedDict
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.
Looks great!
Skipped
RequestQueue
class as it's quite big and not that related toActor
class. Will be in next PR