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

get_handler_file_from_name when path contains periods #5849

Merged
merged 3 commits into from Apr 19, 2022
Merged

get_handler_file_from_name when path contains periods #5849

merged 3 commits into from Apr 19, 2022

Conversation

tmbobbins
Copy link
Contributor

@tmbobbins tmbobbins commented Apr 12, 2022

Fixes for get_handler_file_from_name, fixing incorrectly returning '.js' from '.build/handler.execute' (Typescript).

Revisiting #1774 / #1775
Also appears related to both #5485 and #3969 with '.webpack/service/first.hello'
This appears to have either regressed or not been fixed, due to handler_name.split(delimiter)[0] will return '.js' from '.build/handler.execute' instead of '.build/handler.js'

Have added in a few tests to with my assumptions on how it's currently working.
Please do let me know if any of these assumptions are incorrect, or anything else for that matter and I'll update the PR.

Thanks,
Matt

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

LGTM! Please just apply the suggestions regarding fstrings (I know they were there before, but we try to replace them wherever we stumble upon them), then we are good to go from my side!
Thanks for this PR!

localstack/services/awslambda/lambda_utils.py Outdated Show resolved Hide resolved
localstack/services/awslambda/lambda_utils.py Outdated Show resolved Hide resolved
tmbobbins and others added 2 commits April 19, 2022 18:27
Co-authored-by: Daniel Fangl <daniel.fangl@gmail.com>
Co-authored-by: Daniel Fangl <daniel.fangl@gmail.com>
@dfangl dfangl merged commit c222d8e into localstack:master Apr 19, 2022
@tmbobbins
Copy link
Contributor Author

@dfangl Thank you!

dominikschubert added a commit that referenced this pull request May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants