Repository navigation
Conversation
Add `__aiter__` to `Dataset` (delegating to `iterate_items`) and to `KeyValueStore` (delegating to the new `iterate_entries`), plus `KeyValueStore.iterate_values` and `KeyValueStore.iterate_entries`. Values are fetched one at a time as the iteration advances, so only a single value is held in memory regardless of the record sizes. On the Apify platform this means one request per record on top of the paginated key listing, which is the only way the API offers to read values. The memory key-value store client now skips keys deleted while a key iteration is suspended instead of raising `KeyError`, matching the other backends. Closes #1745 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
Add a non-abstract `KeyValueStoreClient.iterate_entries` to the storage client base class, yielding `KeyValueStoreRecord`s. The default implementation lists the keys with `iterate_keys` and reads each value with `get_value` as the iteration advances, so existing and third-party clients need no changes. Backends that can read keys together with their values more efficiently can override it. `KeyValueStore.iterate_entries` and `iterate_values` now delegate to the client method instead of combining `iterate_keys` and `get_value` themselves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
Override `iterate_entries` where the backend can do better than one value read per key: - The SQL client selects keys, metadata and values in a single streamed query, so a store with N records costs one query instead of N+1 and still holds only one row in memory at a time. - The Redis client fetches values with one HMGET per batch instead of two round trips per record. Batches are bounded by key count and by the record sizes known from the metadata hash, so large values do not pile up in memory. A record larger than the byte limit is fetched alone. Both clients share the value decoding with `get_value` through a new `_build_record` helper. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2267 +/- ##
==========================================
+ Coverage 93.87% 93.97% +0.10%
==========================================
Files 182 183 +1
Lines 13130 13252 +122
==========================================
+ Hits 12326 12454 +128
+ Misses 804 798 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Whether a record deleted mid-iteration is yielded depends on how the storage client reads values, which is not part of the contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
…e client `set_value` stores an empty byte string for `None`, so the batched read can fetch those records like any other and let `_build_record` map the `application/x-none` content type to `None`. This drops the key filtering and the per-key lookup dict from the batch fetch. Also reword the base `iterate_entries` docstring so it does not present the handling of records deleted mid-iteration as a contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
…ype checker redis-py types every reply as `bytes | str | None` because a client created with `decode_responses=True` returns strings. The key-value store client stores binary values and needs the raw bytes back, so it narrowed the type with `ty: ignore` and would fail with an `AttributeError` on such a client. Add `expect_bytes`, which narrows a reply to bytes and raises a `TypeError` naming the unsupported option, use it at both value-reading sites, and document the requirement on `RedisStorageClient`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
`expect_bytes` now handles a single reply, which `isinstance` narrows on its own, and the HMGET result is narrowed with a comprehension. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
…ient Datasets use RedisJSON and request queues parse JSON strings, so both work with a decoding Redis client. Only key-value store values are binary. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
`stream_results` buffers up to 1000 rows client side, so iterating a store of large values held hundreds of megabytes before the first yield, and the single open transaction lasted for the whole iteration. The SQL client now reads record metadata in keyset-paginated pages, splits each page into batches bounded by the record sizes, and fetches every batch's values with one query. Each query runs in its own short session. The batching rule is shared with the Redis client through a new `batch_records_by_size` helper. Measured with 300 records of 1 MiB: peak memory drops from 300 MiB to 9 MiB. With 2000 small records the iteration issues 41 selects instead of 2001 and is about 40 times faster than the default implementation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
The default iteration retries every read through `get_value`, but the batched HMGET in the Redis override was not retried. It now returns a list under the same `retry_on_error` as the other Redis reads, matching the SQL client. Add a direct unit test for `batch_records_by_size`, covering the count bound, the size bound, an oversized record alone in its batch and records of unknown size. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
The test directories have no `__init__.py`, so pytest cannot import two modules named `test_utils.py` and aborted the whole unit test collection. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
vdusek
left a comment
There was a problem hiding this comment.
I think this needs a little more thought before we merge it.
The main question is: should we mirror the JS API, or follow Python conventions?
IMO:
async for item in datasetshould behave like iterating over a list. And it currently does.async for ... in kvsshould behave like iterating over a dict (or tuple?). And now, it doesn't. Iterating over a Python dict yields keys, and you explicitly call.items()when you want(key, value)pairs. Our__aiter__, however, yields(key, value)tuples.
That means someone writing async for key in kvs would silently get a tuple instead of a string, which feels surprising in Python.
Also, I think both storages should support both styles: the __aiter__ dunder for the natural/default async for behavior. And the explicit methods when you want something more specific/explicit.
- Dataset:
__aiter__yields items, whileiterate_items()supports the filtering options. - KVS:
__aiter__yields keys (dict-like), withiterate_keys(),iterate_values(), anditerate_entries()for keys, values, and(key, value)pairs respectively.
`async for key in kvs` now yields the keys, matching how iterating over a `dict` behaves. Values and `(key, value)` pairs stay available through `iterate_values` and `iterate_entries`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wg6jZgyQp7XkvVvxuueJyV
Description
Dataset.__aiter__, soasync for item in datasetyields the items, like iterating over a list.KeyValueStore.iterate_values,iterate_entriesand__aiter__.async for key in kvsyields the keys, like iterating over adict;iterate_valuesanditerate_entriesgive the values and(key, value)pairs. This follows Python conventions and differs from JS, where iterating a KVS yields[key, value]pairs.KeyValueStoreClient.iterate_entrieswith a key-by-key default; the SQL and Redis clients read in batches bounded by record count and size.decode_responses=Trueis not supported for KVS and fix related typing.Issues
Testing
Checklist