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

Add AWS SNS Plugin #392

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Conversation

ctasada
Copy link
Collaborator

@ctasada ctasada commented Oct 5, 2023

Although AsyncAPI allows to add more details for the binding, the underlying library jAsyncApi has no support for further binding information. Therefore, only the sns binding without properties is supported.

Manual configuration is supported through the usual @AsyncPublisher annotation in combination with the @SnsAsyncOperationBinding annotation.

The code is mostly copied and adjusted based on the other plugins.

@netlify
Copy link

netlify bot commented Oct 5, 2023

Deploy Preview for springwolf-ui canceled.

Name Link
🔨 Latest commit b1b81f1
🔍 Latest deploy log https://app.netlify.com/sites/springwolf-ui/deploys/6529409541205c0008849fa4

@ctasada ctasada force-pushed the ctasada/feat-sns-plugin branch from 0ce8dd9 to 8687385 Compare October 5, 2023 06:51
@timonback
Copy link
Member

timonback commented Oct 6, 2023

This is great!

If possible, we like to merge #386 first, which will have an impact on this PR as well.

Can you add SNS also to:

  • the README.md file
  • the RELEASING.md file
  • the springwolf-ui mock-server.ts (we can then map the sqs.demo.springwolf.dev domain)
  • update the pipeline (springwolf-plugin.yml -> matrix) after Chore/gh matrix builds #383 has been merged.

@ctasada ctasada force-pushed the ctasada/feat-sns-plugin branch 2 times, most recently from d4244a7 to 856eb08 Compare October 9, 2023 09:51
@ctasada ctasada changed the title Add AWS SQS Plugin Add AWS SNS Plugin Oct 9, 2023
@ctasada
Copy link
Collaborator Author

ctasada commented Oct 9, 2023

@timonback I just saw #370 which is a great addition.

Since this plugin is only adding a new @SnsAsyncOperationBinding and not a full integration. Does it even make sense to merge it?

SNS is intended for publishing only, and AFAICT there's no AWS annotation for binding it. So this plugin can be easily be replaced by the new GenericBinding you are working on.

@ctasada ctasada force-pushed the ctasada/feat-sns-plugin branch from 856eb08 to 35fb606 Compare October 9, 2023 10:03
@timonback
Copy link
Member

Good question. I wasn't aware that there is no snslistener, but instead theses would be sqs queues.

I like the native support of a protocol specific annotations, namely SnsAsyncOperationBinding - even if jAsyncApi does not support further binding info.

When implementing sqs, I wondered if sns should be part of the same plugin as well or whether the plugin should be called aws...
I could imagine the binding annotation to be part of sqs, as there is neither a listener nor additional dependency needed.

An own plugin just for the annotation might be too much.
What are your thoughts?

@ctasada
Copy link
Collaborator Author

ctasada commented Oct 9, 2023

SNS can be processed via SQS, HTTP,... so including it in the SQS plugin may make sense, but so adding it to a generic/core library with other bindings like Google PubSub, for example

ctasada and others added 3 commits October 13, 2023 14:31
Although AsyncAPI allows to add more details for the binding, the underlying library jAsyncApi has no support for further binding information. Therefore, only the sns binding without properties is supported.

Manual configuration is supported through the usual `@AsyncPublisher` annotation in combination with the `@SnsAsyncOperationBinding` annotation.

The code is mostly copied and adjusted based on the other plugins.
@sam0r040 sam0r040 force-pushed the ctasada/feat-sns-plugin branch from 35fb606 to b1b81f1 Compare October 13, 2023 13:05
@sam0r040 sam0r040 merged commit 359599f into springwolf:master Oct 13, 2023
11 checks passed
@timonback
Copy link
Member

The change has been merged and will be part of the next release as a new plugin :)

If you want to try and verify it in your application, use the current SNAPSHOT build as described in our README.md

Thank you for the report/contribution!

@timonback
Copy link
Member

For context: We decided that we like to extend our support of native supports, therefore adding the custom annotation in favour of having the user figure out which properties to set on the generic binding.
Also, we think the amount of effort to maintain a new plugin is doable and better than to place it somewhere, where it does not really belong to (core, sqs, ...).
Additionally, this leaves options open in case there will be better sqs support in the jAsyncApi library.

@ctasada ctasada deleted the ctasada/feat-sns-plugin branch March 29, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants