feat(lambda-python): add optional poetry bundling exclusion list parameter #23670

ryanandonian · 2023-01-12T23:37:02Z

A summary of this change is: change from use of cp to rsync --exclude='x' in the bundling commands for poetry based lambdas.

The intention of this PR is to enable the bundling code for Poetry projects to exclude certain files and/or folders from the bundled assets. Currently, if developing a python lambda using either virtualenv itself or a toolchain that leverages virtual environments (re: poetry, specifically with virtualenv.in-project = true, which is strongly recommended for leveraging python tools in VSCode), the bundling code will copy the entire folder passed in. This leads to copying the entire .venv directory into the bundled assets, even though the directory is ignored. Ultimately this leads to inflating the assets by the size of unzipped dependencies (numpy, for instance, is 50Mb by itself).

I verified this concept works in another project I maintain which leverages @aws-cdk/aws-lambda-python-alpha by manually editing the bundling file (I mentioned it in more detail in the linked issue #22585), but this temporary approach requires manually editing files from node_modules, so it is not a proper fix.

fixes #22585

[Post-merge edit] This feature was extended for all Python bundle tools (pipenv, virtualenv, etc), not just Poetry. I forgot to edit the title before merge. Sorry for the confusion.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

Note: I was unable to implement an integration test. My plan was to verify an asset was ignored from the existing poetry integration test sample directory packages/@aws-cdk/aws-lambda-python/test/lambda-handler-poetry by passing in ['.ignorefile'] and confirming that asset was excluded, but I was unable to get the test working due to SSM parameters missing. I wasn't sure if I could bootstrap this and get it working.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2023-01-12T23:37:13Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

aws-cdk-automation · 2023-01-28T00:09:17Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-29T00:09:38Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-30T00:09:15Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

ryanandonian · 2023-01-30T19:57:23Z

Accidentally messed up my fork with a sync. It should be back to a normal state.

aws-cdk-automation · 2023-02-03T00:09:09Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

add snapshot test verifying functionality

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

TheRealAmazonKendra · 2023-02-09T07:43:37Z

Accidentally messed up my fork with a sync. It should be back to a normal state.

Story of my life, lol.

comcalvi · 2023-02-09T23:17:37Z

packages/@aws-cdk/aws-lambda-python/lib/types.ts

+ *
+ * @default - Empty list
+ */
+  readonly poetryAssetExcludes?: string[];


Is there a reason this should be scoped to poetry only and not just be called assetExcludes?

Honestly, I was just trying to minimize the blast radius of this PR. But there is no real reason why this should be limited to poetry only. I can make the change to support all the packagers.

Seems like there might be existing issues that this PR could help out with too #23313

You could definitely add support for that if you wanted, but this PR is also fine in its current scope 🙂

I just pushed some new changes moving it to just assetExcludes. I will definitely need help verifying and cleaning up these integration tests, that seemed to have had a bit more of an impact adding this to the other tests 👀

…s for python

comcalvi

lgtm

mergify · 2023-02-14T20:18:08Z

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

mergify · 2023-02-15T15:34:49Z

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

Pull request has been modified.

mergify · 2023-02-18T07:16:16Z

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

aws-cdk-automation · 2023-02-18T08:35:16Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: a37ea01
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2023-02-18T08:35:31Z

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

cwaldbieser · 2023-03-01T16:23:45Z

2 quick questions-- will this also allow exclusions for those of us using pipenv rather than poetry? Or for those who just want to create the virtualenv in the assets folder for other logistical reasons?
And are there some docs on how to use the bundling option? I think I get the idea from looking at the code, but a short example would help.

ryanandonian · 2023-03-01T19:14:53Z

Hey @cwaldbieser - it originally was slated to be poetry-only (hence the title), but after feedback it was raised up to be for all Python build systems. I unfortunately forgot to change the title to reflect those changes. I'm not sure if there are mechanical implications in this repository to changing that title post-merge, it might complicate something that I'm unaware of so I can't safely change it. But these changes should let you utilize it regardless of packager.

There should be a simple usage example in the Packaging > Excluding source files section of this specific module's README.

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Jan 12, 2023

aws-cdk-automation requested a review from a team January 12, 2023 23:37

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels Jan 12, 2023

aws-cdk-automation previously requested changes Jan 12, 2023

View reviewed changes

ryanandonian mentioned this pull request Jan 12, 2023

(aws-lambda-python-alpha): bundling fails with .venv dir in "entry" dir #22585

Closed

ryanandonian closed this Jan 30, 2023

ryanandonian force-pushed the feat/aws-lambda-python-exclude-parameter-poetry branch from c22f341 to 642b4ac Compare January 30, 2023 18:45

ryanandonian reopened this Jan 30, 2023

ryanandonian changed the title ~~feat(aws-lambda-python): add optional poetry bundling exclusion list parameter~~ feat(lambda-python): add optional poetry bundling exclusion list parameter Jan 30, 2023

feat(aws-lambda-python): support for poetry bundle asset exclusion list

98a6fbb

ryanandonian force-pushed the feat/aws-lambda-python-exclude-parameter-poetry branch from 6edf153 to 98a6fbb Compare February 7, 2023 19:23

fix bundle props to be correctly sent to bundler

0449eef

add snapshot test verifying functionality

Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry

1b77de0

Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry

57ec161

comcalvi reviewed Feb 9, 2023

View reviewed changes

ryanandonian added 2 commits February 13, 2023 15:28

remove poetry-specific assetExcludes in favor of general assetExclude…

dc9dbb2

…s for python

Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry

405b72d

comcalvi previously approved these changes Feb 14, 2023

View reviewed changes

Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry

2ec7893

TheRealAmazonKendra approved these changes Feb 18, 2023

View reviewed changes

Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry

a37ea01

mergify bot merged commit 53beeae into aws:main Feb 18, 2023

ryanandonian deleted the feat/aws-lambda-python-exclude-parameter-poetry branch February 21, 2023 16:17

filletofish mentioned this pull request Jul 2, 2023

feat(scheduler): ScheduleGroup #26196

Merged

WontonSam mentioned this pull request Oct 25, 2024

[Snyk] Upgrade @aws-cdk/assert from 1.128.0 to 2.68.0 WontonSam/apollo-federation-subgraph-compatibility#621

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lambda-python): add optional poetry bundling exclusion list parameter #23670

feat(lambda-python): add optional poetry bundling exclusion list parameter #23670

ryanandonian commented Jan 12, 2023 •

edited

Loading

gitpod-io bot commented Jan 12, 2023

aws-cdk-automation left a comment

aws-cdk-automation commented Jan 28, 2023

aws-cdk-automation commented Jan 29, 2023

aws-cdk-automation commented Jan 30, 2023

ryanandonian commented Jan 30, 2023

aws-cdk-automation commented Feb 3, 2023

TheRealAmazonKendra commented Feb 9, 2023

comcalvi Feb 9, 2023

ryanandonian Feb 9, 2023

comcalvi Feb 13, 2023

ryanandonian Feb 13, 2023

comcalvi left a comment

mergify bot commented Feb 14, 2023

mergify bot commented Feb 15, 2023

mergify bot commented Feb 18, 2023

aws-cdk-automation commented Feb 18, 2023

mergify bot commented Feb 18, 2023

cwaldbieser commented Mar 1, 2023

ryanandonian commented Mar 1, 2023

feat(lambda-python): add optional poetry bundling exclusion list parameter #23670

feat(lambda-python): add optional poetry bundling exclusion list parameter #23670

Conversation

ryanandonian commented Jan 12, 2023 • edited Loading

All Submissions:

Adding new Construct Runtime Dependencies:

New Features

gitpod-io bot commented Jan 12, 2023

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 28, 2023

aws-cdk-automation commented Jan 29, 2023

aws-cdk-automation commented Jan 30, 2023

ryanandonian commented Jan 30, 2023

aws-cdk-automation commented Feb 3, 2023

TheRealAmazonKendra commented Feb 9, 2023

comcalvi Feb 9, 2023

Choose a reason for hiding this comment

ryanandonian Feb 9, 2023

Choose a reason for hiding this comment

comcalvi Feb 13, 2023

Choose a reason for hiding this comment

ryanandonian Feb 13, 2023

Choose a reason for hiding this comment

comcalvi left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 14, 2023

mergify bot commented Feb 15, 2023

mergify bot commented Feb 18, 2023

aws-cdk-automation commented Feb 18, 2023

AWS CodeBuild CI Report

mergify bot commented Feb 18, 2023

cwaldbieser commented Mar 1, 2023

ryanandonian commented Mar 1, 2023

ryanandonian commented Jan 12, 2023 •

edited

Loading