-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Consumer can getValue return null #4848
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
Consumer can getValue return null #4848
Conversation
|
run Integration Tests |
|
run java8 tests |
|
run Integration Tests |
|
run java8 tests |
| } | ||
| } else { | ||
| return schema.decode(getData()); | ||
| if (getData().length == 0) { |
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 think the fix here is a bit problematic. because an application can produce a zero-length bytes array. Can we find a way to differentiate a zero-length bytes payload from a null payload?
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.
we should use metadata to store the null payload.
|
@congbobo184 are you still working on this change? |
|
I am not working on it.It can be moved to the next version. |
|
Close this PR since #6379 has fixed the problem. |
Motivation
Consumer can getValue return null
fix the #4803
Verifying this change
Add the tests for it
Does this pull request potentially affect one of the following parts:
If yes was chosen, please highlight the changes
Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (yes)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)
Documentation
Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation