resolve string index out of range errors #212
Conversation
resolves PyMySQL#189, PyMySQL#175, PyMySQL#170, PyMySQL#169, etc.
added clarity and common fixes for PyMySQL#169 related macOS issues.
README.md
Outdated
| @@ -26,6 +26,23 @@ On Windows, there are binary wheel you can install without MySQLConnector/C or M | |||
| `sudo yum install python3-devel ` # Red Hat / CentOS | |||
|
|
|||
| `brew install mysql-connector-c` # macOS (Homebrew) | |||
| #### Note on macOS | |||
| homebrew's `mysql-connector-c` may have incorrect configuration options that cause compilation errors. Under the `#create options` comment on or about line 112 of `/usr/local/bin/mysql_config`: | |||
There was a problem hiding this comment.
The issue (MySQL's bug) is not homebrew specific.
There was a problem hiding this comment.
I'll update to be more general.
setup_posix.py
Outdated
| if s[0] in "\"'" and s[0] == s[-1]: | ||
| s = s[1:-1] | ||
| return s | ||
| return s.strip("'\"") |
There was a problem hiding this comment.
I don't like this code.
Raising exception for wrong input is good thing. It's not reason for breaking our code.
There was a problem hiding this comment.
This could change so that line 12 is if s and s[0] in "\"'" and s[0] == s[-1]: .
While I agree that raising exceptions is the way to go:
IndexErroris not informative about the root cause -- so this could be caught higher up and rethrown with more context if it is to be preserved.- Removing any possibility of the error (as in my proposed change(s)) allowed me to proceed to install the package, so raising an error may be too conservative in this case.
There was a problem hiding this comment.
First of all, this issue is very special. Once MySQL fixed, it never happen. So IndexError is OK.
And "Errors should never pass silently."
You should fix mysql_config side, not here.
clarity in docs.
README.md
Outdated
| @@ -26,6 +26,25 @@ On Windows, there are binary wheel you can install without MySQLConnector/C or M | |||
| `sudo yum install python3-devel ` # Red Hat / CentOS | |||
|
|
|||
| `brew install mysql-connector-c` # macOS (Homebrew) | |||
| #### MySQL note | |||
There was a problem hiding this comment.
- Insert blank line before heading
- Note for MySQL on macOS, because this issue is macOS specific.
README.md
Outdated
| @@ -26,6 +26,25 @@ On Windows, there are binary wheel you can install without MySQLConnector/C or M | |||
| `sudo yum install python3-devel ` # Red Hat / CentOS | |||
|
|
|||
| `brew install mysql-connector-c` # macOS (Homebrew) | |||
| #### MySQL note | |||
| Versions of `mysql`may have incorrect default configuration options that cause compilation errors when `mysqlclient-python` is installed. (As of November 2017, this is known to be true for homebrew's `mysql-connector-c` and `mysql` packages) | |||
There was a problem hiding this comment.
Replace "mysql" with MySQL Connector/C.
When you did brew install mysql, it is OK.
When you did brew install mysql-connector-c, mysql_config is broken.
There was a problem hiding this comment.
Add link to https://bugs.mysql.com/bug.php?id=86971
resolves #189, #175, #170, #169, etc.