django-rest-framework
django-rest-framework copied to clipboard
Async implementation
Note: Before submitting this pull request, please review our contributing guidelines.
Description
This PR is related to issue refs #8496. It is a tentative implementation of async for APIView.
Regarding the check if not async_to_sync I have time it and the overhead is small (if not is faster than is None):
In [1]: %timeit if None is None:pass
11.3 ns ± 0.0369 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [2]: %timeit if not None:pass
5.8 ns ± 0.00717 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [3]: def func():
...: pass
...:
In [4]: %timeit if func is None:pass
17.9 ns ± 0.162 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [5]: %timeit if not func:pass
16.3 ns ± 0.115 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
Great - thanks for your time on this - all looks neatly done.
My concerns here are around introducing false expectations to our users.
I'll explore this over on https://github.com/encode/django-rest-framework/issues/8496
- How would we like the documentation for this to look? Should we add a new section to the "Views"?
- I'd really really only want to merge this if we could ensure we didn't have scope-creep. For example once we have async here, it potentially makes sense to have async variants of the generic views etc. Perhaps we'd want to have basic support built in, but with a third party package for anything more.
- I find the test cases a bit confusing. Do async API views work okay with the regular
APIRequestFactory? Does this differ from how Django'sRequestFactoryworks with regular Django async views?
Docs
Yeah this probably makes the most sense as a Async Views section in the Views docs.
Scope creep
Since this solution was implemented within the class APIView(View), there shouldn't be any scope creep on anything that inherits from that.
The only thing I can think of is the current solution doesn't think about DRF function views. Based on the way rest_framework.decorators.api_view is implemented, this current solution might just automatically work? New tests to cover async function views are required regardless.
I updated the docstring and added support for async function based views.
I also added the documentation under Views.
I have not replicated, but if these issues are confirmable then this PR is not ready for merge yet.
Looks like this PR could consider adding robustness on the testing suite, such as using playwright (preferred) or selenium to test the Django admin and DRF UI.
#8655
We will definitely need to do some deeper considerations on how DRF will handle user authentication in the future now. The quick and dirty solution might just be to add database_sync_to_async to anything that handles auth.
Needs more consideration on how to fix this.
#8656
Not entirely sure how the handler function got morphed into a response. Needs further digging.
#8657
The data serialization issue makes sense though. Running the DRF serializer within an async view would definitely raise Django ORM issues.
Thanks @dongyuzheng for finding the issues. I wrote some additional test cases to uncover issues https://github.com/encode/django-rest-framework/issues/8655 and https://github.com/encode/django-rest-framework/issues/8656 and fixed them. Regarding issue https://github.com/encode/django-rest-framework/issues/8657 this is expected since ORM queries need to be wrapped by the sync_to_async function or need to use the new async API of the ORM.
@em1208, great work on the fast fix.
Regarding issue #8657 this is expected since ORM queries need to be wrapped by the
sync_to_asyncfunction or need to use the new async API of the ORM.
This might be a problem, because returning serializer.data should be a common use case. Wrapping sync_to_async will make all of these async APIs a bit worse.
I once worked in a codebase where to add true async support, they had to copy paste everything and rewrite a v2 as async.
I am worried we will add too many if async; else all over the place
@dongyuzheng thanks! Unfortunately regarding https://github.com/encode/django-rest-framework/issues/8657 there is no other solution except the ones I mentioned. A complete async rewrite is definitely possible but if we want to keep using the same class APIView there will be many if async; else to handle both the sync and async cases.
For user APIs like serializer.data, we should follow Django's current conventions and create *.adata counterparts that are simple wrappers around *.data combined with database_sync_to_async.
So, my honest perspective based on the above is that it's strengthened my position on not really wanting to add async support to REST framework. It looks to me like we'd end up pushing folks into making poor development choices(*), and it'll also personally make my life more tedious because we'll end up with a whole bunch of new kinds of support tickets, that I could really do without.
There aren't any technical reasons why a third party package couldn't fulfil this space. If folks really want async view support in REST framework, then that's a route I'd suggest.
(*) Tech for the sake of tech, rather than actually improving a clear business use-case / demonstrably improving some important metrics etc.etc.
The main benefit of async comes from IO-bound scenarios. For example, these benchmarks show that fully async python web stacks are considerably faster thanks to the ability for python to execute during HTTP transmit. The amount of time Python views take to execute is generally an order of magnitude faster than any IO operation.
For example, a large portion of DRF endpoints utilize database queries. The ability for the webserver to process other views while performing database queries and HTTP responses/receives is absolutely significant.
In my opinion, passing on integrated async support would be deferring something that DRF will eventually implement. Having an external async package will most likely cause more maintainability issues (more specifically, OSS developers going MIA) for something that will only become more important as Django core embraces async.
If you disagree, this PR can be closed.
I can only imagine the headache involved in handling async support on top of all the existing tickets regarding normal DRF. That being said, this is something my team absolutely needs due to being IO-bound on many of our endpoints (forwarding requests to other APIs etc). If the plan is to spin this off into a 3rd party package, then I suppose that is fine. But I really hope we can figure out a way to get quality async view support into DRF asap.
Really appreciate all the hard work here!!
This pull request shows exactly how to subclass APIView and create an AsyncAPIView.
Y'all can add this into your projects now, today, and immediately, without being blocked by REST framework at all. 🎉
Good steps for folks wanting to make that more easily accessible...
- Create a
gistwith just this subclass, and a bit of example text, showing folks how to create anAsyncAPIViewthat they can use in their own projects. - Next step beyond that, is wrapping that up into a Python package.
- And then expanding on that if wanted... eg. adding the decorator, or whatever other feature requests end up coming in.
It'd also be valuable to have someone demo an AsyncAPIView with an outgoing async HTTP request and actually benchmark if you're able to achieve higher throughput than the sync equivalent case.
@tomchristie we may attempt to integrate async support in an upcoming sprint using this PR. If so, I will try and get our team to put together a gist and / or other useful for others.
I fail to see how this pull request shows how to create an AsyncAPIView? Do you mean the README with this:
class AsyncView(APIView):
async def get(self, request):
return Response({"message": "This is an async class based view."})
If so, that won't work without the other changes in this PR. This minimal view is enough to make DRF call into the ORM which doesn't like being called synchronously in an async context, and it errors out. I might be (and probably am) missing something obvious, but so far I see no way to use DRF in an async way. I much rather use built-in async support than having to use yet another dependency which we have to keep up-to-date and verify for stability and security, but if there is no other option we'll (have to) resort to it.
With the comments made by you here @tomchristie, can I assume this PR won't be accepted and there will be no native async support in DRF?
@em1208 Your PR on this is a really great starting point.
As I see it, the next step is taking that work, and making it more accessible to users to start trying out.
I'm wondering if you (or anyone else) would be interested in adapting it to into a repository that exposes an AsyncAPIView class and @async_api_view decorator?
This wouldn't be clone of REST framework, but would instead just subclass APIView...
class AsyncAPIView(rest_framework.APIView):
# The code changes demonstrated in this pull request
Together with a nicely put together README, this would give us something easily installable for users to start working with, and giving feedback on. That is our step 0 here.
I'm happy to collaborate with anyone who wants to work on this if it's unclear.
@tomchristie sure, I will be happy to work on it. I will post here again once ready.
I just released a separate package to implement the async views support for Django REST framework following the approach of this PR. Here is the link to the code: https://github.com/em1208/adrf. The package has been published in PyPI as well: https://pypi.org/project/adrf/.
There are already two other packages in PyPI that aim to add async support to Django REST Framework: django-rest-framework-async and async-drf. Both wraps Django REST framework code with sync_to_async where needed to allow async views. While using sync_to_async works too, it is not as performant as fully async code, which is the aim of adrf.
Any feedback would be highly appreciated!
I just released a separate package to implement the async views support for Django REST framework following the approach of this PR. Here is the link to the code: https://github.com/em1208/adrf. The package has been published in PyPI as well: https://pypi.org/project/adrf/.
There are already two other packages in PyPI that aim to add async support to Django REST Framework: django-rest-framework-async and async-drf. Both wraps Django REST framework code with
sync_to_asyncwhere needed to allow async views. While usingsync_to_asyncworks too, it is not as performant as fully async code, which is the aim ofadrf.Any feedback would be highly appreciated!
that is great. IMHO, async support in django is not yet a first class/complete from developers and technical points of view. so we should still wait for further django releases and try async drf as a 3rd party extension for some time.
Next step would be to add this to the docs.
Adding "Third Party Packages" section to https://www.django-rest-framework.org/api-guide/views/ would probably make sense, right?
Next step would be to add this to the docs.
Adding "Third Party Packages" section to https://www.django-rest-framework.org/api-guide/views/ would probably make sense, right?
I do agree with this
Bump
Do we expect this to be merged anytime soon, or will adrf be the way to go as for now?
adrf will be the way to go for now
Hi — I'm trying to understand how practical using something like adrf is at the moment. From what I understand, it provides only a base ViewSet that supports async. I don't think the more powerful features of DRF (e.g., ModelViewSet) are supported yet — am I understanding that correctly?
On the surface it seems like supporting ModelViewSet would require changes to a lot of downsteam components of the DRF since all the "magic" of DRF happens in the mixins and backends, and there are a lot of sync assumptions being made in there.
I wonder if someone who has investigated this more deeply has thoughts — please correct me if I'm wrong.
There aren't any technical reasons why a third party package couldn't fulfil this space. If folks really want async view support in REST framework, then that's a route I'd suggest.
So I looked into this a bit and this doesn't really seem like a feasible path forward from what I can tell, there's a lot of plumbing needed to make async work properly in a way that allows for incremental migrations of projects to asyncio drf views. Additionally a 3rd party package would likely introduce dependency hell issues in larger projects.
The issue here is that async code needs to be cross compatible with sync code in a number of places that isn't all that feasible to handle in a 3rd party package as one may be mixing sync and async 3rd party packages in various ways.
From what I was seeing the best approach is to plumb the async view code into drf in a way that makes mixing async/sync code during a migration is feasible.
I made an initial attempt at this in #8978 which also handles async/sync bidirectional cross compatibility for permissions and throttles for sync and async views.
On the surface it seems like supporting
ModelViewSetwould require changes to a lot of downsteam components of the DRF since all the "magic" of DRF happens in the mixins and backends, and there are a lot of sync assumptions being made in there.
I think my granular autodetecting approach is what is needed to make it possible to mix and/or migrate these sync components with/to async.