Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider mentioning Django's Async ORM Interface #1999

Closed
johnthagen opened this issue Apr 5, 2023 · 6 comments · Fixed by #2090
Closed

Consider mentioning Django's Async ORM Interface #1999

johnthagen opened this issue Apr 5, 2023 · 6 comments · Fixed by #2090

Comments

@johnthagen
Copy link
Contributor

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
Copy link
Member

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

@johnthagen
Copy link
Contributor Author

johnthagen commented Apr 5, 2023

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 johnthagen changed the title Consider mentioning Django's Async ORM Interface in Tutorial 3 Consider mentioning Django's Async ORM Interface Apr 5, 2023
@johnthagen
Copy link
Contributor Author

johnthagen commented Apr 17, 2023

@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

  • Python 3.10.11
  • Django 4.1.8
  • channels 4.0.0
  • channels-redis 4.1.0
  • pytest 7.3.0
  • pytest-asyncio 0.21.0

@carltongibson
Copy link
Member

Grrr. Ok. Grrr.

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

Thanks!

@johnthagen
Copy link
Contributor Author

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
Copy link
Collaborator

I just found this issue while going through the issue backlog. I’ve put up #2090 to document the API!

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 a pull request may close this issue.

3 participants