Skip to content

fix: Don't use deps dir for non-cached builds #4052

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.

Sign up for GitHub

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

Merged
mildaniel merged 6 commits into from
Jul 14, 2022
Merged

fix: Don't use deps dir for non-cached builds #4052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
42570fe
Don't use deps dir for non-cached builds
mildaniel Jul 13, 2022
cab4802
Remove unused import
mildaniel Jul 13, 2022
cdbb12c
Fix test, rename variable to cached
mildaniel Jul 13, 2022
e8eb1d9
Make default cached false
mildaniel Jul 13, 2022
ed10a76
Add function definition
mildaniel Jul 13, 2022
ba50d9b
Remove debug prints
mildaniel Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion samcli/lib/build/app_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def build(self) -> ApplicationBuildResult:
"""
build_graph = self._get_build_graph(self._container_env_var, self._container_env_var_file)
build_strategy: BuildStrategy = DefaultBuildStrategy(
build_graph, self._build_dir, self._build_function, self._build_layer
build_graph, self._build_dir, self._build_function, self._build_layer, self._cached
)

if self._parallel:
Expand Down
6 changes: 4 additions & 2 deletions samcli/lib/build/build_strategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,13 @@ def __init__(
build_dir: str,
build_function: Callable[[str, str, str, str, str, Optional[str], str, dict, dict, Optional[str], bool], str],
build_layer: Callable[[str, str, str, List[str], str, str, dict, Optional[str], bool], str],
cached: bool = False,
) -> None:
super().__init__(build_graph)
self._build_dir = build_dir
self._build_function = build_function
self._build_layer = build_layer
self._cached = cached

def build_single_function_definition(self, build_definition: FunctionBuildDefinition) -> Dict[str, str]:
"""
Expand Down Expand Up @@ -167,7 +169,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini
single_build_dir,
build_definition.metadata,
container_env_vars,
build_definition.dependencies_dir,
build_definition.dependencies_dir if self._cached else None,
build_definition.download_dependencies,
)
function_build_results[single_full_path] = result
Expand Down Expand Up @@ -214,7 +216,7 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition)
layer.build_architecture,
single_build_dir,
layer_definition.env_vars,
layer_definition.dependencies_dir,
layer_definition.dependencies_dir if self._cached else None,
layer_definition.download_dependencies,
)
}
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/lib/build_module/test_app_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,48 @@ def test_deprecated_runtimes(self, runtime):
artifact_dir="artifact_dir",
)

def test_must_not_use_dep_layer_for_non_cached(self):
mocked_default_build_strategy = Mock()
mocked_default_build_strategy.return_value = mocked_default_build_strategy

function = Function(
function_id="name",
name="name",
functionname="function_name",
runtime="runtime",
memory="memory",
timeout="timeout",
handler="handler",
imageuri="imageuri",
packagetype=ZIP,
imageconfig="imageconfig",
codeuri="codeuri",
environment="environment",
rolearn="rolearn",
layers="layers",
events="events",
codesign_config_arn="codesign_config_arn",
metadata=None,
inlinecode=None,
architectures=[X86_64],
stack_path="",
function_url_config=None,
)

resources_to_build_collector = ResourcesToBuildCollector()
resources_to_build_collector.add_functions([function])

builder = ApplicationBuilder(
resources_to_build_collector, "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)
builder._build_function = Mock()

builder.build()

builder._build_function.assert_called_with(
"name", "codeuri", ZIP, "runtime", X86_64, "handler", str(Path("builddir/name")), {}, {}, None, True
)


class PathValidator:
def __init__(self, path):
Expand Down
15 changes: 10 additions & 5 deletions tests/unit/lib/build_module/test_build_strategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ def setUp(self):

self.function_build_definition1 = FunctionBuildDefinition("runtime", "codeuri", ZIP, X86_64, {}, "handler")
self.function_build_definition2 = FunctionBuildDefinition("runtime2", "codeuri", ZIP, X86_64, {}, "handler")

self.function_build_definition1.add_function(self.function1_1)
self.function_build_definition1.add_function(self.function1_2)
self.function_build_definition2.add_function(self.function2)

self.build_graph.put_function_build_definition(self.function_build_definition1, self.function1_1)
self.build_graph.put_function_build_definition(self.function_build_definition1, self.function1_2)
self.build_graph.put_function_build_definition(self.function_build_definition2, self.function2)
Expand Down Expand Up @@ -164,7 +169,7 @@ def test_build_layers_and_functions(self, mock_copy_tree):
self.function_build_definition1.get_build_dir(given_build_dir),
self.function_build_definition1.metadata,
self.function_build_definition1.env_vars,
self.function_build_definition1.dependencies_dir,
None,
True,
),
call(
Expand All @@ -177,7 +182,7 @@ def test_build_layers_and_functions(self, mock_copy_tree):
self.function_build_definition2.get_build_dir(given_build_dir),
self.function_build_definition2.metadata,
self.function_build_definition2.env_vars,
self.function_build_definition2.dependencies_dir,
None,
True,
),
]
Expand All @@ -194,7 +199,7 @@ def test_build_layers_and_functions(self, mock_copy_tree):
self.layer1.build_architecture,
self.layer1.get_build_dir(given_build_dir),
self.layer_build_definition1.env_vars,
self.layer_build_definition1.dependencies_dir,
None,
True,
),
call(
Expand All @@ -205,7 +210,7 @@ def test_build_layers_and_functions(self, mock_copy_tree):
self.layer2.build_architecture,
self.layer2.get_build_dir(given_build_dir),
self.layer_build_definition2.env_vars,
self.layer_build_definition2.dependencies_dir,
None,
True,
),
]
Expand Down Expand Up @@ -476,7 +481,7 @@ def setUp(self):
self.build_layer = Mock()
self.build_graph = Mock()
self.delegate_build_strategy = DefaultBuildStrategy(
self.build_graph, Mock(), self.build_function, self.build_layer
self.build_graph, Mock(), self.build_function, self.build_layer, cached=True
)
self.build_strategy = IncrementalBuildStrategy(
self.build_graph,
Expand Down