#17760 closed Bug (fixed)
connection.features.supports_transactions is None
Reported by: | Craig de Stigter | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Background: To speed up our tests we have a custom nose-based test runner which *doesn't* drop/create the database. It shouldn't need to when tests are run in a transaction. This prevents us having to load fixtures etc, as most of the test data lives permanently in the database.
However after upgrading to django 1.3, connection.features.supports_transactions
is always None, causing our tests to use TRUNCATEs instead of transactions.
This appears to be because connection.features.confirm() is only called from create_test_db(): https://code.djangoproject.com/browser/django/tags/releases/1.3/django/db/backends/creation.py#L357
I'd argue that's the wrong place to do it. It should probably happen when the connection is first initialised, or at least in TestCase._pre_setup()
This has caused issues before: #16622 #16885 (possibly others)
(To be fair it shouldn't really be on connection.features at all, since it's only set during testing and is otherwise always None)
Tested with 1.3.1 and master.
Attachments (5)
Change History (14)
comment:1 by , 13 years ago
Has patch: | set |
---|
comment:2 by , 13 years ago
I think the connection.features.supports_transactions should be a cached property. It checks the support when first accessed, later on it is just a normal boolean. That way there would be no need to run features.confirm() at all, supports transactions is always usable.
I haven't verified the issue, so the above is more a general note...
comment:3 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I just translated akaariai's comment:2 into a patch. Not sure if something should be tested here.
comment:4 by , 13 years ago
There is a @cached_property decorator at django/utils/functional.py, it is meant for situations like this. Maybe it would make the implementation a bit cleaner.
You could perhaps test that no queries are made after first use of the property, but if you use the @cached_property decorator, I don't know how necessary that is. It might be better to write a test for the @cached_property decorator, and then trust that it does the right thing. In short, I don't know if there is any need for additional tests.
comment:5 by , 13 years ago
Thanks for the hint Anssi, it's indeed nicer with the cached_property decorator.
comment:6 by , 13 years ago
FWIW I recently committed a patch to south to use the same approach for DDL transaction support detection and it works quite well.
comment:7 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:8 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
initial fix: https://github.com/koordinates/django/commit/66de288e80377e645369d56e44fcdb02dbed0276