Edgewall Software

Opened 7 years ago

Last modified 4 years ago

#12821 closed enhancement

Add support for pymysql library — at Version 10

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.2
Component: database backend Version:
Severity: normal Keywords: mysql
Cc: Branch:
Release Notes:

PyMySQL is the supported MySQL library, with support dropped for MySQL-python.

API Changes:
Internal Changes:

Description

Changes from #12130 propose to add support for the pymysql library. Tests are passing for me, so I'd suggest we add the changes soon. I'll post proposed changes shortly, cherry-picked from #12130.

Change History (10)

comment:1 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

Proposed changes in [4bacb9a74/rjollos.git].

DONE Add Travis CI and AppVeyor build configurations that use pymysql.

Last edited 7 years ago by Ryan J Ollos (previous) (diff)

comment:2 by Ryan J Ollos, 7 years ago

I omitted something important. Tests pass with the following change:

  • setup.py

    diff --git a/setup.py b/setup.py
    index 633a4d3b3..cd6fa235f 100755
    a b facilities.  
    9595    extras_require = {
    9696        'genshi': ['Genshi>=0.6'],
    9797        'babel': ['Babel>=0.9.5'],
    98         'mysql': ['MySQL-python >= 1.2.2'],
     98        'mysql': ['PyMySQL'],
    9999        'postgresql': ['psycopg2 >= 2.0'],
    100100        'pygments': ['Pygments>=1.0'],
    101101        'rest': ['docutils>=0.3.9'],

I haven't looked closely if or how we can specify "MySQL-python or PyMySQL" for the installation requirement in setup.py.

Also, looks like signature of _show_warnings changed in PyMySQL 0.7.6, and latest package has a strange version string:

>>> import pymysql as MySQLdb
>>> MySQLdb.__version__
'0.7.11.None'

Additional changes in [42f422aeb/rjollos.git].

comment:3 by Peter Suter, 7 years ago

how we can specify "MySQL-python or PyMySQL" for the installation requirement in setup.py.

According to this (and this) it seems something like this would be needed:

  • setup.py

    diff -r 22cef9e62a11 setup.py
    a b  
    4141    print("Jinja2 is needed by Trac setup, pre-installing")
    4242    # give some context to the warnings we might get when installing Jinja2
    4343
     44try:
     45    import pymysql
     46    mysql = 'PyMySQL'
     47except ImportError:
     48    mysql = 'MySQL-python >= 1.2.2'
     49
    4450
    4551setup(
    4652    name = 'Trac',
     
    95101    extras_require = {
    96102        'genshi': ['Genshi>=0.6'],
    97103        'babel': ['Babel>=0.9.5'],
    98         'mysql': ['MySQL-python >= 1.2.2'],
     104        'mysql': [mysql],
    99105        'postgresql': ['psycopg2 >= 2.0'],
    100106        'pygments': ['Pygments>=1.0'],
    101107        'rest': ['docutils>=0.3.9'],

comment:4 by Jun Omae, 7 years ago

It might be good to support mysql+mysqldb://... for MySQL-python and mysql+pymysql://... for PyMySQL, like SQLAlchemy.

Another consideration, how about switching to PyMySQL from MySQL-python? The original MySQL-python doesn't support Python 3 but https://github.com/PyMySQL/mysqlclient-python supports Python 3. No new commits since 2014 on https://github.com/farcepest/MySQLdb1.

comment:5 by Ryan J Ollos, 7 years ago

I like the idea of supporting just one MySQL library.

I started to update the compatible distros matrix. pymysql is available on Ubuntu 16.04. I don't see it in Debian 8.8, and looks like it may still be in unstable. Does anyone think that matters since a user can just pip install it?

Last edited 7 years ago by Ryan J Ollos (previous) (diff)

comment:6 by Jun Omae, 7 years ago

Proposed additional changes in [4f56ad5b2/jomae.git]. I'd like to add charset parameter to database connection string for MySQL, to avoid reconnecting when database charset is not matched with client charset.

Yeah, no matters. Even Trac 1.2 on Debian is still sid.

in reply to:  2 comment:7 by Ryan J Ollos, 7 years ago

Replying to Ryan J Ollos:

Also, looks like signature of _show_warnings changed in PyMySQL 0.7.6, and latest package has a strange version string:

Created a pull request for the issue.

comment:8 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

Latest changes, including comment:6 changes, in log:rjollos.git:t12821_pymysql.1.

Tests pass on OSX with the following versions of pymysql:

  • 0.7.11
  • 0.6.7
  • 0.6
Last edited 7 years ago by Ryan J Ollos (previous) (diff)

in reply to:  6 comment:9 by Ryan J Ollos, 7 years ago

Replying to Jun Omae:

Proposed additional changes in [4f56ad5b2/jomae.git]. I'd like to add charset parameter to database connection string for MySQL, to avoid reconnecting when database charset is not matched with client charset.

I might have overlooked an import change. Should the AppVeyor config be modified?:

  • .appveyor.yml

    diff --git a/.appveyor.yml b/.appveyor.yml
    index 34ad19c22..6b9ea0430 100644
    a b environment:  
    2121    TRAC_TEST_DB_URI: sqlite:test.db
    2222  - SVN_BRANCH: trunk
    2323    PYTHONHOME: C:\Miniconda
    24     TRAC_TEST_DB_URI: mysql://tracuser:password@localhost/trac
     24    TRAC_TEST_DB_URI: mysql://tracuser:password@localhost/trac?charset=utf8mb4
    2525  - SVN_BRANCH: trunk
    2626    PYTHONHOME: C:\Miniconda-x64
    2727    TRAC_TEST_DB_URI: postgres://tracuser:password@localhost/trac?schema=tractest

comment:10 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

Committed to trunk in r15965.

Note: See TracTickets for help on using tickets.