Skip to content

fix: clean build folder before starting sync operation #3982

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
mndeveci merged 3 commits into from
Jun 21, 2022
Merged

fix: clean build folder before starting sync operation #3982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac307e3
fix: clean build folder before starting sync operation
mndeveci Jun 17, 2022
c88f519
use function or layer build folder
mndeveci Jun 17, 2022
55c8e26
Merge branch 'develop' into fix_sync_ruby_dependency_change_issue
mildaniel Jun 21, 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/sync/flows/function_sync_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def __init__(
)
self._function_identifier = function_identifier
self._function_provider = self._build_context.function_provider
self._function = cast(Function, self._function_provider.functions.get(self._function_identifier))
self._function = cast(Function, self._function_provider.get(self._function_identifier))
self._lambda_client = None
self._lambda_waiter = None
self._lambda_waiter_config = {"Delay": 1, "MaxAttempts": 60}
Expand Down
17 changes: 16 additions & 1 deletion samcli/lib/sync/flows/layer_sync_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@

from samcli.lib.build.app_builder import ApplicationBuilder
from samcli.lib.package.utils import make_zip
from samcli.lib.providers.provider import ResourceIdentifier, Stack, get_resource_by_id, Function
from samcli.lib.providers.provider import ResourceIdentifier, Stack, get_resource_by_id, Function, LayerVersion
from samcli.lib.providers.sam_function_provider import SamFunctionProvider
from samcli.lib.sync.exceptions import MissingPhysicalResourceError, NoLayerVersionsFoundError
from samcli.lib.sync.sync_flow import SyncFlow, ResourceAPICall, ApiCallTypes
from samcli.lib.sync.sync_flow_executor import HELP_TEXT_FOR_SYNC_INFRA
from samcli.lib.utils.colors import Colored
from samcli.lib.utils.hash import file_checksum
from samcli.lib.sync.flows.function_sync_flow import wait_for_function_update_complete
from samcli.lib.utils.osutils import rmtree_if_exists

if TYPE_CHECKING: # pragma: no cover
from samcli.commands.build.build_context import BuildContext
Expand Down Expand Up @@ -175,6 +176,18 @@ class LayerSyncFlow(AbstractLayerSyncFlow):
"""SyncFlow for Lambda Layers"""

_new_layer_version: Optional[int]
_layer: LayerVersion

def __init__(
self,
layer_identifier: str,
build_context: "BuildContext",
deploy_context: "DeployContext",
physical_id_mapping: Dict[str, str],
stacks: List[Stack],
):
super().__init__(layer_identifier, build_context, deploy_context, physical_id_mapping, stacks)
self._layer = cast(LayerVersion, build_context.layer_provider.get(self._layer_identifier))

def set_up(self) -> None:
super().set_up()
Expand Down Expand Up @@ -205,6 +218,8 @@ def set_up(self) -> None:
def gather_resources(self) -> None:
"""Build layer and ZIP it into a temp file in self._zip_file"""
with self._get_lock_chain():

rmtree_if_exists(self._layer.get_build_dir(self._build_context.build_dir))
builder = ApplicationBuilder(
self._build_context.collect_build_resources(self._layer_identifier),
self._build_context.build_dir,
Expand Down
2 changes: 2 additions & 0 deletions samcli/lib/sync/flows/zip_function_sync_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from samcli.lib.build.app_builder import ApplicationBuilder
from samcli.lib.sync.sync_flow import ResourceAPICall, ApiCallTypes
from samcli.lib.utils.osutils import rmtree_if_exists

if TYPE_CHECKING: # pragma: no cover
from samcli.commands.deploy.deploy_context import DeployContext
Expand Down Expand Up @@ -79,6 +80,7 @@ def gather_resources(self) -> None:
if self.has_locks():
exit_stack.enter_context(self._get_lock_chain())

rmtree_if_exists(self._function.get_build_dir(self._build_context.build_dir))
builder = ApplicationBuilder(
self._build_context.collect_build_resources(self._function_identifier),
self._build_context.build_dir,
Expand Down
11 changes: 10 additions & 1 deletion samcli/lib/utils/osutils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import sys
import tempfile
from contextlib import contextmanager
from typing import List, Optional
from pathlib import Path
from typing import List, Optional, Union

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -69,6 +70,14 @@ def rmtree_callback(function, path, excinfo):
LOG.debug("rmtree failed in %s for %s, details: %s", function, path, excinfo)


def rmtree_if_exists(path: Union[str, Path]):
"""Removes given path if the path exists"""
path_obj = Path(str(path))
if path_obj.exists():
LOG.debug("Cleaning up path %s", str(path))
shutil.rmtree(path_obj)


def stdout():
"""
Returns the stdout as a byte stream in a Py2/PY3 compatible manner
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/lib/sync/flows/test_layer_sync_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,15 @@ def test_setup_with_unknown_layer(self):
@patch("samcli.lib.sync.flows.layer_sync_flow.make_zip")
@patch("samcli.lib.sync.flows.layer_sync_flow.file_checksum")
@patch("samcli.lib.sync.flows.layer_sync_flow.os")
@patch("samcli.lib.sync.flows.layer_sync_flow.rmtree_if_exists")
def test_setup_gather_resources(
self, patched_os, patched_file_checksum, patched_make_zip, patched_tempfile, patched_app_builder
self,
patched_rmtree_if_exists,
patched_os,
patched_file_checksum,
patched_make_zip,
patched_tempfile,
patched_app_builder,
):
given_collect_build_resources = Mock()
self.build_context_mock.collect_build_resources.return_value = given_collect_build_resources
Expand All @@ -81,6 +88,8 @@ def test_setup_gather_resources(

self.layer_sync_flow.gather_resources()

layer_object = self.build_context_mock.layer_provider.get(self.layer_identifier)
patched_rmtree_if_exists.assert_called_with(layer_object.get_build_dir(self.build_context_mock.build_dir))
self.build_context_mock.collect_build_resources.assert_called_with(self.layer_identifier)

patched_app_builder.assert_called_with(
Expand Down
21 changes: 17 additions & 4 deletions tests/unit/lib/sync/flows/test_zip_function_sync_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

class TestZipFunctionSyncFlow(TestCase):
def create_function_sync_flow(self):
self.build_context_mock = MagicMock()
self.function_identifier = "Function1"
sync_flow = ZipFunctionSyncFlow(
"Function1",
build_context=MagicMock(),
self.function_identifier,
build_context=self.build_context_mock,
deploy_context=MagicMock(),
physical_id_mapping={},
stacks=[MagicMock()],
Expand All @@ -34,9 +36,18 @@ def test_set_up(self, session_mock, client_provider_mock):
@patch("samcli.lib.sync.flows.zip_function_sync_flow.make_zip")
@patch("samcli.lib.sync.flows.zip_function_sync_flow.tempfile.gettempdir")
@patch("samcli.lib.sync.flows.zip_function_sync_flow.ApplicationBuilder")
@patch("samcli.lib.sync.flows.zip_function_sync_flow.rmtree_if_exists")
@patch("samcli.lib.sync.sync_flow.Session")
def test_gather_resources(
self, session_mock, builder_mock, gettempdir_mock, make_zip_mock, file_checksum_mock, uuid4_mock, sha256_mock
self,
session_mock,
rmtree_if_exists_mock,
builder_mock,
gettempdir_mock,
make_zip_mock,
file_checksum_mock,
uuid4_mock,
sha256_mock,
):
get_mock = MagicMock()
get_mock.return_value = "ArtifactFolder1"
Expand All @@ -53,6 +64,8 @@ def test_gather_resources(
sync_flow.set_up()
sync_flow.gather_resources()

function_object = self.build_context_mock.function_provider.get(self.function_identifier)
rmtree_if_exists_mock.assert_called_once_with(function_object.get_build_dir(self.build_context_mock.build_dir))
get_mock.assert_called_once_with("Function1")
self.assertEqual(sync_flow._artifact_folder, "ArtifactFolder1")
make_zip_mock.assert_called_once_with("temp_folder" + os.sep + "data-uuid_value", "ArtifactFolder1")
Expand Down Expand Up @@ -178,7 +191,7 @@ def test_get_resource_api_calls(self, resource_api_call_mock):
function_mock = MagicMock()
function_mock.layers = [layer1, layer2]
function_mock.codeuri = "CodeUri/"
build_context.function_provider.functions.get.return_value = function_mock
build_context.function_provider.get.return_value = function_mock
sync_flow = ZipFunctionSyncFlow(
"Function1",
build_context=build_context,
Expand Down
25 changes: 24 additions & 1 deletion tests/unit/lib/utils/test_osutils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import sys

from unittest import TestCase
from unittest.mock import patch
from unittest.mock import patch, Mock
from samcli.lib.utils import osutils
from samcli.lib.utils.osutils import rmtree_if_exists


class Test_mkdir_temp(TestCase):
Expand Down Expand Up @@ -77,3 +78,25 @@ def test_must_return_sys_stdout(self, patched_open, os_walk):
patched_open.assert_any_call(os.path.join("b", target_file), "rb")
patched_open.assert_any_call(os.path.join("a", target_file), "wb")
patched_open.assert_any_call(os.path.join("b", target_file), "wb")


class Test_rmtree_if_exists(TestCase):
@patch("samcli.lib.utils.osutils.Path")
@patch("samcli.lib.utils.osutils.shutil.rmtree")
def test_must_skip_if_path_doesnt_exist(self, patched_rmtree, patched_path):
mock_path_obj = Mock()
mock_path_obj.exists.return_value = False
patched_path.return_value = mock_path_obj

rmtree_if_exists(Mock())
patched_rmtree.assert_not_called()

@patch("samcli.lib.utils.osutils.Path")
@patch("samcli.lib.utils.osutils.shutil.rmtree")
def test_must_delete_if_path_exist(self, patched_rmtree, patched_path):
mock_path_obj = Mock()
mock_path_obj.exists.return_value = True
patched_path.return_value = mock_path_obj

rmtree_if_exists(Mock())
patched_rmtree.assert_called_with(mock_path_obj)