-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
(aws-lambda-python-alpha): bundling fails with .venv dir in "entry" dir #22585
Comments
same issue here... |
I think the issue may be with the
Since we are running this in a docker container and we have only entry directory mounted inside the container, it probably doesn't make sense to try to copy the symlink. |
@corymhall thanks for updating this issue. |
Just chiming in to say I also have an issue with the build process including the My setup is the same, using
I believe a "most user friendly" solution that would solve both of these two issues would be to provide an exclusion list at the Construct level. The only problem with that is I noticed that pip already does vaguely similar stuff inline but when I tried to do that with Poetry it actually just removed my source
and I was able to "fix" (re: hack) my problem by altering the locally installed // from: node_modules/@aws-cdk/aws-lambda-python-alpha/lib/bundling.js#createBundlingCommand
// use rsync instead of cp to provide exclusion list
console.log(`<!!Manual hack!!>: "rsync -av --exclude '.venv' -r ${options.inputDir}/ ${options.outputDir}"`);
bundlingCommands.push(`rsync -v --exclude '.venv' -r ${options.inputDir}/ ${options.outputDir}`);
// bundlingCommands.push(`cp -rTL ${options.inputDir}/ ${options.outputDir}`); I'd be happy to throw a PR together if that helps, I just haven't contributed yet so it'll be a bit before I get set up. Proposed fix: Changing from Note Separately, I realized that if |
The issue still persists in 2.50.0. Any updates? 🤔 |
@ryanandonian - nice approach
So best way would be an additional property to exclude directories, right ? |
To me that feels like a reasonable solution that keeps existing behavior as to not break clients, but also lets users customize it without too much mental overhead/complexity. I believe this will also fix @jamesoff 's original issue, since the bundler will just completely disregard The only two minor concerns that I have with this as a long term are:
Besides that, I have already "tested" this flow locally on OSX and it worked fairly cleanly with deployments to real production environments. (note: in the snippet I pasted, I didn't use |
I found a workaround for this issue by adjusting the directory structure. Pointing entry to a folder without a venv fixes the problem. So I just placed the poetry package one folder deeper into the folder structure, while keeping the venv on the original folder level. Hereby you retain the use of a local venv for IDE functionality. Only downside is that you have to navigate through one more folder in your IDE. The directory structure that is giving me the error:
The directory structure that solved the error:
|
Hey - any update on this topic ? |
Hey @pharindoko , I finally got around to getting some time to work on submitting what I feel is a proper fix for our issues. I opened #23670 , so hopefully I was able to align with the contributing guidelines and can get this fix approved by the team. |
Thanks @ryanandonian |
…meter (#23670) 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 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: ~* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)~ ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] 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*
|
Describe the bug
If a
.venv
directory is present in the function source directory (i.e. the value ofentry
toPythonFunction
), the asset bundling fails after apparently trying to find the Python binary inside it. This presumably occurs because in my virtualenv,bin/python
is a symlink to the original Python used as part of the virtualenv creation.I often create a virtualenv in my function's directory to allow for code completion etc while working on the code.
I don't believe this used to happen; this project was successfully deployed on 2.35.0a0 and fails with 2.{46,47}.0a0. I have not bisected the library versions to verify.
Expected Behavior
cdk synth
would succeed.Current Behavior
Reproduction Steps
CDK code:
In the
lambda/
directory,Then
synth
.Possible Solution
I was hoping there would be an option to exclude files/directories from the build but couldn't see one in the docs.
Or, CDK should not attempt to use the existing virtualenv during the build, or emit a warning if one is found, which would have aided my troubleshooting.
Additional Information/Context
No response
CDK CLI Version
2.46.0 (build 5a0595e)
Framework Version
No response
Node.js Version
v18.10.0
OS
macOS 12.6
Language
Python
Language Version
Python 3.10.4
Other information
No response
The text was updated successfully, but these errors were encountered: