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
SQS Fifo Kamelet Sink #78
Conversation
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.
Just thinking loud about these two properties, to try to find a better solution.
Here you see some Camel concepts emerging in the connector description (exchange and exchange id/properties) which should be hidden inside the connector and makes a bell to ring..
If I think to the use case of a user using this Kamelet to forward application events into a SQS FIFO queue, then we should allow them to set group and ID programmatically, which in this context means setting them in a header.
The Kamelet can specify the "group" (plus "ce-group") header as the way to specify the group and the "id" (plus "ce-id") header for the deduplication strategy, then it's responsibility of the data producer to set them in the message.
The "ce-id" field is also compatible with cloud events, so messages flowing from knative will have that field set out-of-the-box, with the right semantics.
So I'd go with an approach similar to the one in telegram-sink
here, removing those two options from the connector configuration and making them dynamic. Wdyt?
aws-sqs-fifo-sink.kamelet.yaml
Outdated
messageGroupIdStrategy: | ||
title: Message Group ID Strategy | ||
description: Strategy for setting the messageGroupId on the message. Can be one of the following options useConstant, useExchangeId, usePropertyValue. | ||
type: string | ||
default: useExchangeId | ||
messageDeduplicationIdStrategy: | ||
title: Message Deduplication ID Strategy | ||
description: Strategy for setting the messageDeduplicationId on the message. Can be one of the following options useExchangeId, useContentBasedDeduplication | ||
type: string | ||
default: useExchangeId |
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.
I was talking about these two
I've pushed what I meant.. |
with camel-k cli 1.3.1 it is failing:
|
Looks good! |
Fixes #77