Skip to content

fix cfn stack events resource types#7599

Merged
pinzon merged 4 commits intomasterfrom
fix/cfn-stack-events
Feb 21, 2023
Merged

fix cfn stack events resource types#7599
pinzon merged 4 commits intomasterfrom
fix/cfn-stack-events

Conversation

@pinzon
Copy link
Member

@pinzon pinzon commented Feb 2, 2023

With this change the CFn Api returns the correct resource type in the stack events. Important for cli tools that rely in the stack events for progress indicators.

@pinzon pinzon temporarily deployed to localstack-ext-tests February 2, 2023 00:06 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 40m 18s ⏱️ + 6m 35s
1 732 tests +1  1 383 ✔️ +1  349 💤 ±0  0 ±0 
2 450 runs  +1  1 759 ✔️ +1  691 💤 ±0  0 ±0 

Results for commit 18165e4. ± Comparison against base commit 2a4be3d.

♻️ This comment has been updated with latest results.

@pinzon pinzon temporarily deployed to localstack-ext-tests February 2, 2023 19:55 — with GitHub Actions Inactive
@pinzon pinzon marked this pull request as ready for review February 2, 2023 19:57
@pinzon pinzon temporarily deployed to localstack-ext-tests February 3, 2023 17:12 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the fix/cfn-stack-events branch from 20510e1 to e1a855b Compare February 6, 2023 14:46
@pinzon pinzon temporarily deployed to localstack-ext-tests February 6, 2023 14:46 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the fix/cfn-stack-events branch from e1a855b to 80e3132 Compare February 7, 2023 14:25
@pinzon pinzon temporarily deployed to localstack-ext-tests February 7, 2023 14:25 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the fix/cfn-stack-events branch from 80e3132 to 0b13c24 Compare February 9, 2023 22:01
@pinzon pinzon temporarily deployed to localstack-ext-tests February 9, 2023 22:01 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the fix/cfn-stack-events branch from 0b13c24 to 57c9169 Compare February 14, 2023 15:56
@pinzon pinzon temporarily deployed to localstack-ext-tests February 14, 2023 15:56 — with GitHub Actions Inactive
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing this.

1 minor question regarding the snapshot construction. Not blocking the merge though

Comment on lines +541 to +542
resource_types = set([event["ResourceType"] for event in events])
snapshot.match("resource_types", dict.fromkeys(resource_types, 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not just use the set or a sorted list of the types? 🤔

Copy link
Member Author

@pinzon pinzon Feb 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't occurred to me 😅

@pinzon pinzon temporarily deployed to localstack-ext-tests February 15, 2023 15:51 — with GitHub Actions Inactive
@pinzon pinzon force-pushed the fix/cfn-stack-events branch from 608cb1c to 18165e4 Compare February 17, 2023 17:13
@pinzon pinzon temporarily deployed to localstack-ext-tests February 17, 2023 17:13 — with GitHub Actions Inactive
@coveralls
Copy link

Coverage Status

Coverage: 85.035% (+0.06%) from 84.971% when pulling 18165e4 on fix/cfn-stack-events into 2a4be3d on master.

@pinzon pinzon merged commit d5934f8 into master Feb 21, 2023
@alexrashed alexrashed deleted the fix/cfn-stack-events branch March 27, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants