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 WriterData field to the message struct #1059

Merged
merged 11 commits into from
Feb 17, 2023

Conversation

3AceShowHand
Copy link
Contributor

@3AceShowHand 3AceShowHand commented Jan 9, 2023

close #1054

This PR adds a new field metadata interface{} into the message struct, which can be used to hold any information, this can be helpful for the application to do post work after confirming the message is delivered.

@3AceShowHand
Copy link
Contributor Author

3AceShowHand commented Jan 10, 2023

@achille-roussel @rhansen2

Please take a look at this PR~

@dominicbarnes
Copy link
Contributor

We're open to merging this, but we'd like to rename this new field to make it's purpose and limitations as clear as possible.

Since Kafka has no built-in notion of "metadata" for messages like this, it only exists in the context of the Writer, as it will not be passed to the broker and thus never consumed downstream. Additionally, "metadata" already has usage within Kafka systems (eg: brokers) so we'd prefer to avoid any confusion there.

Would you mind renaming Metadata to WriterData? I feel like this name will be much less ambiguous, while still accomplishing the same thing for the intended use-cases.

@3AceShowHand
Copy link
Contributor Author

thanks for your reply, I will update this PR quickly.

@3AceShowHand
Copy link
Contributor Author

@dominicbarnes please take a look, the field is renamed to WriterData

@3AceShowHand 3AceShowHand changed the title add metadata field to the message struct. add WriterData field to the message struct Feb 2, 2023
@3AceShowHand
Copy link
Contributor Author

@dominicbarnes please take a look at this PR.

It looks the failed one is an unstable case, can you trigger it again?

@3AceShowHand
Copy link
Contributor Author

@dominicbarnes can you take a look at this issue ? any response is appreciated~

@3AceShowHand
Copy link
Contributor Author

@achille-roussel @rhansen2 Can you help to take a look at this issue? We hope this can be merged

@dominicbarnes
Copy link
Contributor

I've triggered the failing kafka-011 run again, as I expect it to just be a flaky test as well. I'll review this PR once more with my team today, I expect it will be merged shortly.

@dominicbarnes dominicbarnes merged commit bf8775e into segmentio:main Feb 17, 2023
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.

Need a way to attach customized information to Message
2 participants