Commit 1769c04
fix(openai): async pagination for OpenAI list methods [backport 3.17] (#15067)
Backport 52929aa from #14911 to 3.17.
Hey, so this is an attempt to fix
#14574 where doing `async
for model in client.models.list()` would fail with `TypeError: 'async
for' requires an object with __aiter__ method, got coroutine`.
### The Problem
Methods like `AsyncModels.list()` and `AsyncFiles.list()` don't actually
return coroutines - they return `AsyncPaginator` objects that you can
either:
- `await` to get the first page (what existing code does)
- Use with `async for` to iterate through all items (what was broken)
But our wrapper in `_patched_endpoint_async` was converting everything
into coroutines, which broke the `async for` use case.
### What I Tried
First attempt was using `inspect.iscoroutinefunction()` to detect which
methods are actually async vs just returning async objects. That got
messy fast because checking unbound methods from classes didn't work
reliably.
Then I tried just using the sync wrapper for list methods:
```python
if method_name == "list":
wrap(openai, async_method, _patched_endpoint(openai, endpoint_hook))
```
This looked promising - the pagination tests passed! But it broke
`test_model_alist` and `test_file_alist` because those tests do `await
client.models.list()` and expect full tracing with response metadata
like `openai.response.count`. Using the sync wrapper meant we lost all
that when the paginator was awaited.
Also tried returning the paginator directly without any wrapping, but
that meant we lost tracing entirely when someone did `async for`. Not
acceptable.
### A Solution
Created a `_TracedAsyncPaginator` wrapper class that implements both
`__aiter__` and `__await__`. This way:
- When you do `await client.models.list()` -> calls `__await__`, traces
properly, returns first page (existing behavior preserved)
- When you do `async for model in client.models.list()` -> calls
`__aiter__`, traces on first iteration, yields items (fixes the bug)
The wrapper is ~50 lines but it's the minimal solution that preserves
100% backward compatibility while fixing the breaking bug. Had to use
`finally` blocks to ensure traces complete even if iteration stops
early.
### Testing
Added two new pagination tests (`test_model_list_pagination` and
`test_model_alist_pagination`) that specifically test the `async for`
pattern.
Co-authored-by: Alexandre Choura <[email protected]>1 parent ac57ec7 commit 1769c04
File tree
6 files changed
+200
-25
lines changed- ddtrace/contrib/internal/openai
- releasenotes/notes
- tests
- contrib/openai
- snapshots
6 files changed
+200
-25
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
260 | 260 | | |
261 | 261 | | |
262 | 262 | | |
263 | | - | |
| 263 | + | |
| 264 | + | |
264 | 265 | | |
265 | 266 | | |
266 | 267 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
286 | 286 | | |
287 | 287 | | |
288 | 288 | | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
289 | 359 | | |
290 | | - | |
291 | 360 | | |
292 | | - | |
| 361 | + | |
293 | 362 | | |
294 | 363 | | |
295 | 364 | | |
296 | 365 | | |
297 | 366 | | |
298 | | - | |
| 367 | + | |
299 | 368 | | |
300 | | - | |
| 369 | + | |
301 | 370 | | |
302 | | - | |
303 | | - | |
304 | | - | |
305 | | - | |
306 | | - | |
307 | | - | |
308 | | - | |
309 | | - | |
310 | | - | |
311 | | - | |
312 | | - | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
313 | 383 | | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
323 | 400 | | |
324 | 401 | | |
325 | 402 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
38 | 54 | | |
39 | 55 | | |
40 | 56 | | |
| |||
46 | 62 | | |
47 | 63 | | |
48 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
49 | 81 | | |
50 | 82 | | |
51 | 83 | | |
| |||
Lines changed: 30 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
Lines changed: 31 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
0 commit comments