Edgewall Software
Modify

Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#11415 closed enhancement (fixed)

Trap exceptions from invalid database connection string

Reported by: Ryan J Ollos Owned by: Jun Omae
Priority: normal Milestone: 1.0.6
Component: general Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
  • Trap various exceptions from parsing invalid database connection strings.
  • Fix raising AttributeError if SQLite database is unavailable and Babel is available.
API Changes:
Internal Changes:

Description

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.

Attachments (0)

Change History (16)

comment:1 by Ryan J Ollos, 6 years ago

Status: newassigned

comment:2 by Ryan J Ollos, 6 years ago

Milestone: 1.0.31.0.4

comment:3 by Ryan J Ollos, 6 years ago

Milestone: 1.0.41.0.5

comment:4 by Ryan J Ollos, 6 years ago

Milestone: 1.0.51.0.6

Narrowing focus for 1.0.5.

comment:5 by Jun Omae, 6 years ago

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.

comment:6 by Jun Omae, 6 years ago

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.

Last edited 6 years ago by Jun Omae (previous) (diff)

comment:7 by Ryan J Ollos, 6 years ago

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.

comment:8 by Jun Omae, 6 years ago

Owner: changed from Ryan J Ollos to 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')

in reply to:  8 comment:9 by Jun Omae, 6 years ago

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.

comment:10 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

comment:11 by Jun Omae, 6 years ago

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

comment:12 by Ryan J Ollos, 6 years ago

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

comment:13 by Ryan J Ollos, 6 years ago

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

comment:14 by Ryan J Ollos, 6 years ago

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

in reply to:  13 comment:15 by Jun Omae, 6 years ago

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….

in reply to:  14 comment:16 by Jun Omae, 6 years ago

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].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.