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

add support for python3.10 lambda runtime, now that it's officially a… #8184

Merged

Conversation

matt-mercer
Copy link
Contributor

@matt-mercer matt-mercer commented Apr 23, 2023

Changes

This PR introduces support for the python3.10 runtime recently released by AWS.

https://aws.amazon.com/blogs/compute/python-3-10-runtime-now-available-in-aws-lambda/

Fixes #8185

@localstack-bot
Copy link
Collaborator

localstack-bot commented Apr 23, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@matt-mercer
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

thanks for tackling this Matt @matt-mercer!

before we move forward with adding the lambda runtime, I'd ask that we move the boto upgrade and necessary service router changes into a separate PR. the solution looks good, just looking to keep the change set minimal and make reviewing the lambda changes easier.

@matt-mercer
Copy link
Contributor Author

thanks for tackling this Matt @matt-mercer!

before we move forward with adding the lambda runtime, I'd ask that we move the boto upgrade and necessary service router changes into a separate PR. the solution looks good, just looking to keep the change set minimal and make reviewing the lambda changes easier.

@thrau sure ... to be honest could use a bit of support on this:

  • I can't get the current failing unit tests to fail locally at the moment, so struggling to debug
  • and I'm presuming there's something I've not done / needs to be unlocked for the pro tests to run ..

@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from ecb56a2 to 3440c5e Compare April 27, 2023 06:31
@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from 3440c5e to a0ca5ba Compare April 27, 2023 07:33
@matt-mercer
Copy link
Contributor Author

based from / depends on #8210

Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

First of all, thank you for your contribution!

Before we can accept it, there are some things that need fixing:

  • All the snapshots (files ending with .snapshot.json) are automatically created by executing the tests against AWS and automatically recorded. I can see that, at least some of the snapshots, were manually edited. Please follow the guide here: https://docs.localstack.cloud/contributing/parity-testing/ for more information, and please do not hesitate to ask here or in the #contributing community slack channel if you have any questions.
  • All files in localstack/aws/api are automatically created from the botocore definitions. Therefore, manual additions are not allowed. Once the botocore update PR is merged, we can regenerate the lambda API, which will fix this.
  • Please add the python 3.10 runtime to the python runtimes in tests/integration/awslambda/conftest.py, in the RUNTIMES_AGGREGATED dict. This will allow the tests in test_lambda_common.py to be executed with the new runtime, which provides several testing scenarios across all supported runtimes. Please also run those tests with a snapshot update, as we need the python3.10 snapshots as well, then.

Sorry for being so nitpicky, but we make a lot of effort to maintain AWS parity, and the automatically generated snapshots are a major tool for this, so we need to rely on those.

@matt-mercer
Copy link
Contributor Author

First of all, thank you for your contribution!

Before we can accept it, there are some things that need fixing:

  • All the snapshots (files ending with .snapshot.json) are automatically created by executing the tests against AWS and automatically recorded. I can see that, at least some of the snapshots, were manually edited. Please follow the guide here: https://docs.localstack.cloud/contributing/parity-testing/ for more information, and please do not hesitate to ask here or in the #contributing community slack channel if you have any questions.
  • All files in localstack/aws/api are automatically created from the botocore definitions. Therefore, manual additions are not allowed. Once the botocore update PR is merged, we can regenerate the lambda API, which will fix this.
  • Please add the python 3.10 runtime to the python runtimes in tests/integration/awslambda/conftest.py, in the RUNTIMES_AGGREGATED dict. This will allow the tests in test_lambda_common.py to be executed with the new runtime, which provides several testing scenarios across all supported runtimes. Please also run those tests with a snapshot update, as we need the python3.10 snapshots as well, then.

Sorry for being so nitpicky, but we make a lot of effort to maintain AWS parity, and the automatically generated snapshots are a major tool for this, so we need to rely on those.

no worries, it's not being nitpicky, unfortunately I don't have access to a sandbox account ( or account that I'm willing to run the snapshot generation against), so would appreciate it if someone else can regenerate the snapshots to help support it getting this runtime available,

also having a lot of problems running the whole test suites locally, feel like there's some missing instructions / assumed knowledge that is missing from the contributing guide ...

@dfangl
Copy link
Member

dfangl commented Apr 27, 2023

No worries, i will generate the snapshots for you and push it directly.

For the problems with you local test setup: there are definitely tests where it is hard to get them running correctly in host mode. Best to ask in our contributors channel for help with the logs of those tests :)

@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from 6edfaca to a02b361 Compare April 28, 2023 08:03
@matt-mercer
Copy link
Contributor Author

matt-mercer commented Apr 28, 2023

No worries, i will generate the snapshots for you and push it directly.

For the problems with you local test setup: there are definitely tests where it is hard to get them running correctly in host mode. Best to ask in our contributors channel for help with the logs of those tests :)

@dfangl best to review this PR (as the boto update has been split off to there,) at the mo, I think .. I managed to find an account down the back of the sofa, that I could run the snapshot regeneration in, this branch is currently based from that one, so looks like it contains more than it does

@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from a02b361 to c41b0d4 Compare April 28, 2023 08:16
@dfangl dfangl self-assigned this Apr 28, 2023
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

I think we are almost done. There was one API change in the meantime which leads to a test failure, which would need to be corrected.

For the lambda feature tests we only test the newest python version (and currently node16, due to the sdk changes with node18). So only using python3.10 there instead of both 3.9 and 3.10 is totally fine (we also have to look at test execution times here).

Also, please change the test tests/integration/awslambda/test_lambda_runtimes.py::TestPythonRuntimes::test_python_runtime_unhandled_errors to test the condition currently only present for python 3.9 for python 3.10 as well.

Other than that, it already looks really well, thank you!

tests/integration/awslambda/test_lambda.py Outdated Show resolved Hide resolved
tests/integration/awslambda/conftest.py Show resolved Hide resolved
@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from c41b0d4 to 52f71f8 Compare April 30, 2023 18:45
@matt-mercer matt-mercer marked this pull request as ready for review April 30, 2023 19:33
@matt-mercer
Copy link
Contributor Author

based on #8210

@matt-mercer matt-mercer requested a review from dfangl April 30, 2023 19:35
@dfangl
Copy link
Member

dfangl commented May 2, 2023

Already looks great. I will rebase to master as the other PR is now merged, and clean up some of the snapshots, then we are good to go :)

@matt-mercer matt-mercer force-pushed the mm-add-support-for-lambda-python-310 branch from 52f71f8 to d580aaa Compare May 2, 2023 10:02
@matt-mercer
Copy link
Contributor Author

Already looks great. I will rebase to master as the other PR is now merged, and clean up some of the snapshots, then we are good to go :)

@dfangl .. I've just rebased ..

@dfangl
Copy link
Member

dfangl commented May 2, 2023

Perfect, thanks!

Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

LGTM now, thank you for the contribution!

I just reverted the manual changes for the API skeleton, and regenerated some incorrect snapshots. Now it's good to go, once the tests are green (those which run).

@dfangl dfangl merged commit 3fc9a7e into localstack:master May 2, 2023
@dfangl
Copy link
Member

dfangl commented May 2, 2023

It's merged now, thank you again!

@matt-mercer matt-mercer deleted the mm-add-support-for-lambda-python-310 branch May 3, 2023 10:17
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.

enhancement request: lambda python3.10 runtime support
4 participants