deps: upgrade to tokio v1.0 ecosystem #717
Conversation
|
tokio-openssl is updated. |
tokio-postgres/src/binary_copy.rs
Outdated
| @@ -153,7 +153,7 @@ impl Stream for BinaryCopyOutStream { | |||
| Some(header) => header.has_oids, | |||
| None => { | |||
| check_remaining(&chunk, HEADER_LEN)?; | |||
| if &chunk.bytes()[..MAGIC.len()] != MAGIC { | |||
| if &chunk.chunk()[..MAGIC.len()] != MAGIC { | |||
There was a problem hiding this comment.
You should be able to just do if !chunk.starts_with(&MAGIC) here I think.
There was a problem hiding this comment.
chunk is a Cursor<Bytes> which unfortunately does not implement AsRef<[u8]>. Switched to !chunk.chunk().starts_with(&MAGIC).
The trait method Buf::chunk is not documented to return the whole slice, but the implementation of Buf for Cursor<Bytes> does make that guarantee. If relying on that is scary, we could instead do chunk.read_bytes(MAGIC.len()) != MAGIC, which is safe for any implementation of Buf.
There was a problem hiding this comment.
Oh - we should just remove the Cursor layer entirely since Bytes implements Buf directly now.
There was a problem hiding this comment.
I tried that, but we need access to the cursor position: https://github.com/sfackler/rust-postgres/blob/master/tokio-postgres/src/binary_copy.rs#L201
Could always do some pointer arithmetic to compute the position, but didn’t seem like an obvious win.
There was a problem hiding this comment.
Ah right - should be fine as-is then.
postgres/src/copy_out_reader.rs
Outdated
| @@ -46,7 +46,7 @@ impl BufRead for CopyOutReader<'_> { | |||
| }; | |||
| } | |||
|
|
|||
| Ok(self.cur.bytes()) | |||
| Ok(self.cur.chunk()) | |||
There was a problem hiding this comment.
I think this can just be Ok(&self.cur).
benesch
force-pushed
the
tokio-10
branch
3 times, most recently
from
456fe29 to
56876af
Compare
|
All the TLS related crates have new releases now, so this is ready to go pending whatever review feedback you might have, @sfackler. As always feel free to just take over the PR. |
postgres-openssl/src/lib.rs
Outdated
|
|
||
| fn connect(self, stream: S) -> Self::Future { | ||
| let future = async move { | ||
| let stream = tokio_openssl::connect(self.ssl, &self.domain, stream).await?; | ||
| let ssl = self.ssl.into_ssl(&self.domain)?; | ||
| let mut stream = Box::pin(SslStream::new(ssl, stream)?); |
There was a problem hiding this comment.
Since S is Unpin, SslStream<S> is as well so there's no need to use Box::pin here. You should be able to just do Pin::new(&mut stream).connect().await;
There was a problem hiding this comment.
Ah yeah, thanks, that's much nicer. I missed the Unpin bound on S.
postgres-openssl/src/lib.rs
Outdated
| let stream = tokio_openssl::connect(self.ssl, &self.domain, stream).await?; | ||
| let ssl = self.ssl.into_ssl(&self.domain)?; | ||
| let mut stream = Box::pin(SslStream::new(ssl, stream)?); | ||
| stream.as_mut().connect().await?; |
There was a problem hiding this comment.
I think it's worth doing something just a little bit more complicated to get better diagnostics after a certificate validation failure: https://github.com/sfackler/hyper-openssl/blob/master/src/lib.rs#L237-L240
There was a problem hiding this comment.
SGTM! I just shamelessly copy/pasted that code into here.
postgres-openssl/src/lib.rs
Outdated
| @@ -134,13 +134,15 @@ where | |||
| S: AsyncRead + AsyncWrite + Unpin + Debug + 'static + Sync + Send, | |||
There was a problem hiding this comment.
You can remove the Debug + 'static + Sync bounds from here now.
There was a problem hiding this comment.
Ok, Debug and Sync are gone. Compiler still insists on 'static though.
|
Thanks! Looks good other than those few nits. I think I'm probably just going to immediately cut a release with these dependency upgrades to get that out the door, and then start looking into cleaning things up for a 1.0 release from there. |
@sfackler this is still waiting on releases of both tokio-openssl and tokio-native-tls that include the Tokio v1.0 bump, but I got tests for tokio-postgres and postgres passing, so figured I'd put this up.