close_if_unusable_or_obsolete should skip connections in atomic block for autocommit check

[Daniel Hahler]

Via ​https://github.com/django/channels/issues/1091#issuecomment-489488831 I have noticed that close_if_unusable_or_obsolete will close the DB connection used in tests, which has entered an atomic block (via TestCase).

channel's DatabaseSyncToAsync calls close_if_unusable_or_obsolete here.

The following patch might make sense:

diff --git c/django/db/backends/base/base.py i/django/db/backends/base/base.py
index 9fa03cc0ee..f9ca0f8464 100644
--- c/django/db/backends/base/base.py
+++ i/django/db/backends/base/base.py
@@ -497,7 +497,10 @@ def close_if_unusable_or_obsolete(self):
         if self.connection is not None:
             # If the application didn't restore the original autocommit setting,
             # don't take chances, drop the connection.
-            if self.get_autocommit() != self.settings_dict['AUTOCOMMIT']:
+            if (
+                    not self.in_atomic_block and
+                    self.get_autocommit() != self.settings_dict["AUTOCOMMIT"]
+            ):
                 self.close()
                 return
 

[Simon Charette]

close_if_unusable_or_obsolete is certainly an internal API but I agree that not considering self.in_atomic_block in the check against settings_dict["AUTOCOMMIT"] is way too naive to determine the connection is unusable or obsolete.

[Daniel Hahler]

Created ​https://github.com/django/django/pull/11769.

but I agree that not considering self.in_atomic_block in the check against settings_dictAUTOCOMMIT is way too naive to determine the connection is unusable or obsolete.

Not sure I understand that correctly: do you think the patch makes sense, or is still to naive?

[Jacob Walls]

​PR