[PATCH] mysql_backend should parse parameters

[anonymous]

Currently, mysql_backend.py only passes db, user, passwd, host, and port to MySQLdb. Any additional params are discarded.
This is particularly unfortunate (at least in my case) because a number of MySQL installs do not use the default socket location, yet:

mysql://user:password@domain:3306/database?unix_socket=path/to/socket

fails, even though the backend would easily accept it.

[cboos]

(moving to 0.10.x, along with other mysql related issues)

[dekimsey@…]

Agreed, this is actually kind of important in my opinion as I'd really prefer not to have the mysql connect information stored inside the trac.ini file. The mysql library is able to read a configuration file instead.

I implemented a bit of a hack to get the read_default_file parameter to be passed into the connect call, I might upload a patch of it if I can figure out the correct way to pass the params hash properly to the connect sub by name. (I'm not very familiar with Python, I'm a Perl person :/)

--
Danny

[dekimsey@…]

The only issue I have run into so far in implementing this was that certain parameters cannot be left as None and must either be passed as something valid or not passed at all. Some trial and error and I've worked down which is which.

I simply push those with defined values onto the params hash and then pass that to the connect method. It works for me, I have not tested it with an older mysqldb library than the if block checks for.

--
Danny.

[dekimsey@…]

this is a patch to add parameter support to source:/trunk/trac/db/mysql_backend.py@6299​

[thijstriemstra]

patch against 0.12-stable r10371

[thijstriemstra]

The attached patch passes in the params from the connection string, like unix_socket or use_unicode.

[thijstriemstra]

Replying to dekimsey@…:

Agreed, this is actually kind of important in my opinion as I'd really prefer not to have the mysql connect information stored inside the trac.ini file. The mysql library is able to read a configuration file instead.

Agreed.

[thijstriemstra]

Updated patch also passes in -S to mysqldump when doing a backup.

[rblank]

I'm not sure it makes sense to pass all additional parameters to MySQLdb.connect() as keyword arguments, as the values will always be strings. I would prefer we explicitly process and pass in selected parameters.

What's the list of desired parameters? unix_socket is certainly useful, maybe also compress, named_pipe, init_command and read_default_file?

[rblank]

#9949 asks for a way to connect to a MySQL server via SSL, and suggests using a ssl= parameter to specify the certificates and key.

[thijstriemstra]

against 0.12-stable r10394

[thijstriemstra]

Added a patch for 0.12 but moving the ticket to 0.13 because #8954 also has been applied there. Also going to call it an enhancement.

[thijstriemstra]

against 0.13-trunk r10394

[thijstriemstra]

Replying to rblank:

#9949 asks for a way to connect to a MySQL server via SSL, and suggests using a ssl= parameter to specify the certificates and key.

Attached a patch that's probably not pythonic but it gets the job done ;) Once this ticket is fixed it should be easy to add the additional options for SSL in #9949.

Tested with the following connection string:

mysql://admin:admin@localhost/trac?unix_socket=/tmp/mysql.sock&compress=1&
init_command=SELECT COUNT(name) FROM wiki&read_default_file=/tmp/test.cnf&read_default_group=abc

Also updated the options to mysqldump. Wasn't able to test the named_pipe option because it's windows only. See ​MysqlDb docs.

[cboos]

Looks good, however it lacks a bit of documentation: you should also list and explain the paramters in the doc string.

[rblank]

Slightly simpler (and more pythonic) patch applied in [10407]. The changes were only in the form, not the function. The parameters are documented in the component docstring. I have also added a warning about unknown parameters.

Thijs, would you mind quickly checking that the arguments all work as expected?

[thijstriemstra]

Replying to rblank:

Slightly simpler (and more pythonic) patch applied in [10407]. The changes were only in the form, not the function. The parameters are documented in the component docstring. I have also added a warning about unknown parameters.

Thijs, would you mind quickly checking that the arguments all work as expected?

Thanks for applying the patch, it all seems to work properly here.

[thijstriemstra]

Replying to thijstriemstra:

Thanks for applying the patch, it all seems to work properly here.

I forgot to test hotcopy :-/ It's now throwing:

$ trac-admin . hotcopy /tmp/tester
Hotcopying /Users/thijstriemstra/Sites/projects/opensource/test3 to /tmp/tester ...
Backing up database ...
TracError: mysqldump failed: mysqldump: unknown variable 'defaults-file='

it seems mysqldump wants the defaults-file as it's first argument, which is the reason i didn't use db_params.iteritems() in the patch to ensure that defaults-file was the first param..

[rblank]

Good catch! Fixed in [10410].

[thijstriemstra]

This ticket isnt marked as closed in the trunk​ browser view for some reason..

[rblank]

Replying to thijstriemstra:

This ticket isnt marked as closed in the trunk​ browser view for some reason..

That's because the link in the changeset is to a comment in the ticket (comment:16:ticket:5120), not to the ticket itself, and we seem to miss the strikeout in that case. Which can be considered as a bug.

[thijstriemstra]

Replying to rblank:

That's because the link in the changeset is to a comment in the ticket (comment:16:ticket:5120), not to the ticket itself, and we seem to miss the strikeout in that case. Which can be considered as a bug.

Opened #9981 for that.