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

Cleanup cassandra protocol negotiation #772

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

rukai
Copy link
Member

@rukai rukai commented Aug 30, 2022

Improves error reporting and makes the cassandra protocol implementation easier to understand.
Downgrades the warnings caused by cassandra protocol negotiation to info! level.
It certainly makes the integration tests quieter, but maybe the slight performance loss of protocol negotiation is worth a warning?

Theres some magic trait type stuff going on here:
In these lines:

pub trait CodecReadHalf: Decoder<Item = Messages, Error = CodecReadError> + Clone + Send {}
impl<T: Decoder<Item = Messages, Error = CodecReadError> + Clone + Send> CodecReadHalf for T {}

We can set any type to be the Error as long as it implements From<std::io::Error>.
Internally tokio then uses that implementation to generate errors in our type when it hits an io::Error while receiving from the TcpStream.
So while we never create a CodecReadError::Io directly, tokio will return those sometimes when we call FrameRead::next.

@rukai rukai force-pushed the cleanup_protocol_negotiation branch 2 times, most recently from f556bf7 to 7126a5b Compare August 30, 2022 23:16
@rukai rukai marked this pull request as ready for review August 31, 2022 00:10
@rukai rukai force-pushed the cleanup_protocol_negotiation branch from 7126a5b to 098c8a6 Compare August 31, 2022 07:35
@conorbros
Copy link
Member

Let's merge this, I assume Ben just forgot to hit the resolve button.

@rukai rukai merged commit f2227f6 into shotover:main Aug 31, 2022
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.

3 participants