Skip to content

Fix for error when missing MySQLdb #1728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 11, 2018
Merged

Fix for error when missing MySQLdb #1728

merged 5 commits into from
Oct 11, 2018

Conversation

iotmani
Copy link
Contributor

@iotmani iotmani commented Oct 3, 2018 •
edited
Loading

While looking into the cause of an exception:

ModuleNotFoundError: No module named 'MySQLdb'

I noticed the pymysql initialisation was missing, yet it was done for the python2 tutorial settings.py counterpart..

Note that this was noticed even when running the app in "production" on GAE (link for my own error in case it's accessible by Google employees).
So worth looking into if this is the case here:

...This is a convenience feature for developers who cannot install
# MySQLdb locally; when running in production on Google App Engine Standard
# Environment, MySQLdb will be used.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 3, 2018
@iotmani
Copy link
Contributor Author

iotmani commented Oct 10, 2018

@theacodes can you please review :)? Ref 90bfd6a.

@theacodes theacodes requested a review from andrewsg October 10, 2018 00:16
@theacodes
Copy link
Contributor

@andrewsg should review, not me.

@andrewsg
Copy link
Member

Thank you! This is indeed an issue but actually I think we can just use pymysql across the board here, since it's in the requirements file (mysqldb is not) and pymysql is fully compatible with the new Standard runtime (whereas it's not with the Python 2 one).

@michaelawyu Can you please switch over to using pymysql here entirely, and also look into why our tests didn't catch the mismatch?

@andrewsg andrewsg requested review from michaelawyu and removed request for andrewsg October 10, 2018 20:27
@michaelawyu
Copy link
Contributor

michaelawyu commented Oct 10, 2018 •
edited
Loading

@iotmani I am afraid there's some misunderstanding here. MySQLdb does not support Python 3; Django 2 however, requires Python 3. The sample Django app does use PyMySQL for default.

@iotmani
Copy link
Contributor Author

iotmani commented Oct 10, 2018

I see... I think we might have a bug in Django 2 in that case (that we'll probably want to handle here nonetheless)...

The sample code uses engine 'django.db.backends.mysql'.
Looking at the Django 2 code, it does import MySQLdb.

Stacktrace I got while trying to deploy the sample app in the first comment:

django.core.exceptions.ImproperlyConfigured: Error loading MySQLdb module.
at <module> (/env/lib/python3.7/site-packages/django/db/backends/mysql/base.py:20)
at import_module (/env/lib/python3.7/importlib/__init__.py:127)
at load_backend (/env/lib/python3.7/site-packages/django/db/utils.py:110)
at __getitem__ (/env/lib/python3.7/site-packages/django/db/utils.py:202)
at __getattr__ (/env/lib/python3.7/site-packages/django/db/__init__.py:33)
at contribute_to_class (/env/lib/python3.7/site-packages/django/db/models/options.py:203)
at add_to_class (/env/lib/python3.7/site-packages/django/db/models/base.py:305)
at __new__ (/env/lib/python3.7/site-packages/django/db/models/base.py:101)
at <module> (/srv/polls/models.py:7)
at import_module (/env/lib/python3.7/importlib/__init__.py:127)
at import_models (/env/lib/python3.7/site-packages/django/apps/config.py:198)
at populate (/env/lib/python3.7/site-packages/django/apps/registry.py:112)
at setup (/env/lib/python3.7/site-packages/django/__init__.py:24)
at <module> (/srv/main.py:1)
at import_app (/env/lib/python3.7/site-packages/gunicorn/util.py:350)
at load_wsgiapp (/env/lib/python3.7/site-packages/gunicorn/app/wsgiapp.py:41)
at load (/env/lib/python3.7/site-packages/gunicorn/app/wsgiapp.py:52)
at wsgi (/env/lib/python3.7/site-packages/gunicorn/app/base.py:67)
at load_wsgi (/env/lib/python3.7/site-packages/gunicorn/workers/base.py:138)
at init_process (/env/lib/python3.7/site-packages/gunicorn/workers/base.py:129)
at init_process (/env/lib/python3.7/site-packages/gunicorn/workers/gthread.py:104)
at spawn_worker (/env/lib/python3.7/site-packages/gunicorn/arbiter.py:583)

@michaelawyu
Copy link
Contributor

michaelawyu commented Oct 11, 2018 •
edited
Loading

Hi @iotmani,

I looked further into this issue; Django 2 requires mysqlclient, which is a fork of MySQLdbv1 (that actually supports Python 3), so technically speaking Django's error message is not wrong. At this moment, the best solution might be to ask PyMySQL to install as MySQLdb, as suggested in your patch, though the comment will need to be updated as App Engine Python 3 runtime no longer uses MySQLdb or mysqlclient. I just took the liberty to edit your PR; if you are OK with it I will approve and merge it.

Thanks again for reporting and fixing the problem :)

@michaelawyu michaelawyu merged commit a2f4c7f into GoogleCloudPlatform:master Oct 11, 2018
@iotmani
Copy link
Contributor Author

iotmani commented Oct 11, 2018

Perfect, thanks @michaelawyu!

@iotmani iotmani mentioned this pull request Oct 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelawyu michaelawyu michaelawyu approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iotmani @theacodes @andrewsg @michaelawyu @googlebot