Skip to content

fix(execution): assume no async iterables in execute_sync - #291

Merged
Cito merged 1 commit into
graphql-python:mainfrom
youdie006:execute-sync-async-iterable
Nov 1, 2026
Merged

Cito merged 1 commit into
graphql-python:mainfrom
youdie006:execute-sync-async-iterable

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

Follow-up to #258. 333d0d7 made graphql_sync pass assume_not_async_iterable when check_sync is off, so objects that are both iterable and async-iterable (such as a Django QuerySet) are iterated synchronously. In #258 the plan was to set it "in graphql_sync and execute_sync", but execute_sync only got the is_awaitable part. It still uses the default async-iterable check, takes the async list path, and puts the coroutine into data:

before: execute_sync (dual): ExecutionResult(data={'listField': <coroutine object Executor.complete_async_iterator_value ...>}, errors=None)
        RuntimeWarning: coroutine 'Executor.complete_async_iterator_value' was never awaited
after:  execute_sync (dual): ExecutionResult(data={'listField': ['sync']}, errors=None)

A pure async generator now gives the same "Expected Iterable" field error as in graphql_sync, instead of a coroutine in data. With check_sync=True nothing changes. The helper is duplicated in execute.py, as assume_not_awaitable already is, because execute.py can't import from graphql.py. The tests in test_sync.py cover the dual iterable, a pure async iterable, and the check_sync=True error.

The full test suite passes with the 100% coverage gate on Python 3.14, and ruff check, ruff format --check and mypy src tests pass too. I pinned pytest below 9.2, because pytest-benchmark 5.3 errors on 9.2's benchmark fixtures.

Written with AI assistance (Claude); I have reviewed the change.

333d0d7 made graphql_sync pass assume_not_async_iterable when check_sync
is off (graphql-python#258). execute_sync was not updated, so an object that is both
iterable and async-iterable, such as a Django QuerySet, took the async
list path and execute_sync returned a coroutine in data with no error.
@youdie006
youdie006 requested a review from Cito as a code owner October 29, 2026 08:57
@codspeed

codspeed Bot commented Nov 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing youdie006:execute-sync-async-iterable (c6364f6) with main (679be84)

Open in CodSpeed

@Cito

Cito commented Nov 1, 2026

Copy link
Copy Markdown
Member

Thanks for spotting this and for the clear write-up and tests. I could reproduce the coroutine in data on main, and the fix matches what graphql_sync already does. Merged; it will be part of 3.3.2. I'll probably follow up with a small cleanup so that graphql.py imports both assume_not_* helpers from execute.py instead of keeping its own copies. The issue with pytest 9.2 has been solved in the meantime.

@Cito
Cito merged commit 4464465 into graphql-python:main Nov 1, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants