Commit 8e30197
authored
fix: Stop dataset iterators from skipping items when unwind is used (#1059)
## What was wrong
`iterate_items` advanced the dataset offset by `max(scanned_rows,
len(items))`. With `unwind`, a page returns more
items than the rows it scanned, so the offset jumped past rows the next
request never read. At the default
`chunk_size`, a 5000-row dataset with a 3x unwind yielded about a third
of it.
## The fix
The offset now advances by at most the number of rows the call asked
for. The items endpoint applies `offset` and
`limit` to the rows first and shapes the result afterwards, and it
passes no `maxLimit` (unlike the collection list
routes), so a page never covers more rows than the `limit` it was sent.
Capping the advance there keeps an unwound
page from running past the window it actually read.
The `max()` stays underneath the cap on purpose.
`x-apify-pagination-count` comes from the dataset's `itemCount`,
which the API increments through a throttled write while the items
themselves stream fresh. Following the header
alone, which is what
[apify-client-js#1044](apify/apify-client-js#1044)
does and what #1058
suggested, would truncate `iterate_items` right after `push_items` for
anything over one page. The JS iterator
bounds itself by `total` and so is exposed to that lag either way; this
one consults neither, which is why the two
clients end up with different fixes.
`DatasetItemsPage.count` keeps its current value of `max(header,
len(items))`. That is not the no-op it looks like:
it is what makes `count` usable right after a push, and two integration
assertions depend on it. Its docstring
changes, along with the `limit` docs on `iterate_items` and `chunk_size`
on both twins, since all of them promised
items where the field and the parameters have always counted scanned
rows.
One case stays imperfect. On a final short page with `unwind`, the
advance is still the full `limit` rather than the
rows the page covered, so rows appended by a concurrent push after that
page can be missed. Reaching them needs the
raw header, which `list_items` folds into `count`. It is strictly better
than before, and the docstring says so
rather than claiming the overshoot is free.
## Behavior change worth a changelog line
`iterate_items(limit=N)` with `unwind` and a `chunk_size` below `N` now
walks all N rows instead of stopping after
the first page, so it can yield several times more items for the same
`limit`. That is the fix working, but it is
user-visible.
## Tests
- `unwind` reached the pagination tests for the first time. The fake API
now splits each row into three items after
the row window is picked, exactly as the transform stream does, and
three cases iterate through it.
- The fake API used to cap every page at 1000 items "mirroring the real
API", which is only true of the collection
endpoints. It now applies the requested limit verbatim on the items
endpoint, which is what let a `chunk_size`
above that cap be covered at all.
- A page whose `count` lags behind its items has a test of its own.
Nothing pinned that before, and it is the
invariant the `max()` rests on.
- `test_dataset_iterate_items_unwound` covers the whole thing against
the live API. It has not run locally, so CI
is its first real execution.
## One thing found while verifying
`get_cursor_iterator` justified its termination rule with filters that
"can drop every item on a page while a live
cursor still points at more data". The API rules that out: the key-value
store returns the last key of the page as
the next cursor, and the request queue returns a cursor only once a page
came back full, so neither can hand back a
cursor that outlives its page. The behavior is fine and unchanged; the
docstring now says why it holds.
Closes #1058
*✍️ Drafted by Claude Code*1 parent f52190d commit 8e30197
5 files changed
Lines changed: 210 additions & 46 deletions
File tree
- docs/02_concepts
- src/apify_client
- _resource_clients
- tests
- integration
- unit
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
23 | | - | |
24 | | - | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
41 | | - | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
49 | 48 | | |
50 | 49 | | |
51 | 50 | | |
52 | | - | |
| 51 | + | |
| 52 | + | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
62 | 63 | | |
63 | | - | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
68 | | - | |
| 69 | + | |
69 | 70 | | |
70 | 71 | | |
71 | 72 | | |
| |||
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| 93 | + | |
92 | 94 | | |
93 | | - | |
| 95 | + | |
94 | 96 | | |
95 | 97 | | |
96 | 98 | | |
97 | 99 | | |
98 | 100 | | |
99 | | - | |
| 101 | + | |
100 | 102 | | |
101 | 103 | | |
102 | 104 | | |
| |||
126 | 128 | | |
127 | 129 | | |
128 | 130 | | |
129 | | - | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
130 | 144 | | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
137 | 149 | | |
138 | 150 | | |
139 | | - | |
140 | | - | |
| 151 | + | |
| 152 | + | |
141 | 153 | | |
142 | | - | |
| 154 | + | |
143 | 155 | | |
144 | 156 | | |
145 | 157 | | |
| |||
218 | 230 | | |
219 | 231 | | |
220 | 232 | | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| |||
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
207 | | - | |
208 | | - | |
| 207 | + | |
| 208 | + | |
209 | 209 | | |
210 | 210 | | |
211 | 211 | | |
| |||
237 | 237 | | |
238 | 238 | | |
239 | 239 | | |
240 | | - | |
| 240 | + | |
| 241 | + | |
241 | 242 | | |
242 | 243 | | |
243 | 244 | | |
| |||
260 | 261 | | |
261 | 262 | | |
262 | 263 | | |
263 | | - | |
| 264 | + | |
264 | 265 | | |
265 | 266 | | |
266 | 267 | | |
| |||
763 | 764 | | |
764 | 765 | | |
765 | 766 | | |
766 | | - | |
767 | | - | |
| 767 | + | |
| 768 | + | |
768 | 769 | | |
769 | 770 | | |
770 | 771 | | |
| |||
796 | 797 | | |
797 | 798 | | |
798 | 799 | | |
799 | | - | |
| 800 | + | |
| 801 | + | |
800 | 802 | | |
801 | 803 | | |
802 | 804 | | |
| |||
819 | 821 | | |
820 | 822 | | |
821 | 823 | | |
822 | | - | |
| 824 | + | |
823 | 825 | | |
824 | 826 | | |
825 | 827 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
596 | 596 | | |
597 | 597 | | |
598 | 598 | | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
599 | 640 | | |
600 | 641 | | |
601 | 642 | | |
| |||
0 commit comments