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

(lambda-python): PythonFunction index/handler logic broken if handler in subdirectory #15391

Closed
ewahl-al opened this issue Jul 1, 2021 · 7 comments
Labels
@aws-cdk/aws-lambda-python bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@ewahl-al
Copy link

ewahl-al commented Jul 1, 2021

If a handler function is located in a subdirectory of the lambda code package, the aws_lambda.Function handler property is set to an incorrect value by this line of code in aws-lambda-python/lib/function.ts:

      handler: `${index.slice(0, -3)}.${handler}`,

Reproduction Steps

  1. Create a python lambda code package where a handler function is not in the root directory of the package. Example
root/
--- main_file.py
--- handlers/
------ __init__.py
------ my_handler.py
--- lib/
------ __init__.py
------ other_python_code.py
  1. Then deploy a CDK stack containing a PythonFunction construct using this handler. Example:
from aws_cdk import aws_lambda_python as lambda_python

lambda_python.PythonFunction(
    self, 'ExampleFunction',
    entry="../root",
    handler="handle_lambda",
    index="handlers/my_handler.py")

What did you expect to happen?

A lambda function should be deployed with the handler property set to handlers.my_handler.handle_lambda

What actually happened?

A lambda function was deployed with the handler property set to handlers/my_handler.handle_lambda

Environment

  • **CDK CLI Version : 1.107.00
  • **Framework Version: 1.107.0
  • **Node.js Version: v12.8.3
  • **OS : macos
  • **Language (Version): Python 3.8.5

Other

The index.slice string substitution should replace / characters with . characters.


This is 🐛 Bug Report

@ewahl-al ewahl-al added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2021
@BenChaimberg BenChaimberg added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 9, 2021
@BenChaimberg
Copy link
Contributor

Workaround:

(func.node.defaultChild as lambda.CfnFunction).addPropertyOverride('Handler', 'overridden.handler');

Solution is as described above: do string substitution to replace / with ..

@ewahl-al
Copy link
Author

ewahl-al commented Jul 9, 2021

Thank you, I didn't know that trick on using the Cfn property override.

@vincent-turato
Copy link
Contributor

@ewahl-al Could you explain why or point to documentation that shows that the handler should be using dot notation? Do you receive an error?

@ewahl-al
Copy link
Author

@ewahl-al Could you explain why or point to documentation that shows that the handler should be using dot notation? Do you receive an error?

If the lambda function is deployed with the handler path that contains a slash like handlers/my_handler.handle_lambda the lambda function will not execute at all. Python package paths use . characters and won't work with filesystem / characters in the path. For example, in python one writes import os.path not import os/path

@ztane
Copy link

ztane commented Dec 7, 2021

The Python handler name is not a file or a simple top-level name inside the file on the lambda. It is an importable object. It could even make sense for it to be inside a layer. Checking that the handler is a file or anything to that effect is an unnecessary restriction over what AWS Lambda Python runtimes allow outside CDK.

setu4993 added a commit to setu4993/aws-cdk that referenced this issue Dec 29, 2021
mergify bot pushed a commit that referenced this issue Dec 30, 2021
…mage (#18082)

This refactors the bundling process to match the NodeJs and Go Lambda functions and allows providing a custom bundling docker image.

Changes:
- refactor bundling to use `cdk.BundlingOptions`
- Use updated `Bundling` class
- Update tests to use updated `Bundling` class


Fixes #10298, #12949, #15391, #16234, #15306

BREAKING CHANGE: `assetHashType` and `assetHash` properties moved to new `bundling` property.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@setu4993
Copy link
Contributor

#18082 should have fixed this. We replaced parsing of the handler there for subdirectories.

const resolvedHandler =`${index.slice(0, -3)}.${handler}`.replace('/', '.');

@github-actions
Copy link

github-actions bot commented Jan 5, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…mage (aws#18082)

This refactors the bundling process to match the NodeJs and Go Lambda functions and allows providing a custom bundling docker image.

Changes:
- refactor bundling to use `cdk.BundlingOptions`
- Use updated `Bundling` class
- Update tests to use updated `Bundling` class


Fixes aws#10298, aws#12949, aws#15391, aws#16234, aws#15306

BREAKING CHANGE: `assetHashType` and `assetHash` properties moved to new `bundling` property.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-python bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

7 participants