Skip to content

feat: implement _snapshot_client for Snapshotter #957

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.

Sign up for GitHub

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 2 commits into from
Feb 11, 2025
Merged

feat: implement _snapshot_client for Snapshotter #957

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8722ce
chore: implement _snapshot_client for Snapshotter
Mantisus Feb 4, 2025
3e250f8
add `get_rate_limit_errors` in `BaseStorageClient`
Mantisus Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/crawlee/_autoscaling/snapshotter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,11 @@ def _snapshot_client(self) -> None:
Only errors produced by a 2nd retry of the API call are considered for snapshotting since earlier errors may
just be caused by a random spike in the number of requests and do not necessarily signify API overloading.
"""
# TODO: This is just a dummy placeholder. It can be implemented once `StorageClient` is ready.
# Attribute `self._client_rate_limit_error_retry_count` will be used here.
# https://github.com/apify/crawlee-python/issues/60
client = service_locator.get_storage_client()

error_count = 0
rate_limit_errors: dict[int, int] = getattr(client, 'rate_limit_errors', {})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I need some explanation here... What values can be in rate_limit_errors? What value is going to be there for the MemoryStorageClient (nothing?) and for the ApifyStorageClient? Thanks.

Copy link
Collaborator Author

@Mantisus Mantisus Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For MemoryStorageClient it is empty dict. Since getattr will return the default value if the specified attribute does not exist
For ApifyStorageClient it is rate_limit_errors from https://github.com/apify/apify-client-python/blob/master/src/apify_client/_statistics.py#L15. Relate PR - apify/apify-sdk-python#387

Copy link
Collaborator

Choose a reason for hiding this comment

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

I came here from apify/apify-sdk-python#387. I'd much prefer to add another method to the storage client interface, along with a dummy implementation (return {}) on the memory storage client.

Then we can avoid the getattr here, which is pretty confusing and non-standard.


error_count = rate_limit_errors.get(self._CLIENT_RATE_LIMIT_ERROR_RETRY_COUNT, 0)
snapshot = ClientSnapshot(error_count=error_count, max_error_count=self._max_client_errors)

snapshots = cast(list[Snapshot], self._client_snapshots)
Expand Down
Loading