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

remove library that aws lambda cannot find #1060

Merged

Conversation

jottenlips
Copy link
Contributor

@jottenlips jottenlips commented Oct 15, 2021

Description

I do not see why this hardcoded library is necessary in the source code. If someone wants to include it they can add it to include: [].
We ran into this issue when updating from an old version of Zappa that did not have this hardcoded reference. We are also using MySQL. Happy to discuss alternatives, this just seemed the most straightforward solution. Thanks!

GitHub Issues

#940

Copy link
Contributor

@timj98 timj98 left a comment

Choose a reason for hiding this comment

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

I agree with this change. At the time this original change was created, this file was needed and might as well have automatically been included. Things change over time and it is not necessary.

@jottenlips
Copy link
Contributor Author

jottenlips commented Oct 18, 2021

Awesome, Does include: [] in Zappa Settings work as a workaround for now? It would be great to be able to get this out in a release soon. Seems like it has been a point of frustration for a lot of users.

@timj98
Copy link
Contributor

timj98 commented Oct 18, 2021

Awesome, Does include: [] in Zappa Settings work as a workaround for now? It would be great to be able to get this out in a release soon. Seems like it has been a point of frustration for a lot of users.

I tried it with a local build and it resolved the specific issue with the libmysqlclient.so.18 error, I believe there is still an improper configuration mysqldb error. Or something similar to that if you are still relying on mysql. At least people that do not need mysql will find some relief.

@jottenlips
Copy link
Contributor Author

Nice, i am a little hesitant to switch to PyMySQL, but I believe that is also an option. Do you know the cause of the other MySQL error? I am willing to work on it since we rely heavily on Zappa for our deployments.

@timj98
Copy link
Contributor

timj98 commented Oct 18, 2021 via email

@jottenlips
Copy link
Contributor Author

jottenlips commented Oct 18, 2021

It is the same issue that created the problem years ago. I believe that a certain library file is missing and was included in previous builds, but has changed. So need to figure out what the current MySQL library file is and include that in the path but make sure it doesn’t break if it is not needed. TimJ

I believe this library was included in lambda-packages==0.20.0 and removed in this release. Miserlou/Zappa@f4e846f

https://pypi.org/project/lambda_packages/ *mysqlclient

Going to try adding this dependency to our project and see if the error is resolved. We are on an old version of MySQL that should be compatible with the libmysqlclient.so found in lambda packages.

I still think it should be manually included by the user though and not hardcoded in Zappa. Zappa's docs could steer people to use PyMySQL or build and bundle their own libmysqlclient.so like in the tutorial linked below. I am curious to see how much slower PyMySQL is in our staging environment.

https://newbedev.com/problems-using-mysql-with-aws-lambda-in-python

@jottenlips
Copy link
Contributor Author

jottenlips commented Oct 18, 2021

I could also see Zappa forking https://github.com/Miserlou/lambda-packages, seems like there is a lot of good stuff in it like the ability to use Pillow on lambda easily

@jottenlips
Copy link
Contributor Author

jottenlips commented Oct 18, 2021

Going to try adding this dependency to our project and see if the error is resolved. We are on an old version of MySQL that should be compatible with the libmysqlclient.so found in lambda packages.

Looks like there is more to it, it needs to be in the path like in previous versions of Zappa.

    def extract_lambda_package(self, package_name, path):
        """
        Extracts the lambda package into a given path. Assumes the package exists in lambda packages.
        """

Going to try adding a lambda layer for MySQLClient https://github.com/nonbeing/mysqlclient-python3-aws-lambda#tldr

Looks like there was a zappa blog post about adding dependencies after removing lambda_packages, but it was deleted.

Thanks!

Copy link

@luistm luistm left a comment

Choose a reason for hiding this comment

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

LGTM

@jottenlips
Copy link
Contributor Author

jottenlips commented Oct 22, 2021

For anyone that needs MySQL we are going to go with PyMySQL since it is a simpler implementation and doesn't require this library, which won't work anyway because lambda-packages has been removed.

@hellno
Copy link
Collaborator

hellno commented Nov 9, 2021

Thanks for bringing this issue up and giving context, super helpful 👍🏽

Will this change break existing projects using mysql based on zappa?
If yes, we need clear instructions how users can manually add it back via include: [...]

@jottenlips
Copy link
Contributor Author

Current projects using mysql broke when lambda-packages was removed. This change should have no effect. To use mysql with zappa as of now you have to either make your own lambda layer or rely on pymysql.

@javulticat
Copy link
Member

Looks like black needs to be run. Try running make black before pushing. LGTMO

@jottenlips
Copy link
Contributor Author

Thanks!

@jottenlips
Copy link
Contributor Author

looks like the automation wants this to be reviewed again?
"At least 1 approving review is required by reviewers with write access."

@coveralls
Copy link

coveralls commented Nov 22, 2021

Coverage Status

Coverage remained the same at 73.521% when pulling d4c8961 on jottenlips:bugfix/remove-libmysqlclient.so.18 into f5efdcd on zappa:master.

@javulticat javulticat self-requested a review December 8, 2021 20:49
@jottenlips
Copy link
Contributor Author

thanks @javulticat! glad to see the tests are passing

@javulticat javulticat merged commit ddc3fbb into zappa:master Dec 29, 2021
Ian288 pushed a commit to tackle-io/Zappa that referenced this pull request Jul 11, 2023
* remove library that aws lambda cannot find

* run black

Co-authored-by: javulticat <[email protected]>
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.

6 participants