-
-
Notifications
You must be signed in to change notification settings - Fork 692
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
feat: Allow multiple filters in event source mappings #379
feat: Allow multiple filters in event source mappings #379
Conversation
main.tf
Outdated
dynamic "filter" { | ||
for_each = try(each.value.filter_criteria, []) | ||
content { | ||
pattern = try(filter.value.pattern, "") |
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.
pattern = try(filter.value.pattern, "") | |
pattern = try(filter.value.pattern, null) |
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.
done
main.tf
Outdated
filter { | ||
pattern = try(each.value["filter_criteria"].pattern, null) | ||
dynamic "filter" { | ||
for_each = try(each.value.filter_criteria, []) |
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.
Please add newline after for_each
.
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.
There should be a way to make it without introducing a break for existing users.
Please look at this PR where I did a similar change and added support for both types map
and list(map)
- terraform-aws-modules/terraform-aws-cloudfront#91
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.
Tried to do this supporting both the object and list of object configuration.
Should we use just the new notation in the examples? I kept one using the old notation for testing
@antonbabenko I pushed changes to address your PR feedback. What do you think? |
This PR has been automatically marked as stale because it has been open 30 days |
Ping |
This PR has been automatically marked as stale because it has been open 30 days |
@antonbabenko @bryantbiggs any chance we get this done? |
## [4.10.0](v4.9.0...v4.10.0) (2023-02-10) ### Features * Allow multiple filters in event source mappings ([#379](#379)) ([66eb330](66eb330))
This PR is included in version 4.10.0 🎉 |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Enhance configuration of event source mappings to support multiple filters.
Motivation and Context
Fixes #350
Breaking Changes
none
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request