Skip to content
This repository was archived by the owner on Oct 31, 2024. It is now read-only.

Feature/sns producer message attributes#48

Open
massa-man wants to merge 4 commits into
aspecto-io:masterfrom
riseart:feature/sns-producer-message-attributes
Open

Feature/sns producer message attributes#48
massa-man wants to merge 4 commits into
aspecto-io:masterfrom
riseart:feature/sns-producer-message-attributes

Conversation

@massa-man
Copy link
Copy Markdown

PR for issue #46

@YanivD
Copy link
Copy Markdown
Contributor

YanivD commented Mar 1, 2022

@massa-man Thank you for opening a PR and making the effort. We'll review it in the following days

Copy link
Copy Markdown
Contributor

@YanivD YanivD left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution!

Please implement few changes before we'll be able to merge and publish it:

  1. This feature should be added also for sqs-producer.
  2. Please document this feature in README file.
  3. WDYT move snsMessageAttributes prop inside an options object? In order to make it easy in the future to add new options.

🙏

@massa-man
Copy link
Copy Markdown
Author

Thanks for the comments. Happy to implement the changes proposed. As for your point 1 (add to sqs-producer), the message attributes for SNS are different from SQS message attributes. I can look at adding them here, but technically it is not the same functionality.

https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html#sqs-message-attributes

https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html

@YanivD
Copy link
Copy Markdown
Contributor

YanivD commented Mar 6, 2022

@massa-man thanks for pointing the difference between SNS message attributes and SQS message attributes.
Please ignore suggested point 1, and update the PR according point 2+3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants