Edgewall Software

Changes between Initial Version and Version 1 of Ticket #9536, comment 6


Ignore:
Timestamp:
Sep 14, 2010, 8:42:37 AM (11 years ago)
Author:
Christian Boos

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #9536, comment 6

    initial v1  
    2929We have 1. for the same reason we had `Environment.with_transaction` so that we don't need to import `trac.db.api` from everywhere.
    3030For 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.
     31Now, 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.
    3232
    3333I'd really prefer to inline `DatabaseManager(env).get_connection()` in the context managers and fine a way to make the tests work nevertheless.