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 
   
<https://github.com/django/django/blob/e4a5527d1dc2f8183883931560f3a6dcdef0ab0c/django/contrib/auth/base_user.py#L126>
   - get_many() / a_getmany() in cache/backends/base.py 
   
<https://github.com/django/django/blob/e4a5527d1dc2f8183883931560f3a6dcdef0ab0c/django/core/cache/backends/base.py#L208>

and of course the way I fixed #34717, now we have some code duplication in 
aget_object_or_404 
<https://github.com/olibook/django/blob/b9473cac65190822e7c94f695f1f7b4d5b49502a/django/shortcuts.py#L92>
 
/ aget_list_or_404 
<https://github.com/olibook/django/blob/b9473cac65190822e7c94f695f1f7b4d5b49502a/django/shortcuts.py#L134C11-L134C27>

As I'm working on #34757, and following this pattern, there would be some 
duplication of the TestClient._handle_redirects 
<https://github.com/olibook/django/blob/a564f44350afc4823b9ff6eb14c56e8793ed4b31/django/test/client.py#L1170>
 
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  
<https://github.com/django/deps/blob/main/accepted/0009-async.rst>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





-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/dfe0f343-9c2a-4d01-9a3f-1b4f7c0d45bfn%40googlegroups.com.

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

[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

 



-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3e860290-7cf3-461d-b595-250c167e53f3n%40googlegroups.com.

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

[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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a911e320-0166-4681-bfd0-c58112b54b63n%40googlegroups.com.