django-ninja
django-ninja copied to clipboard
Async Pagination support
Fixes https://github.com/vitalik/django-ninja/issues/547
This PR adds support for async pagination and updates existing pagination classes to work with async pagination. The _inject_pagination function now checks to see if the view function is async and then, if so, adds view_with_pagination as an async function that calls an apaginate_queryset method expected in the new AsyncPaginationBase. The async view_with_pagination also forces async evaluation of the results queryset if necessary.
Hi @jamesrkiger
I wanted async pagination in a project of my own and didn't want to wait for this to be merged. So I tried to incorporate what you wrote here into my codebase. I ran into the following error:
Traceback (most recent call last):
File "my_project/.venv/lib/python3.12/site-packages/ninja/operation.py", line 281, in run
result = await self.view_func(request, **values)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "my_project/backend/api/apagination.py", line 216, in view_with_pagination
result[paginator.items_attribute] = [
^
File "my_project/backend/api/apagination.py", line 206, in evaluate
for result in results:
File "my_project/.venv/lib/python3.12/site-packages/django/db/models/query.py", line 400, in __iter__
self._fetch_all()
File "my_project/.venv/lib/python3.12/site-packages/django/db/models/query.py", line 1928, in _fetch_all
self._result_cache = list(self._iterable_class(self))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "my_project/.venv/lib/python3.12/site-packages/django/db/models/query.py", line 91, in __iter__
results = compiler.execute_sql(
^^^^^^^^^^^^^^^^^^^^^
File "my_project/.venv/lib/python3.12/site-packages/silk/sql.py", line 100, in execute_sql
return self._execute_sql(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "my_project/.venv/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 1560, in execute_sql
cursor = self.connection.cursor()
^^^^^^^^^^^^^^^^^^^^^^^^
File "my_project/.venv/lib/python3.12/site-packages/django/utils/asyncio.py", line 24, in inner
raise SynchronousOnlyOperation(message)
django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async.
It seems to be a problem with the following lines:
https://github.com/vitalik/django-ninja/pull/1030/files#diff-27c0497d3e3ccd2e01267109383e86fd3bcb7685ce39d96e9ce6af4864d04aeeR199-R201
async def evaluate(results: Union[List, QuerySet]) -> AsyncGenerator:
for result in results:
yield result
Rewriting it to the following made it work for me:
async def evaluate(results: Union[List, QuerySet]) -> AsyncGenerator:
if isinstance(results, QuerySet):
async for result in results:
yield result
else:
for result in results:
yield result
I don't know if it is a general issue and you might want to incorporate this or it's a weird issue that only occured to me.
@shmulvad what does your api code look like? It sounds possibly related to my comment here. Understanding the various use cases would be helpful.
@jamesrkiger and I are working together to add async + cursor pagination to our project GlitchTip. Feel free to snoop around our open source code.
Let us know if we can help further @vitalik I'd be happy to make changes, do something boring to free up your time to review, or publish pagination as a third party package. My goal is that async just works and cursor pagination is an option, built upon existing django-ninja pagination.
@bufke I was just returning basic querysets from the view function. Something like return MyModel.objects.all().order_by('id'). Unfortunately I cannot share it in more details.
Thank you
Testing this out in the 1.2.0 release, I'm also seeing a SynchronousOnlyOperation tracing back to https://github.com/vitalik/django-ninja/blob/1638dd825e99dbe3f9fc43dc8770059249cfa8c6/ninja/pagination.py#L206-L208
The return from the route is again a queryset, ie return MyModel.objects.all(). Updating that to an async for works.
@stumpylog @shmulvad Thank you for your notes, I will investigate this and submit another PR for a fix.