Skip to content

Commit

Permalink
fix: pass copy of environment variables for keeping cache valid (aws#…
Browse files Browse the repository at this point in the history
…2943)

* fix: pass copy of environment variables for keeping cache valid

* add integ tests

* update docs

* make black happy

Co-authored-by: Qingchuan Ma <[email protected]>
  • Loading branch information
2 people authored and moelasmar committed Aug 4, 2021
1 parent 25b511b commit 8a8f5e9
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 3 deletions.
7 changes: 6 additions & 1 deletion samcli/lib/build/build_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pathlib
import shutil
from abc import abstractmethod, ABC
from copy import deepcopy
from typing import Callable, Dict, List, Any, Optional, cast

from samcli.commands.build.exceptions import MissingBuildMethodException
Expand Down Expand Up @@ -114,6 +115,10 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini

LOG.debug("Building to following folder %s", single_build_dir)

# we should create a copy and pass it down, otherwise additional env vars like LAMBDA_BUILDERS_LOG_LEVEL
# will make cache invalid all the time
container_env_vars = deepcopy(build_definition.env_vars)

# when a function is passed here, it is ZIP function, codeuri and runtime are not None
result = self._build_function(
build_definition.get_function_name(),
Expand All @@ -123,7 +128,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini
build_definition.get_handler_name(),
single_build_dir,
build_definition.metadata,
build_definition.env_vars,
container_env_vars,
)
function_build_results[single_full_path] = result

Expand Down
30 changes: 30 additions & 0 deletions tests/integration/buildcmd/test_build_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,36 @@ def test_cache_build(self, use_container, code_uri, function1_handler, function2
expected_messages, command_result, self._make_parameter_override_arg(overrides)
)

@skipIf(SKIP_DOCKER_TESTS, SKIP_DOCKER_MESSAGE)
def test_cached_build_with_env_vars(self):
"""
Build 2 times to verify that second time hits the cached build
"""
overrides = {
"FunctionCodeUri": "Python",
"Function1Handler": "main.first_function_handler",
"Function2Handler": "main.second_function_handler",
"FunctionRuntime": "python3.8",
}
cmdlist = self.get_command_list(
use_container=True, parameter_overrides=overrides, cached=True, container_env_var="FOO=BAR"
)

LOG.info("Running Command (cache should be invalid): %s", cmdlist)
command_result = run_command(cmdlist, cwd=self.working_dir)
self.assertTrue(
"Cache is invalid, running build and copying resources to function build definition"
in command_result.stderr.decode("utf-8")
)

LOG.info("Re-Running Command (valid cache should exist): %s", cmdlist)
command_result_with_cache = run_command(cmdlist, cwd=self.working_dir)

self.assertTrue(
"Valid cache found, copying previously built resources from function build definition"
in command_result_with_cache.stderr.decode("utf-8")
)


@skipIf(
((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE),
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/lib/build_module/test_build_strategy.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from copy import deepcopy
from unittest import TestCase
from unittest.mock import Mock, patch, MagicMock, call, ANY

Expand Down Expand Up @@ -218,11 +219,15 @@ def test_build_single_function_definition_image_functions_with_same_metadata(sel
function2.name = "Function2"
function2.full_path = "Function2"
function2.packagetype = IMAGE
build_definition = FunctionBuildDefinition("3.7", "codeuri", IMAGE, {})
build_definition = FunctionBuildDefinition("3.7", "codeuri", IMAGE, {}, env_vars={"FOO": "BAR"})
# since they have the same metadata, they are put into the same build_definition.
build_definition.functions = [function1, function2]

result = default_build_strategy.build_single_function_definition(build_definition)
with patch("samcli.lib.build.build_strategy.deepcopy", wraps=deepcopy) as patched_deepcopy:
result = default_build_strategy.build_single_function_definition(build_definition)

patched_deepcopy.assert_called_with(build_definition.env_vars)

# both of the function name should show up in results
self.assertEqual(result, {"Function": built_image, "Function2": built_image})

Expand Down

0 comments on commit 8a8f5e9

Please sign in to comment.