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

[Christian Boos]

(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​

[Thijs Triemstra]

patch against 0.12-stable r10371

[Thijs Triemstra]

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

[Thijs Triemstra]

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.

[Thijs Triemstra]

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

[Remy Blank]

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?

[Remy Blank]

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

[Thijs Triemstra]

against 0.12-stable r10394

[Thijs Triemstra]

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.

[Thijs Triemstra]

against 0.13-trunk r10394

[Thijs Triemstra]

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.

[Christian Boos]

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

[Remy Blank]

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?

[Thijs Triemstra]

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.

[Thijs Triemstra]

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

[Remy Blank]

Good catch! Fixed in [10410].

[Thijs Triemstra]

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

[Remy Blank]

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.

[Thijs Triemstra]

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.

[Christian Boos]

Lower the severity of these MySQL enhancements as other major enhancements are really the highlights of this release.