-
Notifications
You must be signed in to change notification settings - Fork 83
Adding the support for key for trigger & output binding #328
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
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…nctions-kafka-extension into shrohilla/add-key-support
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.
You might add a simple unit testing for the new logic.
src/Microsoft.Azure.WebJobs.Extensions.Kafka/Output/KafkaProducerAsyncCollector.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Azure.WebJobs.Extensions.Kafka/Output/KafkaProducerAsyncCollector.cs
Show resolved
Hide resolved
src/Microsoft.Azure.WebJobs.Extensions.Kafka/Trigger/KafkaTriggerAttributeBindingProvider.cs
Show resolved
Hide resolved
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.
Should we set the Key as well in the constructor at line 110 in KafkaEventData.cs?
Yes, even if the key is null it will write the key as null and it's customer choice in his code if it wants to read that or not. |
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.
LGTM
Kafka supported any library provide customer the support of reading key & value(message), so this is a required support for the customer