Changes between Initial Version and Version 1 of Ticket #9536, comment 6
- Timestamp:
- Sep 14, 2010, 8:42:37 AM (14 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #9536, comment 6
initial v1 29 29 We have 1. for the same reason we had `Environment.with_transaction` so that we don't need to import `trac.db.api` from everywhere. 30 30 For 2. it's fine to have the context managers themselves in `trac.db.api`. 31 Now, 3. is problematic: at least we should rename it to `_get_db_cnx` to discourage its use (would that be enough? people would also do the same rename and curse us ;-) ). The interest of having to go through the !Environment here as opposed to inline directly `DatabaseManager(env).get_connection()` is that the **tests** (and more precisely the `EnvironmentStub`) do some hacking at this level in order to reuse one single !DatabaseManager instance across all test environments, especially useful when dealing with for PostgreSQL and MySQL backend.31 Now, 3. is problematic: at least we should rename it to `_get_db_cnx` to discourage its use (would that be enough? people would also do the same rename and curse us ;-) ). The interest of having to go through the Environment here as opposed to inline directly `DatabaseManager(env).get_connection()` is that the **tests** (and more precisely the `EnvironmentStub`) do some hacking at this level in order to reuse one single !DatabaseManager instance across all test environments, especially useful when dealing with for PostgreSQL and MySQL backend. 32 32 33 33 I'd really prefer to inline `DatabaseManager(env).get_connection()` in the context managers and fine a way to make the tests work nevertheless.