Skip to content
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

Build patch #4867

Merged
merged 7 commits into from
Feb 7, 2024
Merged

Build patch #4867

merged 7 commits into from
Feb 7, 2024

Conversation

gnodar01
Copy link
Collaborator

@gnodar01 gnodar01 commented Feb 7, 2024

Latest build fails because mysqlclient fails to build (on MacOS).

problem:

  • there are no wheels mysqlclient for mac, instead pip builds the wheel
  • this has generally worked fine, but recently, for mysql >= 8.3.0, the MySQL team killed
    some APIs that have been deprecated for a while now, causing the mysqlclient build to fail

solution:

  • we can pin the brew install of mysql to < 8.3.0, or we can bump the
    pip install of mysqlclient to >= 2.2.3 which as of 2/4/24 has a patch
  • later seems more "responsible"

pkg-config is requested in the installation docs for homebrew: https://pypi.org/project/mysqlclient/
v4 has deprecated use of Node.js 16 and github actions complains about
it
problem:
- there are no wheels mysqlclient for mac, instead pip builds the wheel
- this has generally worked fine, but recently, for mysql >= 8.3.0, the MySQL team killed
  some APIs that have been deprecated for a while now, causing the
mysqlclient build to fail

solution:
- we can pin the brew install of mysql to < 8.3.0, or we can bump the
  pip install of mysqlclient to >= 2.2.3 which as of 2/4/24 has a patch: PyMySQL/mysqlclient#688
- later seems more "responsible"
Copy link
Member

@bethac07 bethac07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fair to me, assuming tests pass!

MySQLdb.escape_string was removed in v2.1.0, here: PyMySQL/mysqlclient#511

mysqlclient still allows escape_string to be used via the connection
object

really we should be using parameterized queries: https://pynative.com/python-mysql-execute-parameterized-query-using-prepared-statement/

that would require more substantial changes however
This reverts commit 89ef8a5.

the connection object returned my mysqlclient (`MySQLdb.connect`)
does expose `escape_string` however we do not always access the
`create_database_tables` function with a mysql database
connection object, sometimes we use sqlite, which does not have
`escape_string`
MySQLdb.escape_string was removed in v2.1.0, here: PyMySQL/mysqlclient#511

we can still access `escape_string` via the underlying `_mysql` object
that mysqlclient wraps

really we should be using parameterized queries: https://pynative.com/python-mysql-execute-parameterized-query-using-prepared-statement/

that would require more substantial changes however
@gnodar01
Copy link
Collaborator Author

gnodar01 commented Feb 7, 2024

Sounds fair to me, assuming tests pass!

They did not, but do now other than the #4859 issue.

@gnodar01 gnodar01 merged commit 134e6c3 into main Feb 7, 2024
2 of 3 checks passed
@gnodar01 gnodar01 deleted the build_patch branch February 7, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants