impl(gax-internal): use universe_domain when creating gRPC/HTTP transport#5302
impl(gax-internal): use universe_domain when creating gRPC/HTTP transport#5302alvarowolfx wants to merge 4 commits intogoogleapis:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5302 +/- ##
========================================
Coverage 97.78% 97.78%
========================================
Files 220 220
Lines 45833 45939 +106
========================================
+ Hits 44818 44923 +105
- Misses 1015 1016 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/gax-internal/src/grpc.rs
Outdated
| let universe_domain = | ||
| crate::universe_domain::resolve(config.universe_domain.as_deref(), &credentials) | ||
| .await | ||
| .map_err(BuilderError::transport)?; |
There was a problem hiding this comment.
Why is this a transport error? The client does not even try to initialize the transport layer. It fails before then because of bad configuration.
Should we expand the ClientBuilder::Error to have a case for mismatched universe domains?
|
|
||
| mockall::mock! { | ||
| #[derive(Debug)] | ||
| Credentials {} |
There was a problem hiding this comment.
Aside, non-blocking: We don't need to define this more than once per crate.
And we could even put it in google-cloud-test-utils to define it once for all tests
| universe_domain: &str, | ||
| ) -> Result<(Uri, String), HostError> { | ||
| let default_origin = Uri::from_str(default_endpoint).map_err(HostError::Uri)?; | ||
| let default_endpoint = default_endpoint.replace(DEFAULT_UNIVERSE_DOMAIN, universe_domain); |
There was a problem hiding this comment.
nit: this isn't really the "default" after we swap in a custom universe domain.
| custom_host.strip_suffix(&custom_suffix), | ||
| default_host.strip_suffix(&custom_suffix), |
There was a problem hiding this comment.
This seems wrong. The universe domain is not necessarily tied to the endpoint right?
Like for the .with_endpoint("foo.googleapis.com").with_universe_domain("my-universe-domain.com") example.
The host should be "foo.googleapis.com" but this would return "foo.my-universe-domain.com"
2 participants
Towards #3646