Experiment with custom service for bigtable. #30
Closed
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
rnarubin
added a commit
to rnarubin/ya-gcp
that referenced
this pull request
Nov 30, 2023
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. Supersedes standard-ai#30 Fixes standard-ai#35
rnarubin
added a commit
to rnarubin/ya-gcp
that referenced
this pull request
Nov 30, 2023
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
rnarubin
added a commit
that referenced
this pull request
Dec 1, 2023
* Separate services from connectors 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
|
Replicated in #36 |
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.
This is a little outline for what "BYO grpcservice" could look like, just for bigtable so far. I couldn't figure out how to rope
ClientBuilderinto this, because the whole point ofClientBuilderis to reuse some of the service-building configuration.If this seems reasonable, I think we could even go further by removing the
Cparameter fromClientBuilderand just always useDefaultConnector, the idea being thatClientBuilderis the "easy" mode without customization and so if you want to customize the connector, just customize the whole service.