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

#776 fix for libmysqlclient.so.18 error when using slim_handler #789

Merged
merged 9 commits into from
Apr 16, 2017

Conversation

timj98
Copy link
Contributor

@timj98 timj98 commented Apr 13, 2017

Added include to zappa_settings
Added ctypes to handler to load the selected libraries directly to
PYTHONPATH

Description

This PR fixes the issue where Django using MySQLdb looks for libmysqlclient.so.18 and could not find it in any path when using the slim_handler. I creates a setting in the zappa_settings file for the specific library to add at handler init.

GitHub Issues

#776

Added include to zappa_settings
Added ctypes to handler to load the selected libraries directly to
PYTHONPATH
@coveralls
Copy link

coveralls commented Apr 13, 2017

Coverage Status

Coverage decreased (-63.3%) to 15.81% when pulling 59b7e66 on timj98:master into 08bed45 on Miserlou:master.

timj98 added 4 commits April 13, 2017 14:48
added a check to see if include was in the settings file.
Added empty list for default if user did not include it in the settings
file or was not using slim_handler
Does not set include if users did set it.
For got parthenese :)
@coveralls
Copy link

coveralls commented Apr 13, 2017

Coverage Status

Coverage decreased (-0.3%) to 78.873% when pulling 0bbf12d on timj98:master into 08bed45 on Miserlou:master.

@Miserlou
Copy link
Owner

Okay, this looks like we're on the right track.

Two things:

  1. Can we include some sane defaults for INCLUDE so that MySQL and others affected by this in lambda-packages all work out of the box?

  2. Please change the print statements to print() so that we don't have to update them when P3 drops.

@timj98
Copy link
Contributor Author

timj98 commented Apr 14, 2017 via email

timj98 added 4 commits April 15, 2017 07:06
Added a default library so this problem is not encountered when shipping
Changed comments to be python 3 compatible
removed extra space.
Added a check for slim_handler
Added default library for MySQL-python
@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage decreased (-0.3%) to 78.851% when pulling 815964d on timj98:master into 6e6fc19 on Miserlou:master.

@Miserlou Miserlou merged commit 78e61f1 into Miserlou:master Apr 16, 2017
@Miserlou
Copy link
Owner

Miserlou commented Apr 16, 2017

I've merged this in now, thank you for going in on this!

Just as a style point, you included a line of exceptionally ugly code:

settings_s += "INCLUDE=[" + ','.join(("'","'")).join(self.stage_config.get('include', [])).join(("'","'")) + "]\n"

Since it's already a list, we can simply cast it to a string:

include = self.stage_config.get('include', [])
if len(include) >= 1:
    settings_s += "INCLUDE=" + str(include) + '\n'

I've gone ahead and changed this already, but in future it's generally good to try to keep the code as short, readable and WTF-free as possible!

Thanks again,
R

@timj98
Copy link
Contributor Author

timj98 commented Apr 16, 2017 via email

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.

3 participants