Fixed #33277 -- Disallowed database connections in threads in SimpleTestCase. #17639
Conversation
David-Wobrock
force-pushed
the
ticket-33277
branch
7 times, most recently
from
28fa0b9
to
f04c37d
Compare
|
@blueyed Does it work for you? |
django/test/testcases.py
Outdated
| # Copied from BaseDatabaseWrapper.ensure_connection as the original as | ||
| # been patched at the base class level. | ||
| if self.connection is None: | ||
| with self.wrap_database_errors: | ||
| self.connect() |
There was a problem hiding this comment.
What about 3rd-party database backends with the custom ensure_connection() implementation?
There was a problem hiding this comment.
Also, the original ensure_connection() is @async_unsafe decorated.
There was a problem hiding this comment.
Of course, copying the source code is a bad idea here 😛
Let's call the actual BaseDatabaseWrapper.ensure_connection method.
Now, the monkey-patching approach means that, as long as we call BaseDatabaseWrapper.ensure_connection, the behaviour should work as expected.
Meaning that the Sentry implementation should keep working I believe.
But if another backend overrides ensure_connection without calling super().ensure_connection(...), then the behaviour remains the same as today: database connections still work in threads.
Would there be another method(s) that we can leverage? 🤔
David-Wobrock
force-pushed
the
ticket-33277
branch
from
f04c37d
to
4d3c61c
Compare
|
Thanks for the review @felixxm 🙏 I pushed some changes in the commit. |
felixxm
changed the title
There was a problem hiding this comment.
@David-Wobrock Thanks 👍
I pushed small edits to a release note, and used enterClassContext().
ticket-33277
Picked up from #17075