RSDK-6283 - parallel dial attempts #115
Conversation
| @@ -459,7 +453,7 @@ impl<T: AuthMethod> DialBuilder<T> { | |||
| let mut uri_parts = uri.clone().into_parts(); | |||
| uri_parts.scheme = Some(Scheme::HTTP); | |||
| let uri = Uri::from_parts(uri_parts)?; | |||
| Channel::builder(uri.clone()).connect().await? | |||
| Channel::builder(uri).connect().await? | |||
There was a problem hiding this comment.
(flyby) this was an unnecessary clone
| fn clone(&self) -> Self { | ||
| DialBuilder { | ||
| state: WithoutCredentials(()), | ||
| config: DialOptions { | ||
| credentials: None, | ||
| webrtc_options: self.config.webrtc_options.clone(), | ||
| uri: self.duplicate_uri(), | ||
| disable_mdns: self.config.disable_mdns, | ||
| allow_downgrade: self.config.allow_downgrade, | ||
| insecure: self.config.insecure, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
the connect() method consumes a DialBuilder, so we need to be able to clone them in order to allow parallel attempts. Incidentally, this allows us to clean up a bit some of the dial logic below.
| let original_uri2 = duplicate_uri(&original_uri).ok_or(anyhow::anyhow!( | ||
| "Attempting to connect but there was no uri" | ||
| ))?; |
There was a problem hiding this comment.
Slightly annoying that this is duplicative. original_uri2 will never fail (if it were to do so, original_uri would have failed before so we could unwrap() rather than the ok_or block, but I think adding unwraps is a bit uglier/less idiomatic than the minorly duplicative error handling here.
src/rpc/dial.rs
Outdated
| "Attempting to connect but there was no uri" | ||
| ))?; | ||
| // We want to short circuit and return the first `Ok` result from our connection | ||
| // attempts, whish `tokio::select!` does great. Buuuuut, we don't want to |
src/rpc/dial.rs
Outdated
| // attempts, whish `tokio::select!` does great. Buuuuut, we don't want to | ||
| // abandon the `Err` results, and we want to provide comprehensive logging for | ||
| // debugging purposes. Hence the loop and pinning. The pinning lets us reference | ||
| // the same future multiple times, while the loop lets immediately return on the |
There was a problem hiding this comment.
nit - "while the loop immediately returns"
Cool totally fair, thanks for looking it over! Let me know if you'd like to set aside a couple minutes to talk through the PR, I'd be happy to explain anything that's unclear :) |
No description provided.