@jogold After looking at issue #9406, I suggested to @adamelmore that once #9576 (caching bundler output) is merged, pip installing to a dedicated lambda layer could close the issue. I have opened this draft PR to collect comments.

I'll resume after we merge #9576. The dependencies pip installs may differ by the runtime, but the asset hashing can't yet tell the difference since it doesn't yet include bundler options.

Edit: Since 2edf744, this is no longer relevant. Using docker build and the layer cache to cache the dependencies has effectively made this detail a non-issue.

@adamelmore Hey. I was thinking about this PR this weekend after working on #9728. I figure that if we build the assets in a Docker build, then we can take pretty good advantage of the layer cache. I have a rough implementation up on this PR if you want to take a look.

There's been some traction in #9728. If that PR lands first, I can update this PR to use the new features.

I'll try to review this tomorrow morning; I'm working on an Elasticsearch L2 PR or I would have reviewed it today!

I'll try to review this tomorrow morning; I'm working on an Elasticsearch L2 PR or I would have reviewed it today!

No rush. :)

Is this ready for review?

Is this ready for review?

@eladb Yes it is.

@eladb This python lambda PR is ready for review when you've got a chance.

Merge 8747b6b from master resulted in an unexpected build failure. It seems that recent sam cli build images pip install the chardet 3.0.4 dependency slightly differently than older images.

From the diff below, it seems that there's something changed in the easy_install console script stubs generated by setuptools. I'm uncertain whether this is a one-shot deal or if it will be a recurring problem in the sam cli build images. For now, I've simply updated the expectations. I've left my notes below.

Notes:

• When manually tested, integ.function.pipenv-inline and integ.function.project run on AWS without any problems
• It appears that only assets bundled for Python 3.7 are affected. integ.function.pipenv-inline and integ.function.project are the only integration tests targeting 3.7 - all others are unaffected.
• With recent amazon/aws-sam-cli-build-image-python3.7, the tests also fail in commit d9e0f68 before the merge, so the cause is probably not related to the merge from master.
• When I re-run with a 6-week-old copy of amazon/aws-sam-cli-build-image-python3.7 (4338c0baedef), the integration tests pass.
• I'm uncertain whether this will be a recurring problem with the sam cli build images. For now, I've simply updated the expectations.

integ.function.pipenv-inline asset diff between old and new amazon/aws-sam-cli-build-image-python3.7:

diff -ru cdk.out.oldimg/asset.771170a5b339defba72bda907cf9ccb30fce9b91e45bbb3152dd9df29589957d/bin/chardetect cdk.out.newimg/asset.6327fef54763f920a63cab392d9bd4bc9b14917985a3f95700fcd35610b5e83c/bin/chardetect
--- cdk.out.oldimg/asset.771170a5b339defba72bda907cf9ccb30fce9b91e45bbb3152dd9df29589957d/bin/chardetect        2020-09-27 12:25:54.730254541 -0600
+++ cdk.out.newimg/asset.6327fef54763f920a63cab392d9bd4bc9b14917985a3f95700fcd35610b5e83c/bin/chardetect        2020-09-27 12:18:17.686252265 -0600
@@ -2,9 +2,7 @@
 # -*- coding: utf-8 -*-
 import re
 import sys
-
 from chardet.cli.chardetect import main
-
 if __name__ == '__main__':
-    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
+    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
     sys.exit(main())
diff -ru cdk.out.oldimg/asset.771170a5b339defba72bda907cf9ccb30fce9b91e45bbb3152dd9df29589957d/chardet-3.0.4.dist-info/RECORD cdk.out.newimg/asset.6327fef54763f920a63cab392d9bd4bc9b14917985a3f95700fcd35610b5e83c/chardet-3.0.4.dist-info/RECORD
--- cdk.out.oldimg/asset.771170a5b339defba72bda907cf9ccb30fce9b91e45bbb3152dd9df29589957d/chardet-3.0.4.dist-info/RECORD        2020-09-27 12:25:54.734254490 -0600
+++ cdk.out.newimg/asset.6327fef54763f920a63cab392d9bd4bc9b14917985a3f95700fcd35610b5e83c/chardet-3.0.4.dist-info/RECORD        2020-09-27 12:18:17.690252286 -0600
@@ -1,4 +1,4 @@
-../../bin/chardetect,sha256=bDJN3SiDn9YaLlVuQisJzBXSUWH1KDqMdPjZKiY1Pxc,231
+../../bin/chardetect,sha256=cvP0mdvKK8hDplcSQ2LSWTZAe5uUxr8swGu_T3pnmtc,228
 chardet-3.0.4.dist-info/DESCRIPTION.rst,sha256=PQ4sBsMyKFZkjC6QpmbpLn0UtCNyeb-ZqvCGEgyZMGk,2174
 chardet-3.0.4.dist-info/INSTALLER,sha256=zuuue4knoyJ-UwPPXg8fezS7VCrXJQrAP7zeNuwvFQg,4
 chardet-3.0.4.dist-info/METADATA,sha256=RV_2I4B1Z586DL8oVO5Kp7X5bUdQ5EuKAvNoAEF8wSw,3239

integ.function.project asset diff between old and new amazon/aws-sam-cli-build-image-python3.7:

diff -ru cdk.out.oldimg/asset.4931a239511545d5c1e3ab8d27ce860ac9ac33d0501d86ec31749c676d678b27/python/bin/chardetect cdk.out.newimg/asset.7a572697b6cea2f1d84014e9f44093d47abc652f5d0fe9e8857a5ea6cd994bff/python/bin/chardetect
--- cdk.out.oldimg/asset.4931a239511545d5c1e3ab8d27ce860ac9ac33d0501d86ec31749c676d678b27/python/bin/chardetect 2020-09-27 12:26:03.414144722 -0600
+++ cdk.out.newimg/asset.7a572697b6cea2f1d84014e9f44093d47abc652f5d0fe9e8857a5ea6cd994bff/python/bin/chardetect 2020-09-27 12:18:17.778252749 -0600
@@ -2,9 +2,7 @@
 # -*- coding: utf-8 -*-
 import re
 import sys
-
 from chardet.cli.chardetect import main
-
 if __name__ == '__main__':
-    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
+    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
     sys.exit(main())
diff -ru cdk.out.oldimg/asset.4931a239511545d5c1e3ab8d27ce860ac9ac33d0501d86ec31749c676d678b27/python/chardet-3.0.4.dist-info/RECORD cdk.out.newimg/asset.7a572697b6cea2f1d84014e9f44093d47abc652f5d0fe9e8857a5ea6cd994bff/python/chardet-3.0.4.dist-info/RECORD
--- cdk.out.oldimg/asset.4931a239511545d5c1e3ab8d27ce860ac9ac33d0501d86ec31749c676d678b27/python/chardet-3.0.4.dist-info/RECORD 2020-09-27 12:26:03.418144671 -0600
+++ cdk.out.newimg/asset.7a572697b6cea2f1d84014e9f44093d47abc652f5d0fe9e8857a5ea6cd994bff/python/chardet-3.0.4.dist-info/RECORD 2020-09-27 12:18:17.782252769 -0600
@@ -1,4 +1,4 @@
-../../bin/chardetect,sha256=bDJN3SiDn9YaLlVuQisJzBXSUWH1KDqMdPjZKiY1Pxc,231
+../../bin/chardetect,sha256=cvP0mdvKK8hDplcSQ2LSWTZAe5uUxr8swGu_T3pnmtc,228
 chardet-3.0.4.dist-info/DESCRIPTION.rst,sha256=PQ4sBsMyKFZkjC6QpmbpLn0UtCNyeb-ZqvCGEgyZMGk,2174
 chardet-3.0.4.dist-info/INSTALLER,sha256=zuuue4knoyJ-UwPPXg8fezS7VCrXJQrAP7zeNuwvFQg,4
 chardet-3.0.4.dist-info/METADATA,sha256=RV_2I4B1Z586DL8oVO5Kp7X5bUdQ5EuKAvNoAEF8wSw,3239

LGTM, is this ready to be merged?

@eladb I'll fix the merge conflict and then yep, it'll be ready!

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@eladb Oh jeeze. I didn't see the merge bot message until after I fixed the conflict via github. Is this going to be a problem?

Edit: I'm going to grab a coffee and figure out what I missed in the conflict resolution. Sorry about this confusion.
Edit2: Ok I've found the problem - I missed removing an extra import declaration. I have the fix in hand in my local repo, but I'm not sure if I should be waiting for mergify to do something first?
Edit3: I'm going to push up the fix for the issue I introduced by merging via github ui.

@eladb Hey. It looks like because I accidentally fixed the merge conflict concurrently with the mergify bot action, this PR got stuck and not merged into the main branch. It may need your eyes.

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: a82411d
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).