Adding "reason" to django.shortcuts.redirect

[Skrattoune]

Hi,

I've recently discovered the possibility to add a `reason` to a redirect.

It's extremely useful for testing or debugging when a redirect to a same 
page can have different causes.

I'm proposing to add this functionality to `django.shortcuts.redirect` :

def redirect(to, *args, reason=None, permanent=False, **kwargs):
"""
Return an HttpResponseRedirect to the appropriate URL for the arguments
passed.

The arguments could be:

* A model: the model's `get_absolute_url()` function will be called.

* A view name, possibly with arguments: `urls.reverse()` will be 
used
  to reverse-resolve the name.

* A URL, which will be used as-is for the redirect location.

Issues a temporary redirect by default; pass permanent=True to issue a
permanent redirect.

a redirect reason can be specified using the `reason` optional parameter
"""
redirect_class = HttpResponsePermanentRedirect if permanent else 
HttpResponseRedirect
return redirect_class(resolve_url(to, *args, **kwargs), reason=reason)

It does not affect any previous implementation.
only impact could be if somebody has decided to pass on to a specific page 
the kwarg `reason`

which is extremely minimal risk, and should be anyway easily detected 
through already present testing

Do I have go ahead to propose a PR?

-- 
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/71d5d7a0-da83-468b-8abb-96478fdcc0e4n%40googlegroups.com.

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

[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.

On Sat, Aug 5, 2023 at 8:45 PM Jon Janzen  wrote:

> 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
> On Saturday, August 5, 2023 at 10:23:00 AM UTC-7 Olivier Tabone wrote:
>
>> 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
>>
>>
>>
>>
>>
>> --
> 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/be3a9dba-2702-4fbd-baef-c49587892616n%40googlegroups.com
> 
> .
>

-- 
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/CA%2Bniz3jdeob-E

Re: Adding "reason" to django.shortcuts.redirect

['Adam Johnson' via Django developers (Contributions to Django itself)]

This sounds reasonable, but unfortunately, it's backwards incompatible and
so we cannot make the change. Any URL with a “reason” URL parameter would
no longer be resolvable.

You can make a project-specific shortcut function that allows providing the
reason parameter. Since it’s two lines long, it’s not a huge burden to add.

On Mon, Aug 7, 2023 at 10:48 AM Skrattoune  wrote:

> Hi,
>
> I've recently discovered the possibility to add a `reason` to a redirect.
>
> It's extremely useful for testing or debugging when a redirect to a same
> page can have different causes.
>
> I'm proposing to add this functionality to `django.shortcuts.redirect` :
>
> def redirect(to, *args, reason=None, permanent=False, **kwargs):
> """
> Return an HttpResponseRedirect to the appropriate URL for the arguments
> passed.
>
> The arguments could be:
>
> * A model: the model's `get_absolute_url()` function will be
> called.
>
> * A view name, possibly with arguments: `urls.reverse()` will be
> used
>   to reverse-resolve the name.
>
> * A URL, which will be used as-is for the redirect location.
>
> Issues a temporary redirect by default; pass permanent=True to issue a
> permanent redirect.
>
> a redirect reason can be specified using the `reason` optional
> parameter
> """
> redirect_class = HttpResponsePermanentRedirect if permanent else
> HttpResponseRedirect
> return redirect_class(resolve_url(to, *args, **kwargs), reason=reason)
>
> It does not affect any previous implementation.
> only impact could be if somebody has decided to pass on to a specific page
> the kwarg `reason`
>
> which is extremely minimal risk, and should be anyway easily detected
> through already present testing
>
> Do I have go ahead to propose a PR?
>
> --
> 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/71d5d7a0-da83-468b-8abb-96478fdcc0e4n%40googlegroups.com
> 
> .
>

-- 
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/CAMyDDM2PNsHB3OOxS18MJQncN0NkQu-bdYE8Pua%2BhazgOemp4Q%40mail.gmail.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.

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

[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

-- 
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/3fc1d122-94e5-4e58-a824-8ad1235399acn%40googlegroups.com.

Disabling dark / light mode UI toggle

[Niccolò Mineo]

There is a specific situation in which I wouldn't want the user to be able 
to toggle between dark and light mode for the admin theme, and that is if I 
heavily customized it. I haven't found a way to disable that specific 
toggle, so I suggest that an *AdminSite.ui_toggle* (or the like) boolean be 
added. What do you think?

-- 
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/59b387d8-12cb-444c-8b0d-2e595c58ba46n%40googlegroups.com.

Re: Adding "reason" to django.shortcuts.redirect

['Lily Foote' via Django developers (Contributions to Django itself)]

> it's backwards incompatible and so we cannot make the change. Any URL with a 
> “reason” URL parameter would no longer be resolvable.

I think if we decided this feature was sufficiently worthwhile, we could find a 
way to overcome the backward compatibility problem (a deprecation cycle, with 
perhaps a temporary setting or a new function with the extra functionality).

That said, I'm not clear on what the potential benefits of this change actually 
are, and I suspect they don't reach the high barrier to justify the extra 
hassle here.

Sent with [Proton Mail](https://proton.me/) secure email.

--- Original Message ---
On Monday, August 7th, 2023 at 14:09, 'Adam Johnson' via Django developers 
(Contributions to Django itself)  wrote:

> This sounds reasonable, but unfortunately, it's backwards incompatible and so 
> we cannot make the change. Any URL with a “reason” URL parameter would no 
> longer be resolvable.
>
> You can make a project-specific shortcut function that allows providing the 
> reason parameter. Since it’s two lines long, it’s not a huge burden to add.
>
> On Mon, Aug 7, 2023 at 10:48 AM Skrattoune  wrote:
>
>> Hi,
>>
>> I've recently discovered the possibility to add a `reason` to a redirect.
>>
>> It's extremely useful for testing or debugging when a redirect to a same 
>> page can have different causes.
>>
>> I'm proposing to add this functionality to `django.shortcuts.redirect` :
>>
>> def redirect(to, *args, reason=None, permanent=False, **kwargs):
>> """
>> Return an HttpResponseRedirect to the appropriate URL for the arguments
>> passed.
>>
>> The arguments could be:
>>
>> * A model: the model's `get_absolute_url()` function will be called.
>>
>> * A view name, possibly with arguments: `urls.reverse()` will be used
>> to reverse-resolve the name.
>>
>> * A URL, which will be used as-is for the redirect location.
>>
>> Issues a temporary redirect by default; pass permanent=True to issue a
>> permanent redirect.
>>
>> a redirect reason can be specified using the `reason` optional parameter
>> """
>> redirect_class = HttpResponsePermanentRedirect if permanent else 
>> HttpResponseRedirect
>> return redirect_class(resolve_url(to, *args, **kwargs), reason=reason)
>>
>> It does not affect any previous implementation.
>> only impact could be if somebody has decided to pass on to a specific page 
>> the kwarg `reason`
>>
>> which is extremely minimal risk, and should be anyway easily detected 
>> through already present testing
>>
>> Do I have go ahead to propose a PR?
>>
>> --
>> 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/71d5d7a0-da83-468b-8abb-96478fdcc0e4n%40googlegroups.com.
>
> --
> 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/CAMyDDM2PNsHB3OOxS18MJQncN0NkQu-bdYE8Pua%2BhazgOemp4Q%40mail.gmail.com.

-- 
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/WKaaJBvTePkT-bTTQjpBV6KtiYiNUT_Vmud99WI4SSyEIeGNYEG9v_xBRftUXFGHXUkEc7O5BtCKI8CsIQ8BeQDZaG3xji-SR2q0ctbZOaI%3D%40lilyf.org.

Re: Disabling dark / light mode UI toggle

[Niccolò Mineo]

* I haven't found a convenient way, that is.

Il giorno lunedì 7 agosto 2023 alle 23:26:11 UTC+2 Niccolò Mineo ha scritto:

> There is a specific situation in which I wouldn't want the user to be able 
> to toggle between dark and light mode for the admin theme, and that is if I 
> heavily customized it. I haven't found a way to disable that specific 
> toggle, so I suggest that an *AdminSite.ui_toggle* (or the like) boolean 
> be added. What do you think?

-- 
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/7f09013b-02db-40bf-928f-83a737dec069n%40googlegroups.com.

Re: Disabling dark / light mode UI toggle

['Adam Johnson' via Django developers (Contributions to Django itself)]

You should be able to disable the button by creating an empty
admin/color_theme_toggle.html template in your project.

On Mon, Aug 7, 2023 at 10:28 PM Niccolò Mineo 
wrote:

> * I haven't found a convenient way, that is.
>
> Il giorno lunedì 7 agosto 2023 alle 23:26:11 UTC+2 Niccolò Mineo ha
> scritto:
>
>> There is a specific situation in which I wouldn't want the user to be
>> able to toggle between dark and light mode for the admin theme, and that is
>> if I heavily customized it. I haven't found a way to disable that specific
>> toggle, so I suggest that an *AdminSite.ui_toggle* (or the like) boolean
>> be added. What do you think?
>
> --
> 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/7f09013b-02db-40bf-928f-83a737dec069n%40googlegroups.com
> 
> .
>

-- 
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/CAMyDDM3wTanYZoO-3Rhyb8BbkCf1jo0G3Tp982kVEW0K%2BN2EzQ%40mail.gmail.com.