connection.features.supports_transactions is None

[Craig de Stigter]

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.

[Craig de Stigter]

initial fix: ​https://github.com/koordinates/django/commit/66de288e80377e645369d56e44fcdb02dbed0276

[Anssi Kääriäinen]

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

[Claude Paroz]

Replace confirm() by cached properties

[Claude Paroz]

I just translated akaariai's comment:2 into a patch. Not sure if something should be tested here.

[Anssi Kääriäinen]

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.

[Claude Paroz]

Using the cached_property decorator

[Claude Paroz]

Thanks for the hint Anssi, it's indeed nicer with the cached_property decorator.

[Craig de Stigter]

minor bug fix for 17760-2.diff

[Simon Charette]

FWIW I recently committed a patch to south to use the same approach for ​DDL transaction support detection and it works quite well.

[Ramiro Morales]

Fixed problem with detction of STDDEV on sqlite3

[Claude Paroz]

Updated to current trunk

[Claude Paroz <claude@…>]

In [aa423575e7b464433fcfc4bf4b8e1d7627b17ce6]:

Fixed #17760 -- Implemented callable database features as cached properties

This does remove the requirement to call features.confirm() method
before checking the properties.
Thanks cdestiger and Ramiro Morales for their work on the patch.

[Claude Paroz <claude@…>]

In [ad47364dd324508e8332ea853da59772431398aa]:

Reverted 905e33f, now that DatabaseFeatures does not need confirm

Connection.features does not need to be confirmed any more, after
commit aa42357, rendering obsolete the workaround when using
TEST_MIRROR (Refs #16885, #17760).