Type error in init_db() on environment creation with debug_sql = true

[ebray]

I just started playing around with the new-ish debug_sql option in my development environment, and have found it incredibly useful.

So much so in fact that I enabled it in my global trac.ini while debugging a particular problem. With debug_sql enabled during environment initialization, a 'log' argument is passed to the DB connector's init_db() method. Here's the full traceback:

Traceback (most recent call last):
  File "/mnt/hgfs/home/Source/omnipoint/trunk/OmniPoint/Source/Imports/trac/trac/admin/console.py", line 591, in do_initenv
    options=options)
  File "/mnt/hgfs/home/Source/omnipoint/trunk/OmniPoint/Source/Imports/trac/trac/env.py", line 204, in __init__
    self.create(options)
  File "/mnt/hgfs/home/Source/omnipoint/trunk/OmniPoint/Source/Imports/trac/trac/env.py", line 327, in create
    DatabaseManager(self).init_db()
  File "/mnt/hgfs/home/Source/omnipoint/trunk/OmniPoint/Source/Imports/trac/trac/db/api.py", line 74, in init_db
    connector.init_db(**args)
TypeError: init_db() got an unexpected keyword argument 'log'

This affects all three DB backends. The solution, of course, is to add the 'log' argument to the init_db() method for those components.

But this also raises the point that maybe the signature for the get_connection() and init_db() methods of IDatabaseConnector should be made a little more specific. Maybe something like (path, params={}, log=None), given that all three of the officially supported connectors require those three arguments as a minimum, and that as we see in this case, if one of those is missing from the implementation, there are situations where the whole thing can break.

[Christian Boos]

Good catch, I have effectively forgotten to test environment creation with this option on.

[Remy Blank]

I'll fix that one.

[Remy Blank]

Fixed in [8311]. I have changed the signature of get_connection() and init_db() in IDatabaseConnector to (path, log=None, **kwargs), as path is always present, and log must be supported. params, OTOH, is not always present.

[Christian Boos]

Replying to rblank:

I have changed the signature of get_connection() and init_db() in IDatabaseConnector to (path, log=None, **kwargs), as path is always present, and log must be supported. params, OTOH, is not always present.

I agree with above, but then the actual order of the parameters in the corresponding methods in the various backends should probably be changed as well to match this.

Not that it makes a difference as we only call those methods with ...(**args) currently, but it's better to be consistent.

[Remy Blank]

Replying to cboos:

I agree with above, but then the actual order of the parameters in the corresponding methods in the various backends should probably be changed as well to match this.

I almost did this, and then I thought "it works now, better not break it". Anyway, done in [8316].