[Django] #30448: close_if_unusable_or_obsolete should skip connections in atomic block for autocommit check

[Django]

#30448: close_if_unusable_or_obsolete should skip connections in atomic block for
autocommit check
-------------------------------------+-------------------------------------
Reporter: Daniel | Owner: nobody
Hahler |
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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

}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30448>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#30448: close_if_unusable_or_obsolete should skip connections in atomic block for
autocommit check
-------------------------------------+-------------------------------------

Reporter: Daniel Hahler | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/30448#comment:1>

[Django]

#30448: close_if_unusable_or_obsolete should skip connections in atomic block for
autocommit check
-------------------------------------+-------------------------------------

Reporter: Daniel Hahler | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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_dict["AUTOCOMMIT"] 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?

--
Ticket URL: <https://code.djangoproject.com/ticket/30448#comment:2>

[Django]

#30448: close_if_unusable_or_obsolete should skip connections in atomic block for
autocommit check
-------------------------------------+-------------------------------------

Reporter: Daniel Hahler | Owner: Daniel
| Hahler
Type: Bug | Status: assigned

Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* owner: nobody => Daniel Hahler
* status: new => assigned
* has_patch: 0 => 1
* needs_tests: 0 => 1

Comment:

[https://github.com/django/django/pull/11769 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30448#comment:3>

[Django]

#30448: close_if_unusable_or_obsolete should skip connections in atomic block for
autocommit check
-------------------------------------+-------------------------------------

Reporter: Daniel Hahler | Owner: (none)
Type: Bug | Status: new

Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* owner: Daniel Hahler => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/30448#comment:4>