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.

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(bindings): update FilterPolicy to match AWS API #128

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

Gadam8
Copy link
Contributor

@Gadam8 Gadam8 commented Sep 19, 2023

About the PR

Currently, the FilterPolicy configuration does not match what is outlined in the AWS documentation and API - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-sns-subscription.html. The FilterPolicy should only contain the JSON policy, so no need for the attributes property; which in itself was confusing as the policy can apply to the message attributes or body.

Further to this, FilterPolicyScope was missing which is needed to determine whether the user wishes to filter on the message attributes or the body. I have a PR raised in the AsyncAPI bindings repo to make the changes to the binding documentation - asyncapi/bindings#215. This has been approved and is awaiting merge.

Changelog

  • Add: Added FilterPolicyScope to the Consumer object on the SNS operation binding.
  • Add: Adjusted FilterPolicy to remove attributes field and make the object the direct input for the policy

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you Gadam8 for your first pull request to AsyncAPI.NET repository. Please check out our contributors guide.

@VisualBean
Copy link
Contributor

@dpwdec for reference

VisualBean
VisualBean previously approved these changes Sep 20, 2023
Copy link
Contributor

@VisualBean VisualBean left a comment

Choose a reason for hiding this comment

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

Looks good. As long as @dpwdec is happy, and this follows the Binding - I dont think there are any problems.

@VisualBean VisualBean dismissed their stale review September 22, 2023 08:28

missing extension

@dpwdec
Copy link
Contributor

dpwdec commented Sep 22, 2023

@Gadam8 LGTM! I've approved the binding on the AsyncAPI side, just a case of waiting for lead maintainers to approve now.

@VisualBean Do you think we should leave this open until a merge goes through on the bindings project? Or can we merge this change now in anticipation of the correction on the bindings side?

@VisualBean VisualBean changed the title feat: update FilterPolicy to match AWS API feat(bindings): update FilterPolicy to match AWS API Sep 23, 2023
@Gadam8
Copy link
Contributor Author

Gadam8 commented Sep 25, 2023

Binding PR has now been merged asyncapi/bindings#215

@VisualBean VisualBean merged commit 5b64654 into LEGO:main Sep 25, 2023
@Gadam8 Gadam8 deleted the filterPolicy branch September 26, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants