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.

Sign up for GitHub

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

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

Merged
merged 8 commits into from
Feb 18, 2023
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
98a6fbb
feat(aws-lambda-python): support for poetry bundle asset exclusion list
ryanandonian Jan 30, 2023
0449eef
fix bundle props to be correctly sent to bundler
ryanandonian Feb 7, 2023
1b77de0
Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry
ryanandonian Feb 8, 2023
57ec161
Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry
ryanandonian Feb 9, 2023
dc9dbb2
remove poetry-specific assetExcludes in favor of general assetExclude…
ryanandonian Feb 13, 2023
405b72d
Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry
ryanandonian Feb 13, 2023
2ec7893
Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry
TheRealAmazonKendra Feb 18, 2023
a37ea01
Merge branch 'main' into feat/aws-lambda-python-exclude-parameter-poetry
mergify[bot] Feb 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ Packaging is executed using the `Packaging` class, which:
├── poetry.lock # your poetry lock file has to be present at the entry path
```

If using `poetry`, particularly with `virtualenvs.in-project = true`, you can exclude specific files from the copied files using the optional bundling string array parameter `poetryAssetExcludes`

```ts
new python.PythonFunction(this, 'function', {
entry: '/path/to/poetry-function',
runtime: Runtime.PYTHON_3_8,
bundling: {
// translates to `rsync --exclude='.venv'`
poetryAssetExcludes: ['.venv'],
},
});
```


## Custom Bundling

Custom bundling can be performed by passing in additional build arguments that point to index URLs to private repos, or by using an entirely custom Docker images for bundling dependencies. The build args currently supported are:
Expand Down
10 changes: 8 additions & 2 deletions packages/@aws-cdk/aws-lambda-python/lib/bundling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export class Bundling implements CdkBundlingOptions {
image,
poetryIncludeHashes,
commandHooks,
poetryAssetExcludes = [],
} = props;

const outputPath = path.posix.join(AssetStaging.BUNDLING_OUTPUT_DIR, outputPathSuffix);
Expand All @@ -93,6 +94,7 @@ export class Bundling implements CdkBundlingOptions {
outputDir: outputPath,
poetryIncludeHashes,
commandHooks,
poetryAssetExcludes,
});

this.image = image ?? DockerImage.fromBuild(path.join(__dirname, '../lib'), {
Expand All @@ -118,7 +120,10 @@ export class Bundling implements CdkBundlingOptions {
const packaging = Packaging.fromEntry(options.entry, options.poetryIncludeHashes);
let bundlingCommands: string[] = [];
bundlingCommands.push(...options.commandHooks?.beforeBundling(options.inputDir, options.outputDir) ?? []);
bundlingCommands.push(`cp -rTL ${options.inputDir}/ ${options.outputDir}`);
const exclusionStr = options.poetryAssetExcludes?.map(item => `--exclude='${item}'`).join(' ');
bundlingCommands.push([
'rsync', '-rLv', exclusionStr ?? '', `${options.inputDir}/`, options.outputDir,
].filter(item => item).join(' '));
bundlingCommands.push(`cd ${options.outputDir}`);
bundlingCommands.push(packaging.exportCommand ?? '');
if (packaging.dependenciesFile) {
Expand All @@ -133,8 +138,9 @@ interface BundlingCommandOptions {
readonly entry: string;
readonly inputDir: string;
readonly outputDir: string;
readonly poetryAssetExcludes?: string[];
readonly poetryIncludeHashes?: boolean;
readonly commandHooks?: ICommandHooks
readonly commandHooks?: ICommandHooks;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-lambda-python/lib/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ export interface BundlingOptions extends DockerRunOptions {
*/
readonly poetryIncludeHashes?: boolean;

/**
* When using Poetry bundler, list of file patterns to exclude when copying assets for bundling.
*
* @default - Empty list
*/
readonly poetryAssetExcludes?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 👀


/**
* Output path suffix: the suffix for the directory into which the bundled output is written.
*
Expand Down
60 changes: 51 additions & 9 deletions packages/@aws-cdk/aws-lambda-python/test/bundling.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ test('Bundling a function without dependencies', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output && cd /asset-output',
'rsync -rLv /asset-input/ /asset-output && cd /asset-output',
],
}),
}));
Expand Down Expand Up @@ -66,7 +66,7 @@ test('Bundling a function with requirements.txt', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output && cd /asset-output && python -m pip install -r requirements.txt -t /asset-output',
'rsync -rLv /asset-input/ /asset-output && cd /asset-output && python -m pip install -r requirements.txt -t /asset-output',
],
}),
}));
Expand All @@ -89,7 +89,7 @@ test('Bundling Python 2.7 with requirements.txt installed', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output && cd /asset-output && python -m pip install -r requirements.txt -t /asset-output',
'rsync -rLv /asset-input/ /asset-output && cd /asset-output && python -m pip install -r requirements.txt -t /asset-output',
],
}),
}));
Expand All @@ -109,7 +109,7 @@ test('Bundling a layer with dependencies', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && python -m pip install -r requirements.txt -t /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python && python -m pip install -r requirements.txt -t /asset-output/python',
],
}),
}));
Expand All @@ -129,7 +129,7 @@ test('Bundling a python code layer', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python',
],
}),
}));
Expand All @@ -149,7 +149,7 @@ test('Bundling a function with pipenv dependencies', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > requirements.txt && rm -rf .venv && python -m pip install -r requirements.txt -t /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python && PIPENV_VENV_IN_PROJECT=1 pipenv lock -r > requirements.txt && rm -rf .venv && python -m pip install -r requirements.txt -t /asset-output/python',
],
}),
}));
Expand All @@ -176,7 +176,7 @@ test('Bundling a function with poetry dependencies', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --without-hashes --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --without-hashes --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python',
],
}),
}));
Expand All @@ -189,6 +189,48 @@ test('Bundling a function with poetry dependencies', () => {
expect(files).toContain('.ignorefile');
});

test('Bundling a function with poetry and poetryAssetExcludes', () => {
const entry = path.join(__dirname, 'lambda-handler-poetry');

Bundling.bundle({
entry: path.join(entry, '.'),
runtime: Runtime.PYTHON_3_9,
architecture: Architecture.X86_64,
outputPathSuffix: 'python',
poetryAssetExcludes: ['.ignorefile'],
});

expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
bundling: expect.objectContaining({
command: [
'bash', '-c',
"rsync -rLv --exclude='.ignorefile' /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --without-hashes --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python",
],
}),
}));

});

test('Bundling a function with poetry and no poetryAssetExcludes', () => {
const entry = path.join(__dirname, 'lambda-handler-poetry');

Bundling.bundle({
entry: path.join(entry, '.'),
runtime: Runtime.PYTHON_3_9,
architecture: Architecture.X86_64,
outputPathSuffix: 'python',
});

expect(Code.fromAsset).toHaveBeenCalledWith(entry, expect.objectContaining({
bundling: expect.objectContaining({
command: [
'bash', '-c',
expect.not.stringContaining('--exclude'),
],
}),
}));
});

test('Bundling a function with poetry dependencies, with hashes', () => {
const entry = path.join(__dirname, 'lambda-handler-poetry');

Expand All @@ -204,7 +246,7 @@ test('Bundling a function with poetry dependencies, with hashes', () => {
bundling: expect.objectContaining({
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python && poetry export --with-credentials --format requirements.txt --output requirements.txt && python -m pip install -r requirements.txt -t /asset-output/python',
],
}),
}));
Expand Down Expand Up @@ -234,7 +276,7 @@ test('Bundling a function with custom bundling image', () => {
image,
command: [
'bash', '-c',
'cp -rTL /asset-input/ /asset-output/python && cd /asset-output/python && python -m pip install -r requirements.txt -t /asset-output/python',
'rsync -rLv /asset-input/ /asset-output/python && cd /asset-output/python && python -m pip install -r requirements.txt -t /asset-output/python',
],
}),
}));
Expand Down
Loading