[Django] #19031: Using @override_settings doesn't override DATABASES in threads +SQLite
Django
-----------------------------------+--------------------
Reporter: jonash | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
I'm aware of the fact that SQLite's `:memory:` mode doesn't work with
threads, so I wanted to override `TEST_NAME` using `override_settings` to
make SQLite use a filesystem DB for a single test:
{{{
from django.test import TransactionTestCase
from django.db import models
from django.test.utils import override_settings
from threading import Thread
class OverrideDATABASESTest(TransactionTestCase):
@override_settings(DATABASES={'default': {'BACKEND':
'django.db.backends.sqlite3', 'TEST_NAME': 'test-db'}})
def test_override_DATABASES(self):
t = Thread(target=SomeModel.objects.get)
t.start()
t.join()
}}}
This doesn't work with threads, it fails with a `DatabaseError: no such
table: app_modelname` exception in the thread. The test works if I set
`TEST_NAME` in my `settings.py` though, so this seems like an issue with
`override_settings`.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jonash | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 1.4
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 akaariai):
* status: new => closed
* needs_docs: => 0
* resolution: => wontfix
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
I don't think we should claim that you can switch the used database in-
flight and expect everything to work. My guess is that the problem here is
no syncdb ran for the DB. You could try if things work if you run syncdb
manually in setUp().
Wontfixing this, as ensuring that the database is set up correctly, and is
also torn down correctly seems hard to do.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:1>
Django
Reporter: jonash | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 1.4
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jonash):
In that case, it might be useful to have some sort of warning or exception
if someone tries to override the `DATABASE` setting. Or a note in the docs
-- along with all the other settings that shouldn't/can't be overriden (if
any exist).
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:2>
Django
Reporter: jonash | Owner: nobody
Type: Bug | Status: reopened
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+------------------------------------
Changes (by claudep):
* status: closed => reopened
* resolution: wontfix =>
* component: Testing framework => Documentation
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I do agree with Anssi that overriding DATABASE is too much for
override_settings. However, I also agree that this could be documented.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:3>
Django
Reporter: jonash | Owner: nobody
Type: Bug | Status: reopened
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Architekt):
I don't think that overriding DATABASE settings is too much. It depends on
what do you expect from it. I can imagine proper use cases for it.
In the docs is described that the test database is created before running
tests. And every time you run tests you see in the output: Creating test
database... -> runing tests -> Destoying test database... This should be
enough to understand that overriding DATABASE will not change test
database in the middle of running tests.
Overriding settings is always tricky. Situation described in this ticket
is just one case out of many others. I think this particular situation
does not need any special documentation.
jonash: Right solution for your problem is skippping the test for SQLite's
:memory: database.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:4>
Django
Reporter: jonash | Owner: nobody
Type: Bug | Status: reopened
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by jonash):
It's not since then I either need to run the tests twice (with `:memory:`
and a real database) or my test is not run at all.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:5>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
* owner: nobody => joeri
* status: new => assigned
Comment:
Will write some documentation for this, and also add a warning when using
``override_settings`` with these complex settings.
Will also pull in #20075 (cache settings) regarding the above.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:7>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by joeri):
* cc: joeri (added)
* has_patch: 0 => 1
Comment:
Just added a pull request: https://github.com/django/django/pull/1095
So, {{{override_settings}}} provides a way to change settings in your
Django settings. However, some settings are only accessed during the
initialization of your project. For example, the DATABASES and CACHES
settings are read and cached by Django and Django internals only use these
cached objects (and not read your settings file over and over again).
Still, overriding the DATABASE setting with {{{override_settings}}} does
change the DATABASE setting when accessed via {{{django.conf.settings}}}.
This override however has no effect in practice. I call this unexpected
behaviour because you probably expected that your database backend
actually changed.
In addition to documenting these settings that show unexpected behaviour,
I also added a {{{UserWarning}}} when people try to change these settings.
I specifically not raise an exception because you might want to test
something not related to what Django does internally (although you
probably do).
Note that this also introduces the interesting fact that the Django test
suite shows this warning. This actually related to another ticket #20075
where this problem is described.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:8>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by aaugustin):
The `setting_changed` signal is designed to catch such changes and clear
whatever caches may exist in Django. I would prefer to raise the warning
in a `setting_changed` listener, for consistency.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:9>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by joeri):
Sounds good. However, this will trigger the warning twice since this
signal is also given when leaving the context. Unless there is a way to
check if its change/change back signal.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:10>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by aaugustin):
As discussed together it would make sense to add a boolean argument to the
``setting_changed`` signal to tell entering the block from exiting it.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:11>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by joeri):
Changed pull request as per our discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:12>
Django
Reporter: jonash | Owner: joeri
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timo):
* component: Documentation => Testing framework
Comment:
Updating component since the patch is more than just documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:13>
Django
CACHES
-----------------------------------+------------------------------------
Reporter: jonash | Owner: joeri
Component: Testing framework | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by timo):
* cc: timograham@… (added)
* needs_better_patch: 0 => 1
* type: Bug => New feature
Comment:
Left some comments on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:14>
Django
CACHES
-----------------------------------+------------------------------------
Reporter: jonash | Owner: joeri
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
Comment (by timo):
Was looking into editing the pull request with my comments and noticed
this causes the Django test suite to throw a warning because
`django/contrib/sessions/tests.py` calls `@override_settings` with
`CACHES`. Not sure how we should handle this, since it appears `CACHES`
can apparently be overridden successfully, at least in some cases?
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:15>
Django
Reporter: jonash | Owner: joeri
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
* needs_better_patch: 1 => 0
* version: 1.4 => master
Comment:
On further investigating of #20075, `CACHES` can be overridden but there
are some caveats. I've updated the proposed documentation change with
these details and removed `CACHES` from generating a warning. New
[https://github.com/django/django/pull/1348 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:16>
Django
-----------------------------------+------------------------------------
Reporter: jonash | Owner: joeri
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"66f3d57b79eee0381c29ee4c76582d6b182bfad9"]:
{{{
#!CommitTicketReference repository=""
revision="66f3d57b79eee0381c29ee4c76582d6b182bfad9"
Fixed #19031 -- Added a warning when using override_settings with
'DATABASES'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:17>
Django
-----------------------------------+------------------------------------
Reporter: jonash | Owner: joeri
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"eabc3b6c8dd67e9ff49da9f2f41cc653898cd0a1"]:
{{{
#!CommitTicketReference repository=""
revision="eabc3b6c8dd67e9ff49da9f2f41cc653898cd0a1"
Set stacklevel for the override_settings warning.
Refs #19031.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19031#comment:18>