django / channels

Developer-friendly asynchrony for Django
https://channels.readthedocs.io
BSD 3-Clause "New" or "Revised" License
6.02k stars 793 forks source link

Consider mentioning Django's Async ORM Interface #1999

Closed johnthagen closed 1 week ago

johnthagen commented 1 year ago

Tutorial 3, section "Rewrite the consumer to be asynchronous" states:

Even if ChatConsumer did access Django models or other synchronous code it would still be possible to rewrite it as asynchronous. Utilities like asgiref.sync.sync_to_async and channels.db.database_sync_to_async can be used to call synchronous code from an asynchronous consumer. The performance gains however would be less than if it only used async-native libraries.

This feels like it was written before the Django async ORM interface. It might be worth somehow mentioning that in Django 4.1+, there is at least an async interface for the ORM. Otherwise, new users might get the feeling like that if they do anything with ORM Models, it will be synchronous anyway and might not realize they could pursue more async in a more natural way with channels and modern versions of Django.

carltongibson commented 1 year ago

I'm happy to look at a draft of this certainly! 👍

johnthagen commented 1 year ago

Similarly in Consumers docs:

If you’re calling any part of Django’s ORM or other synchronous code, you should use a SyncConsumer, as this will run the whole consumer in a thread and stop your ORM queries blocking the entire server.

If you want to call the Django ORM from an AsyncConsumer (or any other asynchronous code), you should use the database_sync_to_async adapter instead. See Database Access for more.

Database Access:

The Django ORM is a synchronous piece of code, and so if you want to access it from asynchronous code you need to do special handling to make sure its connections are closed properly.

johnthagen commented 1 year ago

@carltongibson Before we document support for this, are there any reasons why the async ORM interface wouldn't be supported within the AsyncConsumers?

I started playing around with a pytest-asyncio unit that did some setup with database_sync_to_async in the unit test itself and then within my connect() method, called:

my_model = await MyModel.objects.aget(id=1)

But this started generating errors when the unit test was executed.

self = <django.db.backends.utils.CursorWrapper object at 0x11f5dece0>
sql = 'DROP DATABASE "test_backend_db_2d6babdc-3e85-4f6c-813c-821e0bd638d0"'
params = None
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='__no_db__'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x11f5dece0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
>               return self.cursor.execute(sql)
E               django.db.utils.OperationalError: database "test_db" is being accessed by other users
E               DETAIL:  There is 1 other session using the database.

python3.10/site-packages/django/db/backends/utils.py:87: OperationalError

Database is Postgres.

This only seems to happen when I mix in the Async ORM within the AsyncConsumer. database_sync_to_async calls seem fine. I don't understand the internals, but I was wondering if the way that sync calls are put into threads is compatible between the Async ORM and database_sync_to_async?

I just want to make sure something isn't documented that might have some known issues/limitations/considerations.

Test Environment

carltongibson commented 1 year ago

Grrr. Ok. Grrr.

Let's pause. I will try and have a look, but it's not going to be instant.

Thanks!

johnthagen commented 1 year ago

Let's pause. I will try and have a look, but it's not going to be instant.

Sure thing! No rush at all. I just wanted to report some of the initial findings I had.

bigfootjon commented 2 weeks ago

I just found this issue while going through the issue backlog. I’ve put up https://github.com/django/channels/pull/2090 to document the API!