Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
merged 25 commits into from
Jan 16, 2023

Conversation

jirimoravcik
Copy link
Member

@jirimoravcik jirimoravcik commented Jan 4, 2023

Skipped RequestQueue class as it's quite big and not that related to Actor class. Will be in next PR

@github-actions github-actions bot added this to the 54th sprint - Platform team milestone Jan 4, 2023
@github-actions github-actions bot added the t-core-services Issues with this label are in the ownership of the platform team. label Jan 4, 2023
@jirimoravcik jirimoravcik changed the title feat: Implement Dataset, KeyValueStore and RequeestQueue classes feat: Implement Dataset, KeyValueStore classes, create storage management logic Jan 11, 2023
@jirimoravcik jirimoravcik marked this pull request as ready for review January 11, 2023 22:04
@jirimoravcik jirimoravcik requested a review from fnesveda January 11, 2023 22:04
Copy link
Member

@fnesveda fnesveda left a 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.

tests/unit/conftest.py Show resolved Hide resolved
src/apify/actor.py Outdated Show resolved Hide resolved
src/apify/storages/_utils.py Outdated Show resolved Hide resolved
self._cache = OrderedDict()
self._max_length = max_length

def get(self, key: str) -> Optional[T]: # TODO: Consider making this __getitem__ to be more Pythonic
Copy link
Member

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.

Copy link
Member Author

@jirimoravcik jirimoravcik Jan 16, 2023

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

src/apify/storages/storage_manager.py Outdated Show resolved Hide resolved
@jirimoravcik jirimoravcik requested a review from fnesveda January 16, 2023 10:38
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@jirimoravcik jirimoravcik merged commit d1b357c into master Jan 16, 2023
@jirimoravcik jirimoravcik deleted the feature/implement-dataset-rq-kvs-classes branch January 16, 2023 11:06
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-core-services Issues with this label are in the ownership of the platform team. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.