Sync and Async versions of the same function: guidelines for contributors

[Olivier Tabone]

Hi all,

While working on async related tickets (eg #34717, and more recently #34757) I noticed code duplication between sync and async version of same functions:

some examples: (no personal offense ❤️)

• acheck_password /  check_password in base_user.py
• get_many() / a_getmany() in cache/backends/base.py

and of course the way I fixed #34717, now we have some code duplication in aget_object_or_404 / aget_list_or_404

As I'm working on #34757, and following this pattern, there would be some duplication of the TestClient._handle_redirects method to support the async case.

I'm kind of ok when duplicating a 3 lines function. Not that much with a 50 lines function. that could easily become a maintenance burden.

I've read DEP-009 a few times and the plan at the time was (quoted from the "Technical Overview")

Each feature will go through three stages of implementation:

• Sync-only (where it is today)
  
• Sync-native, with an async wrapper
  
• Async-native, with a sync wrapper
  

I was wondering:

1- why do we see almost no sync wrapper (async_to_sync) in django's code base ? Is that a best practice ?

2- what do you think about code duplication of async function ? (please point me to existing threads if the discussion already occurred) What is Ok / What is not ok ? Is there some cleanup to be done ?

Cheers,

- Olivier Tabone

[Jon Janzen]

Hey there,

> Not that much with a 50 lines function. that could easily become a maintenance burden.

> what do you think about code duplication of async function ? (please point me to existing threads if the discussion already occurred)

I flagged this problem in https://forum.djangoproject.com/t/asyncifying-django-contrib-auth-and-signals-and-maybe-sessions/18770

and before that in https://groups.google.com/g/django-developers/c/T8zBnYO78YQ

There doesn't really seem to be a good resolution to this problem right now, unfortunately :(

> why do we see almost no sync wrapper (async_to_sync) in django's code base ? Is that a best practice ?

I can't give an authoritative answer but my intuition is that we are still in the early stages of the process of asyncifying everything and we're at the second bullet point from DEP-009 ("Sync-native, with an async wrapper") and haven't yet gotten started on an async-native implementation with a sync wrapper.

There are a few key things missing from core django before that's possible. Some of which are laid out in that forum thread I linked above, others are listed directly in the DEP like the ORM being fully asyncified (right now it's just an async wrapper around sync code) which is somewhat blocked on async database implementations (psycopg3 being the first one supported).

I don't really have any answers for you, just some more thoughts. Hope that's helpful!

Jon

[Lufafa Joshua]

Hi there, 

Just a quick assumption, code duplication could be the reason  why the follow parameter and the _handle_redirects method was not implemented on the AsyncClient as per the ticket #34757, if my assumption is wrong, no big deal.

Kind regards.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/be3a9dba-2702-4fbd-baef-c49587892616n%40googlegroups.com.

[Olivier Tabone]

Le lundi 7 août 2023 à 14:43:26 UTC+2, Lufafa Joshua a écrit :

Hi there, 

Just a quick assumption, code duplication could be the reason  why the follow parameter and the _handle_redirects method was not implemented on the AsyncClient as per the ticket #34757, if my assumption is wrong, no big deal.

Hi Lufa,

That's probably a pretty good assumption, as it's the first conclusion I came to when looking at the ticket.

I made some progress on #34757 and did my best to limit code duplication. Not ready yet for a PR 

Regards, 

- Olivier

[Olivier Tabone]

Hi Jon,

Thank you for your input. 

Le samedi 5 août 2023 à 19:43:57 UTC+2, Jon Janzen a écrit :

There are a few key things missing from core django before that's possible. Some of which are laid out in that forum thread I linked above, others are listed directly in the DEP like the ORM being fully asyncified (right now it's just an async wrapper around sync code) which is somewhat blocked on async database implementations (psycopg3 being the first one supported).

From my understanding, async ORM calls are wrapped with sync_to_async, and this status-quo will remain for synchronous database libraries, such as psycopg2.

I also understand that the heavy lifting in async_to_sync and sync_to_async wrapper has been implemented and the hard work like thread affinity is managed by these wrapper.

If I try to improve my own work on #34717, a simple change as this one passes all the tests on sqlite and postgres. Any idea of what could break with this kind of changes ?

Cheers

- Olivier

 

[Mariusz Felisiak]

I also understand that the heavy lifting in async_to_sync and sync_to_async wrapper has been implemented and the hard work like thread affinity is managed by these wrapper.

Switching between sync and async context is not free, it can cause a significant performance degradation. As far as I'm aware, using async_to_sync/sync_to_async wrappers is a temporary solution to provide an async interface when we don't have all async elements ready.

Best,
Mariusz

[Adam Johnson]

I think some duplication will always be required, unfortunately. Bridging the two paradigms is necessarily costly as it involve communicating between threads. IMO duplication is worth it to avoid performance regressions for sync code, and to make async code worth using.

I am doubtful there is a way for Django to have a purely async core with a sync wrapper. 

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3fc1d122-94e5-4e58-a824-8ad1235399acn%40googlegroups.com.