Trap exceptions from invalid database connection string

[Ryan J Ollos]

This is a continuation of the work in #11189, attempting to raise helpful error messages for cases that the [trac] database string is invalid.

Proposed changes can be found in log:rjollos.git:t11415.

[Ryan J Ollos]

Narrowing focus for 1.0.5.

[Jun Omae]

Hmmm, I get unit tests failing on PostgreSQL and MySQL after rebasing proposed changes on 1.0-stable.

psycopg2.ProgrammingError: schema "tractest" already exists

make: *** [unit-test] Error 1

_mysql_exceptions.OperationalError: (1050, "Table 'system' already exists")
make: *** [unit-test] Error 1

Revised changes in jomae.git@t11415.2.

[Jun Omae]

Revised again in [9e454a430/jomae.git]. I moved check for existence of SQLite database from api.py to sqlite_backend.py and added tests/sqlite_test.py.

[Ryan J Ollos]

Feel free to re-assign ticket. I'm not sure I'll have time to look at the changes more closely within the next week or so.

[Jun Omae]

Okay. I'll fix it.

I noticed that SQLiteConnection.__init__ already checks existence of database file at tags/trac-1.0.5/trac/db/sqlite_backend.py@:265-266#L252​. It doesn't need to add another check.

I found a rare issue. It can be reproduced with Trac 1.0.5 if SQLite database file is missing (or not permitted to read) and [trac] repository_sync_per_request is empty.

2015-04-18 23:24:17,312 Trac[main] ERROR: can't retrieve session: TimeoutError: Unable to get database connection within 0 seconds. (AttributeError: NullTranslationsBabel instance has no attribute 'isactive')

[Jun Omae]

2015-04-18 23:24:17,312 Trac[main] ERROR: can't retrieve session: TimeoutError: Unable to get database connection within 0 seconds. (AttributeError: NullTranslationsBabel instance has no attribute 'isactive')

Proposed changes in [55657979c/jomae.git].

If SQLiteConnection.__init__ uses Babel's LazyProxy instance for translated messages, trac.db.pool recursively tries to connect database to retrieve user's locale. PooledConnection wrongly is constructed with a tuple instance for the cnx argument. After this, the following AttributeError is raised inside translation.isactive property. As the result, the AttributeError is converted to AttributeError: NullTranslationsBabel instance has no attribute 'isactive'.

Traceback (most recent call last):
  File "/home/jun66j5/src/tracdev/git/trac/util/translation.py", line 183, in isactive
    self.activate(get_locale(), env_path)
  File "/home/jun66j5/src/tracdev/git/trac/web/main.py", line 499, in <lambda>
    translation.make_activable(lambda: req.locale, env.path if env else None)
  File "/home/jun66j5/src/tracdev/git/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/home/jun66j5/src/tracdev/git/trac/web/main.py", line 289, in _get_locale
    preferred = req.session.get('language')
  File "/home/jun66j5/src/tracdev/git/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/home/jun66j5/src/tracdev/git/trac/web/main.py", line 281, in _get_session
    return Session(self.env, req)
  File "/home/jun66j5/src/tracdev/git/trac/web/session.py", line 210, in __init__
    self.get_session(sid)
  File "/home/jun66j5/src/tracdev/git/trac/web/session.py", line 233, in get_session
    super(Session, self).get_session(sid, authenticated)
  File "/home/jun66j5/src/tracdev/git/trac/web/session.py", line 85, in get_session
    """, (sid, int(authenticated))):
  File "/home/jun66j5/src/tracdev/git/trac/db/util.py", line 127, in execute
    cursor = self.cnx.cursor()
  File "/home/jun66j5/src/tracdev/git/trac/db/util.py", line 115, in __getattr__
    return getattr(self.cnx, name)
AttributeError: 'tuple' object has no attribute 'cursor'

I'm going to push these changes.

[Jun Omae]

Committed in [13999-14000] and merged to trunk in [14001].

[Jun Omae]

I get unit tests failing with SQLite database with physical file on trunk. Fixed in [14004].

[Ryan J Ollos]

I felt strange about importing TimeoutError from trac.db.pool in [13957], so I'm glad that [14004] fixed that.

[Ryan J Ollos]

Minor suggested revision to [14000] - we should replace the use of ​deprecated unit test method aliases (#11284).

[Ryan J Ollos]

Test failures have been reported in ​gmessage:trac-users:JHa10tA8za0/kQJmpkFf2nEJ, but unconfirmed so far.

[Jun Omae]

Replying to rjollos:

Minor suggested revision to [14000] - we should replace the use of ​deprecated unit test method aliases (#11284).

The changes [14000] don't seem that deprecated methods are used….

[Jun Omae]

Replying to rjollos:

Test failures have been reported in ​gmessage:trac-users:JHa10tA8za0/kQJmpkFf2nEJ, but unconfirmed so far.

The test failures can be reproduced with root user or via fakeroot utility. The root cause is that os.access() always returns True for root privilege even though a file permission is 0000.

Fixed in [14104] and merged to trunk in [14105].