[Django] #35226: Copying a connection with a new alias fails

[Django]

#35226: Copying a connection with a new alias fails
---------------------------------------------+------------------------
Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 5.0
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
While working on connection pooling for postgres I ran into the following
regression (since
https://github.com/django/django/commit/8fb0be3500cc7519a56985b1b6f415d75ac6fedb
):

{{{
from django.db import connection
from django.test import TestCase


class Test(TestCase):
def test_something(self):
new_connection = connection.copy("asd")
with new_connection.cursor() as cursor:
print(cursor.execute("SELECT 1").fetchall())
}}}

On 5.0 this works fine, on main this fails with:
> django.test.testcases.DatabaseOperationForbidden: Database threaded
connections to 'asd' are not allowed in this test. Add 'asd' to
test_regression.Test.databases to ensure proper test isolation and silence
this failure.

I cannot really add this to `databases` since this is a dynamically
created database. I am opening this as release blocker so it doesn't get
lost (I might be holding it wrong though)
--
Ticket URL: <https://code.djangoproject.com/ticket/35226>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35226: Copying a connection with a new alias fails

-----------------------------------+--------------------------------------

Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 5.0

Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------------+--------------------------------------

Old description:

> While working on connection pooling for postgres I ran into the following
> regression (since
> https://github.com/django/django/commit/8fb0be3500cc7519a56985b1b6f415d75ac6fedb
> ):
>
> {{{
> from django.db import connection
> from django.test import TestCase
>

> class Test(TestCase):
> def test_something(self):
> new_connection = connection.copy("asd")
> with new_connection.cursor() as cursor:
> print(cursor.execute("SELECT 1").fetchall())
> }}}
>
> On 5.0 this works fine, on main this fails with:
> > django.test.testcases.DatabaseOperationForbidden: Database threaded
> connections to 'asd' are not allowed in this test. Add 'asd' to
> test_regression.Test.databases to ensure proper test isolation and
> silence this failure.
>
> I cannot really add this to `databases` since this is a dynamically
> created database. I am opening this as release blocker so it doesn't get
> lost (I might be holding it wrong though)

New description:

While working on connection pooling for postgres I ran into the following

regression (since 8fb0be3500cc7519a56985b1b6f415d75ac6fedb):

{{{
from django.db import connection
from django.test import TestCase


class Test(TestCase):
def test_something(self):
new_connection = connection.copy("asd")
with new_connection.cursor() as cursor:
print(cursor.execute("SELECT 1").fetchall())
}}}

On 5.0 this works fine, on main this fails with:
> django.test.testcases.DatabaseOperationForbidden: Database threaded
connections to 'asd' are not allowed in this test. Add 'asd' to
test_regression.Test.databases to ensure proper test isolation and silence
this failure.

I cannot really add this to `databases` since this is a dynamically
created database. I am opening this as release blocker so it doesn't get
lost (I might be holding it wrong though)

--

Comment (by Mariusz Felisiak):

TBH, I don't see how we could handle this and keep
8fb0be3500cc7519a56985b1b6f415d75ac6fedb. The main question is, do we want
to support something like this? Creating a copy of the connection copy is
tricky, and if you change the alias in the meantime it becomes even more
complicated.
--
Ticket URL: <https://code.djangoproject.com/ticket/35226#comment:1>

[Django]

#35226: Copying a connection with a new alias fails
-----------------------------------+--------------------------------------
Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------

Comment (by Florian Apolloner):

Well I will see if I can get around it without an alias change in the pool
tests, but testing the pooling stuff is tricky without creating completely
new connections :D
--
Ticket URL: <https://code.djangoproject.com/ticket/35226#comment:2>

[Django]

#35226: Copying a connection with a new alias fails
-----------------------------------+--------------------------------------
Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------

Comment (by Mariusz Felisiak):

We could ignore dynamically created connections, e.g.:
{{{#!diff
diff --git a/django/test/testcases.py b/django/test/testcases.py
index 51b07ae50d..768ebd7d52 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -280,6 +280,7 @@ class SimpleTestCase(unittest.TestCase):
self.connection is None
and self.alias not in cls.databases
and self.alias != NO_DB_ALIAS
+ and self.alias in connections
):
# Connection has not yet been established, but the alias
is not allowed.
message = cls._disallowed_database_msg % {

}}}

What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/35226#comment:3>

[Django]

#35226: Copying a connection with a new alias fails
-----------------------------------+--------------------------------------
Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------

Comment (by Florian Apolloner):

That seems like a workable solution to me if it doesn't conflict with what
you have been trying to achieve in the first place 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/35226#comment:4>