Separate services from connectors #36
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the library attempted to abstract over different HTTP(s) connectors by passing them as generics through various types. Although this allows certain flexibility (e.g. choosing openssl vs rustls), it was probably the wrong abstraction layer in the first place. This change moves most of the genericism to the grpc-service layer for grpc services (bigtable and pubsub). The library's "common" path via builders will provide a sensible default implementation, but it will now be possible to substitute the entire tower::service stack provided to the grpc API. This is more flexible than the prior connector abstraction, but also harder to use (you basically have to build everything yourself). This feels like the right balance between a batteries-included default and a build-your-own option. Along the way I've also made some updates to non-exhaustivity in generated code, to help ease future changes without breaking semver. This is unfortunately less ergonomic, but not _too_ bad. Future changes may include some builder pattern to make protobuf construction easier. The GCS API is still largely the same, however it is still not in an ideal place. Future changes may bring it more towards service abstraction. Finally, this change includes updates to tonic and prost, bringing them up to the latest versions. Supersedes standard-ai#30 Fixes standard-ai#35
jneem
approved these changes
Dec 1, 2023
| + Unpin | ||
| + 'static, | ||
| C::Future: Send + Unpin + 'static, | ||
| C::Error: Into<Box<dyn std::error::Error + Send + Sync>>, |
|
An explicit example could help sure. For now, anyone industrious enough to try it should be able to find the client builder's use of it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the library attempted to abstract over different HTTP(s) connectors by passing them as generics through various types. Although this allows certain flexibility (e.g. choosing openssl vs rustls), it was probably the wrong abstraction layer in the first place.
This change moves most of the genericism to the grpc-service layer for grpc services (bigtable and pubsub). The library's "common" path via builders will provide a sensible default implementation, but it will now be possible to substitute the entire tower::service stack provided to the grpc API. This is more flexible than the prior connector abstraction, but also harder to use (you basically have to build everything yourself). This feels like the right balance between a batteries-included default and a build-your-own option.
Along the way I've also made some updates to non-exhaustivity in generated code, to help ease future changes without breaking semver. This is unfortunately less ergonomic, but not too bad. Future changes may include some builder pattern to make protobuf construction easier.
The GCS API is still largely the same, however it is still not in an ideal place. Future changes may bring it more towards service abstraction.
Finally, this change includes updates to tonic and prost, bringing them up to the latest versions.
Supersedes #30
Fixes #35