[Django] #29079: Django settings should not cache user wrapped settings

8 views
Skip to first unread message

Django

unread,
Jan 29, 2018, 5:33:08 AM1/29/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
------------------------------------------------+------------------------
Reporter: Riccardo Di Virgilio | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (Other) | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
django 2.0 and 1.11 are breaking my custom settings.

I'm using settings.configure with a custom object that can change his
properties if the environment is changing.
Starting from 1.11 the function {{{ LazySettings.__getattr__}}} is caching
the values that are returned from the custom object that is provided by
the user.

If all the custom object can do is to provide static values why I should
use a custom object at all instead of a regular module?

In my view is an user needs to use a custom object as settings is because
he needs advanced features that cannot be archived by a normal module.
It's very easy to add cache to all properties in a custom object if this
is behavior the user needs, but is really hard to disable this (if not
impossibile for the way this code is written).

If the staff is strongly against disabling the cache by default, can we at
least add an option to disable it?

thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/29079>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 29, 2018, 9:05:04 AM1/29/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
-------------------------------------+-------------------------------------
Reporter: Riccardo Di | Owner: nobody
Virgilio |

Type: Uncategorized | Status: new
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

You should include a sample project or steps to reproduce the issue.
Ideally, you could also
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#bisecting-a-regression bisect] to find the commit where the
behavior changed.

--
Ticket URL: <https://code.djangoproject.com/ticket/29079#comment:1>

Django

unread,
Jan 29, 2018, 4:13:39 PM1/29/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
-------------------------------------+-------------------------------------
Reporter: Riccardo Di | Owner: nobody
Virgilio |
Type: Uncategorized | Status: new
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Riccardo Di Virgilio):

right ok this is a basic example:


{{{
from django.conf import settings

class CustomSettings(object):

ENVIRONMENT = "production"

@property
def DEFAULT_FROM_EMAIL(self):
if self.ENVIRONMENT == "production":
return "in...@company.com"
return "my-perso...@gmail.com"

obj = CustomSettings()

settings.configure(obj)

print(settings.DEFAULT_FROM_EMAIL, obj.DEFAULT_FROM_EMAIL)
obj.ENVIRONMENT = settings.ENVIRONMENT = "development"
print(settings.DEFAULT_FROM_EMAIL, obj.DEFAULT_FROM_EMAIL)

}}}

the output of this code on django 2.1 is :

{{{
in...@company.com in...@company.com
in...@company.com my-perso...@gmail.com
}}}

my claim is that this property should be computer each time is accessed.

is true that django documentation says that you cannot expect runtime
mutations to work and propagate properly trough django framework, but
still internally I do rely on this behavior: I have internal settings that
are generated for different environments and when I deploy django on
production i'm deploying the settings by mutating the environment string
and computing all properties.

this cannot be done anymore with the new behavior.

--
Ticket URL: <https://code.djangoproject.com/ticket/29079#comment:2>

Django

unread,
Jan 31, 2018, 2:27:12 PM1/31/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
-------------------------------------+-------------------------------------
Reporter: Riccardo Di | Owner: nobody
Virgilio |
Type: Bug | Status: new

Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* cc: Adam (Chainz) Johnson (added)
* type: Uncategorized => Bug


Comment:

The change in behavior is due to #27625
(c1b221a9b913315998a1bcec2f29a9361a74d1ac). I doubt we'll revert the
change to restore an undocumented behavior but I'll cc Adam, the author of
that change, for his input.

--
Ticket URL: <https://code.djangoproject.com/ticket/29079#comment:3>

Django

unread,
Jan 31, 2018, 6:02:00 PM1/31/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
-------------------------------------+-------------------------------------
Reporter: Riccardo Di | Owner: nobody
Virgilio |
Type: Bug | Status: new

Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

Yeah I don't think we should revert as it makes all settings access
faster, noticeably so, as benchmarked by Instagram.

You could always set the setting to an object with dynamic behaviour on
access e.g.

{{{
class DefaultFromEmail:
def __str__(self):
if settings.ENVIRONMENT == 'production':
return 'in...@company.com'
return 'my-perso...@gmail.com'

DEFAULT_FROM_EMAIL = DefaultFromEmail()
}}}

Or use the undocumented, but unlikely to change,
`django.utils.functional.SimpleLazyObject`

--
Ticket URL: <https://code.djangoproject.com/ticket/29079#comment:4>

Django

unread,
Jan 31, 2018, 8:08:29 PM1/31/18
to django-...@googlegroups.com
#29079: Django settings should not cache user wrapped settings
-------------------------------------+-------------------------------------
Reporter: Riccardo Di | Owner: nobody
Virgilio |
Type: Bug | Status: closed

Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* status: new => closed
* resolution: => wontfix


--
Ticket URL: <https://code.djangoproject.com/ticket/29079#comment:5>

Reply all
Reply to author
Forward
0 new messages