-
Notifications
You must be signed in to change notification settings - Fork 249
feat(java): Implement user headers serialization / deserialization in java sdk #2516
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
|
Since |
mmodzelewski
left a comment
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.
Thanks, @slbotbm. I left some minor comments. Otherwise, the PR looks great.
foreign/java/java-sdk/src/main/java/org/apache/iggy/serde/BytesDeserializer.java
Outdated
Show resolved
Hide resolved
foreign/java/java-sdk/src/main/java/org/apache/iggy/serde/BytesDeserializer.java
Outdated
Show resolved
Hide resolved
foreign/java/java-sdk/src/main/java/org/apache/iggy/message/Message.java
Outdated
Show resolved
Hide resolved
|
@mmodzelewski thank you for the review! I have addressed all of your comments. After removing the optional mapping from userHeaders, some http integration tests were failing due to jackson desrializing empty userHeaders as null. To address this, I added the annotation |
foreign/java/java-sdk/src/main/java/org/apache/iggy/message/Message.java
Outdated
Show resolved
Hide resolved
...ggy-connector-library/src/main/java/org/apache/iggy/connector/flink/sink/IggySinkWriter.java
Outdated
Show resolved
Hide resolved
foreign/java/java-sdk/src/main/java/org/apache/iggy/message/Message.java
Outdated
Show resolved
Hide resolved
|
@slbotbm thanks, added a few comments regarding the new code. |
|
@slbotbm all looks great, thanks again for the contribution! :) |
closes #2455