Add support for environment_variables config option #273
Conversation
jamesls
requested review from
dstufft,
stealthycoin,
JordonPhillips and
kyleknap
Codecov Report
@@ Coverage Diff @@
## package #273 +/- ##
===========================================
+ Coverage 85.87% 86.55% +0.68%
===========================================
Files 18 18
Lines 1678 1696 +18
Branches 202 207 +5
===========================================
+ Hits 1441 1468 +27
+ Misses 184 175 -9
Partials 53 53
Continue to review full report at Codecov.
|
chalice/awsclient.py
Outdated
| FunctionName=function_name, ZipFile=zip_contents) | ||
| if environment_variables is not None: |
There was a problem hiding this comment.
The way this code sits currently won't delete environment variables if the user removes all references to it from their config file. I think it makes sense to have the deployment reflect the config as much as possible. Maybe use something like the following if this is functionality that you want.
if not environment_variables:
environment_variables = {}
lambda_client.update_function_configuration(
FunctionName=function_name,
Environment={"Variables": environment_variables})
There was a problem hiding this comment.
Good point, I was hoping to avoid calling update_function_configuration unless needed, I think it's best to just call it every time. Otherwise I'd have to store previously deployed function config values which I'd prefer not to do (I plan on adding support for all the various lambda config options, just wanted to start with environment_variables to make sure there were no issues)
It is an extra function call, but I don't think it will slow things down too much.
I'll get this updated.
This builds on the initial work from @emellis and adds support for the environment variables config option. These changes include: * Updating the config object with an environment_variables property * Adding the concept of "chain merge" to config. This allows a dict to be updated (merged) instead of returning on the first value found. * Update API based deployer to inject env vars when needed. This is needed for both create_function and update_function calls. * Changed update_function_code to just update_function which will optionally update function config if required * Update packager to add environment variables to SAM template. There's still a few things in the initial PR that I'll need to circle back on in follow up PRs.
Handle the case where a user completely removes the env vars from their config.json. We'll need this as more config options for lambda are added.
|
Updated accordingly, should be ready for another look. I also rebased against the latest package branch so the diff should be easier to read. |
tests/functional/test_awsclient.py
Outdated
| @@ -25,8 +25,11 @@ def test_deploy_rest_api(stubbed_session): | |||
|
|
|||
|
|
|||
| def test_update_function_code_only(stubbed_session): | |||
There was a problem hiding this comment.
Probably will want to update the test name now.
These changes include:
This builds on the initial work from @emellis (#243) and adds
support for the environment variables config option.
a dict to be updated (merged) instead of returning on the first value
found.
needed for both create_function and update_function calls.
optionally update function config if required
NOTE: This doesn't merge cleanly on the package branch, and I based
it off the latest PR (#272). The only commit you need to review here
is the latest commit: jamesls@507dd11