search

asyncapi / bindings

AsyncAPI bindings specifications
Apache License 2.0
72 stars 75 forks source link

docs: update FilterPolicy config to match AWS API #215

Closed Gadam8 closed 1 year ago

Gadam8 commented 1 year ago

Currently, filterPolicy configuration outlined in this file 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, this definition file was missing the filterPolicyScope which is needed to determine whether the user wishes to filter on the message attributes or the body.

Description

Related issue(s)

dpwdec commented 1 year ago

LGTM! @KhudaDad414 @iancooper You happy with the change? 🐸

iancooper commented 1 year ago

@dpwdec Approved

VisualBean commented 1 year ago

whether the user wishes to filter on the message attributes or the body.

sounds odd to me. The bindings is not for users to create, it for users to consume and learn more about the topic. I dont think consumer configuration should be a part of it.

Are filterpolicies set by the event producer to be used by the consumer?

Gadam8 commented 1 year ago

whether the user wishes to filter on the message attributes or the body.

sounds odd to me. The bindings is not for users to create, it for users to consume and learn more about the topic. I dont think consumer configuration should be a part of it.

Are filterpolicies set by the event producer to be used by the consumer?

Although the SNS subscription lives under the SNS remit it is under the control of the consumer of the topic. Filter policies are set by the consumer of the event, not the producer. It is the consumer who is deciding how they wish to consume the events sent by the producer, not the producer determining how their events should be consumed.

VisualBean commented 1 year ago

Okay, Does it make sense to communicate to consumers of the specification then? I don't immediately see the value in communicating "we filter events based on this" - but maybe I'm missing something

Nevermind, it makes sense from the point of view of "we only receive events that fits this policy".

Gadam8 commented 1 year ago

Thanks @dpwdec and @iancooper for the approval. @KhudaDad414 can I get a review please?

derberg commented 1 year ago

@dpwdec @KhudaDad414 just n info for future - that when you change structure of binding, you should update binding version too