Implement ChannelCredentials for DynChannelCredentials#2674
Implement ChannelCredentials for DynChannelCredentials#2674nathanielford wants to merge 4 commits into
Conversation
…tials. To support creating channels with various credentials configurations, we need to accept different credential representations (such as concrete types or Arc-wrapped trait objects). This trait provides a common conversion interface to type-erase these representations internally. This refactoring is isolated to its own commit to keep subsequent feature commits focused and legible.
dfawley
left a comment
There was a problem hiding this comment.
I'm not sure I understand exactly why this change is needed. I think we want to convert into the Dyn version of the trait internally, and not expose that trait to anyone else.
We already have the blanket impl of DynChannelCredentials -- why isn't that good enough? Is it because the blanket impl specifies T::Output more narrowly than the channel builder does?
|
@arjan-bal I'm looking into other ways of achieving this, per Doug's comment about this maybe not being necessary, but you can look at what I'm attempting with the channel type builder here: master...nathanielford:grpc-rust-nford:implement/channel-builder As soon as that's stable I'll create it's own PR for it, which currently relies on this PR, but if we decide we don't need it I can excise this chunk easily. |
|
@nathanielford, I was looking at For context, the constructor accepts an An alternative approach I considered was defining a wrapper struct: pub struct SharedChannelCreds {
inner: Arc<dyn DynChannelCredentials>,
}The channel constructor would then accept This prevents double |
|
So, after trying to work this a couple of ways, I agree the If the signature is this: That means that the user has to pass an Alright, so assuming I'm not wrong there, it seems like this can be worked out with an For the sake of discussion I'll update this PR with the proposed impl. |
…nd Arc<T> to eliminate custom conversion boilerplate.
6029df0 to
ee091b3
Compare
|
I'm closing this since I don't think it's immediately necessary. It may be that the concern about internal channels remains but the Channel API isn't dependent on it if we accept that the user is passing an |
Pull request was closed
|
I tried passing an error[E0277]: the size for values of type `dyn dyn_wrapper::DynChannelCredentials` cannot be known at compilation time
--> grpc/src/credentials/dyn_wrapper.rs:307:38
|
307 | let ch = Channel::new("abc", creds, ChannelOptions::default());
| ------------ ^^^^^ doesn't have a size known at compile-time
| |
| required by a bound introduced by this call
|
= help: the trait `Sized` is not implemented for `dyn dyn_wrapper::DynChannelCredentials`
note: required by an implicit `Sized` bound in `channel::Channel::new`
--> grpc/src/client/channel.rs:170:16
|
170 | pub fn new<C>(target: impl Into<String>, credentials: Arc<C>, options: ChannelOptions) -> Self
| ^ required by the implicit `Sized` requirement on this type parameter in `Channel::new`
help: consider relaxing the implicit `Sized` restriction
--> grpc/src/client/channel.rs:172:30
|
172 | C: ChannelCredentials + ?Sized,
| ++++++++
For more information about this error, try `rustc --explain E0277`.This happens because the generic type I didn't catch this issue earlier. For now, I suggest sticking to the existing bounds on the |
To support creating channels with various credentials configurations, we need to accept different credential representations (such as concrete types or Arc-wrapped trait objects). This trait provides a common conversion interface to type-erase these representations internally. This refactoring is isolated to its own commit to keep subsequent feature commits focused and legible.
Motivation
When working on the Channel builder, it became clear having into traits for
DynChannelCredentialswould be useful. Because this trait is private, being able to automatically convert public implementors of theChannelCredentialstrait to what we are using internally was the ergonomic option.Solution
Add the into traits, update the relevant public traits to support it.