fix: add conditions to events and add tests #755
Conversation
Codecov Report
@@ Coverage Diff @@
## release/v1.10.0 #755 +/- ##
==================================================
+ Coverage 94.18% 94.5% +0.31%
==================================================
Files 67 67
Lines 2685 2691 +6
Branches 478 481 +3
==================================================
+ Hits 2529 2543 +14
+ Misses 80 77 -3
+ Partials 76 71 -5
Continue to review full report at Codecov.
|
keetonian
force-pushed
the
release/v1.10.0-fix
branch
from
37b6e02
to
85ba8e1
Compare
keetonian
force-pushed
the
release/v1.10.0-fix
branch
from
85ba8e1
to
281bd2d
Compare
| @@ -47,5 +47,7 @@ def get_subscription_filter(self, function, permission): | |||
| subscription_filter.LogGroupName = self.LogGroupName | |||
| subscription_filter.FilterPattern = self.FilterPattern | |||
| subscription_filter.DestinationArn = function.get_runtime_attr("arn") | |||
| if 'Condition' in function.resource_attributes: | |||
There was a problem hiding this comment.
Is there a way we can not have to check this in every event source (and all new event sources moving forward)? Is there something we can do that's more DRY? I understand for certain event sources, we may need to override and do something different, e.g., S3. If there's no obvious way to do this, then I'm ok leaving it this way.
There was a problem hiding this comment.
I think so, I'll look into it when I get a chance.
| @@ -64,6 +64,8 @@ def to_cloudformation(self, **kwargs): | |||
| lambda_eventsourcemapping.StartingPosition = self.StartingPosition | |||
| lambda_eventsourcemapping.BatchSize = self.BatchSize | |||
| lambda_eventsourcemapping.Enabled = self.Enabled | |||
| if 'Condition' in function.resource_attributes: | |||
There was a problem hiding this comment.
If would be awesome to have a way to do this as a function and not need to worry about it in every resource. As we add more Events, resources, etc, this can be very easily forgotten.
This reverts commit 47b221e.
Issue #, if available:
#142
Description of changes:
Adds another test for Conditions, as well as makes sure they are added to all Event types (except for API). Will follow up with a fix for Conditions for API event types (issue #758).
There were a few bugs in the Conditions feature, including how Events were handled. This pull request fixes these bugs.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.