[patch] DISABLE_TRANSACTION_MANAGEMENT is not working as described in Doc's

[scott.benninghoff@…]

DISABLE_TRANSACTION_MANAGEMENT switch from the settings.py file doesn't do anything to the functions in transactions.py. I debugged the code yesterday and no matter what settings I had, .save() method for various models would either commit changes or raise the error about pending commits.

Created a patch against the function set_dirty() that evaluates the settings.DISABLE_TRANSACTION_MANAGEMENT for being True and if so, simply returns out of the function.

[scott.benninghoff@…]

[patch] against ticket 2304

[Gary Wilson <gary.wilson@…>]

Please include the patch in diff format.

[sam@…]

Here is a unified diff of the attached transaction.py against django/db/transaction.py from SVN revision 3297 (which was current at 2006-07-07 23:39:43):

--- django/db/transaction.py	2007-07-20 19:25:54.000000000 +0100
+++ /dev/stdin	2007-07-20 19:26:43.430521786 +0100
@@ -82,7 +82,17 @@
     Sets a dirty flag for the current thread and code streak. This can be used
     to decide in a managed block of code to decide whether there are open
     changes waiting for commit.
-    """
+    ...
+    Patch: DISABLE_TRANSACTION_MANAGEMENT puts the developer in control
+    of the Unit of Work.  This will ignore the framework of the changes and allow
+    the developer to make the determination of when to commit and rollback as well
+    as assume the responsibility.  This code was not working previous to this change
+    """
+    if (settings.DISABLE_TRANSACTION_MANAGEMENT):
+        return
+    
+    # End of changes
+    
     thread_ident = thread.get_ident()
     if dirty.has_key(thread_ident):
         dirty[thread_ident] = True

Not much there...

[Thejaswi Puthraya]

added the patch wrt r7277

[Carl Meyer]

This patch has been sitting around for quite a while. Meanwhile, the ​documentation still claims DISABLE_TRANSACTION_MANAGEMENT works, when in fact it does nothing. If this patch isn't ready to go in, would it be worth getting a patch in temporarily to remove the documentation?

[henrybaxter]

I agree, it's pretty annoying to go through the process of figuring out the setting does nothing.

[Karen Tracey]

The existing patch is not the correct solution.

Per Jacob's response in this thread: ​http://groups.google.com/group/django-developers/browse_thread/thread/b633d56fdc7d4107/

the problem is not that the function isn't implemented, but rather that that docs and code do not agree on the name for the setting that controls the function. The code uses a setting named TRANSACTIONS_MANAGED (default value False in django/conf/global_settings.py), not a setting named DISABLE_TRANSACTION_MANAGEMENT. The fix, I believe, is a simple global replace in either the code or the doc of one for the other, depending on which one is chosen to keep.

DISABLE_TRANSACTION_MANAGEMENT is the one I would keep. It seems to more clearly convey that a setting of True means Django will not manage transactions. I found the actual default value for TRANSACTIONS_MANAGED to be surprising when I checked on it, because I expected it to reflect whether Django code was responsible for transaction management. In fact its apparently the reverse of what I expected based just on the name (I did not look at the code that uses it, just noted that there is in fact code that references its value, unlike DISABLE_TRANSACTION_MANAGEMENT).

Note neither name is documented in the full list of settings, so it might be good to add doc there as well when this is done.

[tjshewmake]

Proposed fix of manual transactions variable name.

[Aymeric Augustin]

#16492 was a duplicate.

[Łukasz Rekucki]

Patch doesn't apply and it's probably too late to deprecate it in 1.4

[Jeremy Dunck]

This flag has never worked the way it's been documented, while TRANSACTIONS_MANAGED does - probably there is deployed code depending upon TRANSACTIONS_MANAGED, though it is undocumented.

I don't see the point of deprecating TRANSACTIONS_MANAGED while keeping the documented but useless DISABLE_TRANSACTION_MANAGEMENT.

I'd prefer to document TRANSACTIONS_MANAGED and remove DISABLE_TRANSACTION_MANAGEMENT from docs; it's not even a backwards-compatibility issue because people can still happily set the useless (but newly undocumented) flag. - Which is to say, I agree w/ kmtracey of 3 years ago. I'll provide an updated docs patch shortly.

[Karen Tracey]

Well three years ago I preferred to keep the documented name and change the code to match, but at this point just correcting the doc to match the existing code seems better. There likely are people who have found (and thus use) the undocumented name and we shouldn't break them by fixing this.

[Aymeric Augustin]

Since this setting is only useful in extreme circumstances, it doesn't make much sense to go through a deprecation path just to improve its name.

I agree that we should just document the current name. That's what I suggested in #16492 some time ago.

[Aymeric Augustin <aymeric.augustin@…>]

In a001f3c31e751e6ab1622ae7f65f7f8aeca2ef9a:

Fixed #2304 -- Documented TRANSACTIONS_MANAGED.

[Aymeric Augustin <aymeric.augustin@…>]

In 44e56238d79890176a747181d32fbc53be157706:

[1.5.x] Fixed #2304 -- Documented TRANSACTIONS_MANAGED.

Backport of a001f3c.