From c12b5d9277cab5fa5577f439702f3aaf81a36655 Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Fri, 31 Jan 2020 15:32:34 +0900 Subject: [PATCH 1/6] When reached old entries, skip and go next iteration. --- samcli/lib/deploy/deployer.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index 8c19250741..3a0b49e059 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -336,10 +336,14 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): paginator = self._client.get_paginator("describe_stack_events") response_iterator = paginator.paginate(StackName=stack_name) stack_status = describe_stacks_resp["Stacks"][0]["StackStatus"] + latest_time_stamp_marker = time_stamp_marker + is_reached_old_entry = False for event_items in response_iterator: for event in event_items["StackEvents"]: - if event["EventId"] not in events and utc_to_timestamp(event["Timestamp"]) > time_stamp_marker: + current_time_stamp_marker = utc_to_timestamp(event["Timestamp"]) + if event["EventId"] not in events and current_time_stamp_marker > time_stamp_marker: events.add(event["EventId"]) + latest_time_stamp_marker = max(latest_time_stamp_marker, current_time_stamp_marker) row_color = self.deploy_color.get_stack_events_status_color(status=event["ResourceStatus"]) pprint_columns( columns=[ @@ -355,6 +359,12 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): columns_dict=DESCRIBE_STACK_EVENTS_DEFAULT_ARGS.copy(), color=row_color, ) + elif utc_to_timestamp(event["Timestamp"]) < time_stamp_marker: + time_stamp_marker = latest_time_stamp_marker + is_reached_old_entry = True + break + if is_reached_old_entry: + break if self._check_stack_complete(stack_status): stack_change_in_progress = False From 0d154b25a45817c068f03709035d16573950d4b0 Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Sat, 1 Feb 2020 13:50:48 +0900 Subject: [PATCH 2/6] Add comment. --- samcli/lib/deploy/deployer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index 3a0b49e059..1686812e7c 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -340,10 +340,9 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): is_reached_old_entry = False for event_items in response_iterator: for event in event_items["StackEvents"]: - current_time_stamp_marker = utc_to_timestamp(event["Timestamp"]) - if event["EventId"] not in events and current_time_stamp_marker > time_stamp_marker: + if event["EventId"] not in events and utc_to_timestamp(event["Timestamp"]) > time_stamp_marker: events.add(event["EventId"]) - latest_time_stamp_marker = max(latest_time_stamp_marker, current_time_stamp_marker) + latest_time_stamp_marker = max(latest_time_stamp_marker, utc_to_timestamp(event["Timestamp"])) row_color = self.deploy_color.get_stack_events_status_color(status=event["ResourceStatus"]) pprint_columns( columns=[ @@ -359,6 +358,7 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): columns_dict=DESCRIBE_STACK_EVENTS_DEFAULT_ARGS.copy(), color=row_color, ) + # Skip already shown old event entries elif utc_to_timestamp(event["Timestamp"]) < time_stamp_marker: time_stamp_marker = latest_time_stamp_marker is_reached_old_entry = True From 572eca5145b31f5072a68577ef93fb5ac6a9de09 Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Sat, 1 Feb 2020 14:24:14 +0900 Subject: [PATCH 3/6] reformatted with black --- samcli/lib/deploy/deployer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index 1686812e7c..3284d1e6a3 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -342,7 +342,9 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): for event in event_items["StackEvents"]: if event["EventId"] not in events and utc_to_timestamp(event["Timestamp"]) > time_stamp_marker: events.add(event["EventId"]) - latest_time_stamp_marker = max(latest_time_stamp_marker, utc_to_timestamp(event["Timestamp"])) + latest_time_stamp_marker = max( + latest_time_stamp_marker, utc_to_timestamp(event["Timestamp"]) + ) row_color = self.deploy_color.get_stack_events_status_color(status=event["ResourceStatus"]) pprint_columns( columns=[ From e67f09bd169c69b52051ee0f40b61c596d32515b Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Sat, 1 Feb 2020 14:41:15 +0900 Subject: [PATCH 4/6] shorten sentence to pass lint check. --- samcli/lib/deploy/deployer.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index 3284d1e6a3..c399b6de10 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -337,7 +337,6 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): response_iterator = paginator.paginate(StackName=stack_name) stack_status = describe_stacks_resp["Stacks"][0]["StackStatus"] latest_time_stamp_marker = time_stamp_marker - is_reached_old_entry = False for event_items in response_iterator: for event in event_items["StackEvents"]: if event["EventId"] not in events and utc_to_timestamp(event["Timestamp"]) > time_stamp_marker: @@ -363,10 +362,10 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): # Skip already shown old event entries elif utc_to_timestamp(event["Timestamp"]) < time_stamp_marker: time_stamp_marker = latest_time_stamp_marker - is_reached_old_entry = True break - if is_reached_old_entry: - break + else: # go to next loop if not break from inside loop + continue + break # reached here only if break from inner loop! if self._check_stack_complete(stack_status): stack_change_in_progress = False From a425e821f5281731f9596e3c80e49cb2ddff8729 Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Sat, 8 Feb 2020 21:03:15 +0900 Subject: [PATCH 5/6] Add unit test for describe_stack_events(), checking old event skipping. --- tests/unit/lib/deploy/test_deployer.py | 72 ++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/unit/lib/deploy/test_deployer.py b/tests/unit/lib/deploy/test_deployer.py index 38d8267d3b..d1575ae7e2 100644 --- a/tests/unit/lib/deploy/test_deployer.py +++ b/tests/unit/lib/deploy/test_deployer.py @@ -364,6 +364,78 @@ def test_describe_stack_events(self, patched_time): self.deployer.describe_stack_events("test", time.time() - 1) + @patch("time.sleep") + @patch("samcli.lib.deploy.deployer.pprint_columns") + def test_describe_stack_events_skip_old_event(self, patched_pprint_columns, patched_time): + current_timestamp = datetime.utcnow() + + self.deployer._client.describe_stacks = MagicMock( + side_effect=[ + {"Stacks": [{"StackStatus": "CREATE_IN_PROGRESS"}]}, + {"Stacks": [{"StackStatus": "CREATE_IN_PROGRESS"}]}, + {"Stacks": [{"StackStatus": "CREATE_COMPLETE_CLEANUP_IN_PROGRESS"}]}, + {"Stacks": [{"StackStatus": "CREATE_COMPLETE"}]}, + ] + ) + sample_events = [ + { + "StackEvents": [ + { + "EventId": str(uuid.uuid4()), + "Timestamp": current_timestamp, + "ResourceStatus": "CREATE_IN_PROGRESS", + "ResourceType": "s3", + "LogicalResourceId": "mybucket", + } + ] + }, + { + "StackEvents": [ + { + "EventId": str(uuid.uuid4()), + "Timestamp": current_timestamp + timedelta(seconds=10), + "ResourceStatus": "CREATE_IN_PROGRESS", + "ResourceType": "kms", + "LogicalResourceId": "mykms", + } + ] + }, + { + "StackEvents": [ + { + "EventId": str(uuid.uuid4()), + "Timestamp": current_timestamp + timedelta(seconds=20), + "ResourceStatus": "CREATE_COMPLETE", + "ResourceType": "s3", + "LogicalResourceId": "mybucket", + } + ] + }, + { + "StackEvents": [ + { + "EventId": str(uuid.uuid4()), + "Timestamp": current_timestamp + timedelta(seconds=30), + "ResourceStatus": "CREATE_COMPLETE", + "ResourceType": "kms", + "LogicalResourceId": "mykms", + } + ] + }, + ] + invalid_event = {"StackEvents": [{}]} # if deployer() loop read this, KeyError would raise + self.deployer._client.get_paginator = MagicMock( + side_effect=[ + MockPaginator([sample_events[0]]), + MockPaginator([sample_events[1], sample_events[0], invalid_event]), + MockPaginator([sample_events[2], sample_events[1], invalid_event]), + MockPaginator([sample_events[3], sample_events[2], invalid_event]), + ] + ) + + self.deployer.describe_stack_events("test", time.time() - 1) + self.assertEqual(patched_pprint_columns.call_count, 4) + @patch("samcli.lib.deploy.deployer.math") @patch("time.sleep") def test_describe_stack_events_exceptions(self, patched_time, patched_math): From 0468f31666fb52e28c6ba461913e289c3da3d9b1 Mon Sep 17 00:00:00 2001 From: Shinji Matsumoto Date: Sat, 8 Feb 2020 21:05:12 +0900 Subject: [PATCH 6/6] Fix the problem with new stack. In past logic, if the stack is newly created, time_stamp_marker would never be updated. --- samcli/lib/deploy/deployer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index c399b6de10..2036b84c74 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -360,10 +360,11 @@ def describe_stack_events(self, stack_name, time_stamp_marker, **kwargs): color=row_color, ) # Skip already shown old event entries - elif utc_to_timestamp(event["Timestamp"]) < time_stamp_marker: + elif utc_to_timestamp(event["Timestamp"]) <= time_stamp_marker: time_stamp_marker = latest_time_stamp_marker break else: # go to next loop if not break from inside loop + time_stamp_marker = latest_time_stamp_marker # update marker if all events are new continue break # reached here only if break from inner loop!