Skip to content
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

Jump to bottom

Add AWS::Events::* resource providers for CloudFormation #9438

Merged
merged 6 commits into from Oct 24, 2023
Merged

Add AWS::Events::* resource providers for CloudFormation #9438

merged 6 commits into from Oct 24, 2023

Conversation

Morijarti
Copy link
Contributor

@Morijarti Morijarti commented Oct 23, 2023 •
edited

Changes

Added:

  • "AWS::Events::Connection"
  • "AWS::Events::EventBus"
  • "AWS::Events::Rule"
  • "AWS::Events::EventBusPolicy"

@Morijarti Morijarti added semver: patch Non-breaking changes which can be included in patch releases aws:cloudformation AWS CloudFormation labels Oct 23, 2023
@coveralls
Copy link

coveralls commented Oct 23, 2023 •
edited

Coverage Status

Changes unknown when pulling f7a64ec on feat/cfn_events into ** on master**.

@github-actions
Copy link

github-actions bot commented Oct 23, 2023 •
edited

LocalStack Community integration with Pro

       2 files         2 suites   1h 17m 33s ⏱️
2 268 tests 1 691 ✔️ 577 💤 0 ❌
2 269 runs  1 691 ✔️ 578 💤 0 ❌

Results for commit f7a64ec.

♻️ This comment has been updated with latest results.

@Morijarti Morijarti marked this pull request as ready for review October 23, 2023 14:05
simonrw
simonrw approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

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

Only a minor comment about making a note about the exception handling, nothing blocking 🎉

localstack/services/events/resource_providers/aws_events_eventbuspolicy.py
try:
events.remove_permission(**params)
except ClientError as err:
is_resource_not_found = err.response["Error"]["Code"] == "ResourceNotFoundException"
Copy link
Contributor

Choose a reason for hiding this comment

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

As said in another PR: could we add a comment saying that this exception only might happen because of incorrect dependency resolution in the CFn engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing

@Morijarti
Added explanation of reasons behind try/except block when deleting re…
82914ec 
…source.

This change became necessary due to CloudFormation's failure to correctly resolve dependencies between resources, potentially causing the deletion of this resource if its parent resource is deleted first.
@Morijarti
Merge branch 'master' into feat/cfn_events
f7a64ec 
# Conflicts:
#	localstack/services/cloudformation/resource_provider.py
@Morijarti Morijarti merged commit ba25a38 into master Oct 24, 2023
27 checks passed
@Morijarti Morijarti deleted the feat/cfn_events branch October 24, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonrw simonrw simonrw approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

@pinzon pinzon Awaiting requested review from pinzon pinzon is a code owner

Assignees
No one assigned
Labels
aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Morijarti @coveralls @simonrw