Skip to content

Conversation

me2resh
Copy link
Contributor

@me2resh me2resh commented Dec 4, 2019

Issue #, if available:
None

Description of changes:
The current examples and templates for lambda provisioned concurrency works fine for a true valid condition, but if the condition in the example is false, it will fail with and error because the parent property ProvisionedConcurrencyConfig will still be present, but will have no child configuration settings.

Description of how you validated changes:
In the example templates, change the condition AliasProvisionedConcurrencyEnabled to false
Then when you try to deploy the template, cloudformation deploy will fail with the error:

Property ProvisionedConcurrentExecutions cannot be empty.

I also added a missing condition in the lambda_edge/template.yaml

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

@ferencdev ferencdev left a comment

Choose a reason for hiding this comment

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

🥇

@codecov-io
Copy link

codecov-io commented Dec 4, 2019

Codecov Report

Merging #1288 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1288   +/-   ##
========================================
  Coverage    94.43%   94.43%           
========================================
  Files           78       78           
  Lines         4352     4352           
  Branches       860      860           
========================================
  Hits          4110     4110           
  Misses         115      115           
  Partials       127      127

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 188f218...0a4fe99. Read the comment docs.

@praneetap praneetap changed the base branch from master to develop December 4, 2019 21:22
@praneetap
Copy link
Contributor

Changing base to develop.

@praneetap praneetap self-assigned this Dec 4, 2019
Copy link
Contributor

@praneetap praneetap left a comment

Choose a reason for hiding this comment

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

@me2resh Thanks for submitting this change! Having good examples is really important and this change ensures that. However, I think unintended changes have snuck in from the current merge into develop, can you rebase your changes against the latest develop and make sure only the ones you intend are in this pr?

@me2resh
Copy link
Contributor Author

me2resh commented Dec 7, 2019

Hi @praneetap, the branch currently is rebased against the latest develop and has only my intended changes. please re-review.

Copy link
Contributor

@praneetap praneetap left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!!

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.

6 participants