-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(sns): add notExistsCondition method #34712
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
cd6ddb2
to
4f4eafd
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
f5dfe11
to
3ab6c4b
Compare
denylist: ['white', 'orange'], | ||
})), | ||
}), | ||
price: sns.Filter.filter(sns.SubscriptionFilter.numericFilter({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarification Request:
Does it make sense to specify multiple numeric filters that contradict each other? I assume the message has to match all filter rules to be sent to the SQS queue (logic is AND not OR), but the allowed values 100
and 200
will not satisfy the other numeric filters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading the documentation, it appears the OR logic would apply. This means the filter would match any numeric value due to the greaterThan: 500
and lessThan: 1000
constraints.
So, it depends on what the intended purpose of such an integration test is. Should it just check the CloudFormation stack is built and deployed, or also test the behavior of the filter does what is expected and messages get filtered accordingly to subscribers?
packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-sqs.ts
Show resolved
Hide resolved
e25ebe1
to
b2d43cd
Compare
b2d43cd
to
0143a4c
Compare
… work as expected
0143a4c
to
6c0eb9d
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue # (if applicable)
Closes #34707
Reason for this change
Set subscription filter with exists
false
for a property.Description of changes
Add new
notExistsCondition()
method.Describe any new or updated permissions being added
None.
Description of how you validated changes
Extended existing integration test.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license