-
Notifications
You must be signed in to change notification settings - Fork 506
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
[Merged by Bors] - feat: generic implementation of tokio_util::codec::Encoder for FluvioEncoder #1081
Conversation
0c669ab
to
83020e4
Compare
@morenol is this ready for review then? |
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.
Great progress! Few changes requires.
Also, should update bump up creates version for codec, add to changelog
@simlay Can you test if your websocket works with this? You should be able to remove work around for 4 bytes issue |
0ad49ba
to
62d78e0
Compare
This looks great! @simlay This is ready. Can you test your websocket changes with this? However, there is problem with codec's version change since it is exported by wrapper crate. There are 2 ways we can solve this:
|
Nice! In that case, you don't have to bump up major version for codec. Just change to |
can you rebase from master to changes by @simlay ? |
…ric FluvioEncoder instead of raw bytes. feat: make encoder-decoder of RequestMessage and ResponseMessage symmetric feat: change decode_from_file implementation of ApiMessage to ignore data lenght since it is not present in the encoding. test: added test for the cases where it is being used tokio_util::codec::Encoder<T: FluvioEncoder> build: bump fluvio-protocol-api
@simlay can you try now? |
go thru all users of |
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.
Looks great! Excellent work
bors r+ |
…Encoder (#1081) Fixes #1075 * Changes the implementation of ResponseMessage and RequestMessage encoder to not include their lengths so the encode-decoder is symmetric now. * * Added to `FluvioCodec` the capability to handle `FluvioEncoder` types. * Add implementations of FluvioEncoder to &T: FluvioEncoder.
Pull request successfully merged into master. Build succeeded: |
Fixes #1075
Changes the implementation of ResponseMessage and RequestMessage encoder to not include their lengths so the encode-decoder is symmetric now.
Added to
FluvioCodec
the capability to handleFluvioEncoder
types.Add implementations of FluvioEncoder to &T: FluvioEncoder.