Skip to content

Fix unexpected 'decode' AttributeError when MySQLdb module is mapped by PyMySQL #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

ZEALi
Copy link
Contributor

@ZEALi ZEALi commented Apr 9, 2024

In some use cases, pymysql.install_as_MySQLdb() will be used to initialize a 'fake' MySQLdb module, which is actually mapped to pymysql module.

Unfortunately, sw_mysqlclient plugin changes the data type of connection.db from bytes into str, which is regard as bytes in sw_pymysql plugin. That will cause the AttributeError: 'str' object has no attribute 'decode' exception in sw_pymysql.py line 45.

So if the MySQLdb module is only a pointer to pymysql, we should just raise ImportError instead of installation for sw_mysqlclient plugin.

In some use cases, `pymysql.install_as_MySQLdb()` will be used to initialize a 'fake' MySQLdb module,
which is actually mapped to pymysql module.
Then the sw_mysqlclient plugin will change the data type of `connection.db` from bytes into str, 
cause the `AttributeError: 'str' object has no attribute 'decode'` exception in sw_pymysql.py line 45, 
which regard it as bytes.
@wu-sheng wu-sheng requested a review from kezhenxu94 April 9, 2024 07:35
@wu-sheng wu-sheng added this to the 1.1.0 milestone Apr 9, 2024
@wu-sheng wu-sheng added the bug Something isn't working label Apr 9, 2024
@wu-sheng
Copy link
Member

wu-sheng commented Apr 9, 2024

Please fix CI.

double quotes to single quotes
@wu-sheng
Copy link
Member

wu-sheng commented Apr 9, 2024

And you should follow PR template to Update the CHANGELOG.md.

@wu-sheng
Copy link
Member

wu-sheng commented Apr 9, 2024

@kezhenxu94 Please take a look. We also have some CIs fail, but should be irrelevant. We may need some fixes first.

@kezhenxu94 kezhenxu94 requested a review from Superskyyy April 9, 2024 08:46
@Superskyyy
Copy link
Member

Ok I will take a look thanks

@Superskyyy
Copy link
Member

I will temporariy disable the uwsgi case, I believe that uwsgi is getting less attraction and support will be dropped sooner or later.

@Superskyyy
Copy link
Member

LGTM, let's wait for CI.

@Superskyyy
Copy link
Member

Gonna kick off a release soon this weekend.

@wu-sheng wu-sheng merged commit 32fbd35 into apache:master Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants