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

fix: remove functionName from transform-host lambdas #9491

Merged
merged 1 commit into from
Jan 13, 2022
Merged

fix: remove functionName from transform-host lambdas #9491

merged 1 commit into from
Jan 13, 2022

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jan 11, 2022

The @searchable and @predictions directives create a Lambda
function via context.api.host.addLambdaFunction(). This commit
updates addLambdaFunction() to not set the optional functionName
in the CFN. This lets CFN generate a unique name.

The mock functionality also needed to be updated because mock
expects all Lambdas to have this optional property. If the CFN
does not contain a functionName, fall back to the resource name.

Fixes: #9480

The @searchable and @predictions directives create a Lambda
function via context.api.host.addLambdaFunction(). This commit
updates addLambdaFunction() to not set the optional functionName
in the CFN. This lets CFN generate a unique name.

The mock functionality also needed to be updated because mock
expects all Lambdas to have this optional property. If the CFN
does not contain a functionName, fall back to the resource name.
@cjihrig cjihrig requested a review from a team as a code owner January 11, 2022 05:42
@codecov-commenter
Copy link

Codecov Report

Merging #9491 (c4e1c7d) into master (c66de92) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9491   +/-   ##
=======================================
  Coverage   56.13%   56.13%           
=======================================
  Files         902      902           
  Lines       50632    50632           
  Branches    10944    10945    +1     
=======================================
  Hits        28422    28422           
  Misses      20179    20179           
  Partials     2031     2031           
Impacted Files Coverage Δ
...ify-graphql-transformer-core/src/transform-host.ts 23.07% <ø> (ø)
...l-mock/src/CFNParser/resource-processors/lambda.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c66de92...c4e1c7d. Read the comment docs.

@cjihrig cjihrig requested review from lazpavel and yuth January 11, 2022 16:14
@mattfysh
Copy link

LGTM

@DanyPell
Copy link

Waiting on this to be able to deploy to production

@lazpavel lazpavel merged commit 959d6d8 into aws-amplify:master Jan 13, 2022
@mattfysh
Copy link

Can this please be deployed to npm ASAP? It is blocking deployments to production, and downgrading is not a 100% error-proof workaround

@github-actions
Copy link

👋 Hi, this pull request was referenced in the v7.6.10 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v7.6.10.

@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why OpenSearchStreamingLambdaFunction does not have app name prefix and current env suffix?
7 participants