Skip to content

Commit

Permalink
Separate services from connectors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rnarubin committed Nov 30, 2023
1 parent 35ca2d7 commit 73ed0cb
Show file tree
Hide file tree
Showing 30 changed files with 1,904 additions and 1,580 deletions.
1,167 changes: 444 additions & 723 deletions Cargo.lock

Large diffs are not rendered by default.

32 changes: 14 additions & 18 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ya-gcp"
version = "0.10.0"
version = "0.11.0"
authors = ["Renar Narubin <[email protected]>"]
edition = "2021"
description = "APIs for using Google Cloud Platform services"
Expand Down Expand Up @@ -30,10 +30,10 @@ path = "examples/bigtable.rs"
required-features = ["bigtable"]

[features]
default = ["rustls"]
default = ["rustls-native-certs"]

rustls = ["hyper-rustls"]
openssl = ["hyper-openssl"] # TODO maybe should be native-tls instead?
rustls-native-certs = ["dep:rustls-native-certs", "tonic?/tls-roots"]
webpki-roots = ["dep:webpki-roots", "tonic?/tls-webpki-roots"]

# an internal feature used by services running grpc
grpc = ["tonic", "prost", "prost-types", "tower", "derive_more"]
Expand All @@ -51,27 +51,29 @@ futures = "0.3"
http = "0.2"
humantime-serde = "1"
hyper = "0.14"
hyper-rustls = "0.24.2"
paste = "1"
rand = "0.8"
rustls = "0.21.8"
serde = { version = "1", features = ["derive"] }
thiserror = "1"
tokio = { version = "1", features = ["time"] }
tracing = "0.1"
yup-oauth2 = "8.1"
tracing = "0.1.37"
yup-oauth2 = "8.3.0"

async-stream = { version = "0.3", optional = true }
async-channel = { version = "1", optional = true }
derive_more = { version = "0.99", optional = true }
hyper-openssl = { version = "0.9", optional = true }
hyper-rustls = { version = "0.22", features = ["rustls-native-certs"], optional = true }
pin-project = { version = "1.0.11", optional = true }
prost = { version = "0.11", optional = true }
prost-types = { version = "0.11", optional = true }
prost = { version = "0.12.3", optional = true }
prost-types = { version = "0.12.3", optional = true }
rustls-native-certs = { version = "0.6.3", optional = true }
tame-gcs = { version = "0.10.0", optional = true }
tempdir = { version = "0.3", optional = true }
tonic = { version = "0.9", optional = true }
tonic = { version = "0.10.2", optional = true }
tower = { version = "0.4", features = ["make"], optional = true }
uuid = { version = "0.8.1", features = ["v4"], optional = true }
uuid = { version = "1.6", features = ["v4"], optional = true }
webpki-roots = { version = "0.25.3", optional = true }

[dev-dependencies]
approx = "0.5"
Expand All @@ -84,12 +86,6 @@ tokio = { version = "1.4.0", features = ["rt-multi-thread", "time", "test-util"]
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
tracing-tree = "0.2"

[package.metadata.cargo-udeps.ignore]
# hyper-openssl is only used (and thus detected) if the feature "openssl" is
# enabled _and_ "rustls" is disabled. CI builds with --all-features, so udeps
# fails unless we ignore the package explicitly
normal = ["hyper-openssl"]

[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "docsrs"]
all-features = true
10 changes: 10 additions & 0 deletions examples/bigtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
AuthFlow::NoAuth
};

println!("Creating clients");

let config = ClientBuilderConfig::new().auth_flow(auth);
let builder = ClientBuilder::new(config).await?;

Expand All @@ -51,6 +53,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
)
.await?;

println!("Creating table `{}`", &args.table_name);

match admin
.create_table(
&args.table_name,
Expand All @@ -69,13 +73,18 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
}
}

println!("Reading tables");

let tables: Vec<_> = admin.list_tables().await?.collect().await;

println!("got tables {:?}", tables);

let mut client = builder
.build_bigtable_client(bigtable_config, &args.project_name, &args.instance_name)
.await?;

println!("setting data");

client
.set_row_data(
&args.table_name,
Expand All @@ -84,6 +93,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
[("col1", "value"), ("col2", "value")],
)
.await?;

println!("set data done");
println!(
"all data: {:?}",
Expand Down
62 changes: 43 additions & 19 deletions examples/pubsub_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,31 +58,38 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
println!("Creating topic {}", &topic_name);

publisher
.create_topic(pubsub::api::Topic {
name: topic_name.clone().into(),
..pubsub::api::Topic::default()
.raw_api_mut()
.create_topic({
let mut t = pubsub::api::Topic::default();
t.name = topic_name.clone().into();
t
})
.await?;

println!("Creating subscription {}", &subscription_name);

subscriber
.create_subscription(pubsub::api::Subscription {
name: subscription_name.clone().into(),
topic: topic_name.clone().into(),
..pubsub::api::Subscription::default()
.raw_api_mut()
.create_subscription({
let mut s = pubsub::api::Subscription::default();
s.name = subscription_name.clone().into();
s.topic = topic_name.clone().into();
s
})
.await?;

println!("Publishing messages to topic");

futures::stream::iter(0u32..100)
.map(|i| pubsub::api::PubsubMessage {
data: format!("message-{}", i).into(),
..pubsub::api::PubsubMessage::default()
futures::stream::iter(0u32..15)
.map(|i| {
let mut m = pubsub::api::PubsubMessage::default();
let payload = format!("message-{:02}", i);
println!("Sending `{payload}`");
m.data = payload.into();
m
})
.map(Ok)
.forward(publisher.publish_topic_sink(topic_name.clone()))
.forward(publisher.publish_topic_sink(topic_name.clone(), pubsub::PublishConfig::default()))
.await?;

println!("Reading back published messages");
Expand All @@ -93,32 +100,49 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
);
futures::pin_mut!(read_stream);

for i in 0u32..100 {
let mut messages = Vec::new();
for _ in 0u32..15 {
let (ack_token, message) = read_stream
.next()
.await
.ok_or("unexpected end of stream")??;

ack_token.ack().await?;
let payload = std::str::from_utf8(&message.data[..]).unwrap();
println!("Received `{payload}`");
messages.push(payload.to_owned());

assert_eq!(message.data, format!("message-{}", i));
ack_token.ack().await?;
}

messages.sort();
assert_eq!(
messages,
(0..15)
.map(|i| format!("message-{:02}", i))
.collect::<Vec<_>>()
);

println!("All messages matched!");

println!("Deleting subscription {}", &subscription_name);

subscriber
.delete_subscription(pubsub::api::DeleteSubscriptionRequest {
subscription: subscription_name.into(),
.raw_api_mut()
.delete_subscription({
let mut d = pubsub::api::DeleteSubscriptionRequest::default();
d.subscription = subscription_name.into();
d
})
.await?;

println!("Deleting topic {}", &topic_name);

publisher
.delete_topic(pubsub::api::DeleteTopicRequest {
topic: topic_name.into(),
.raw_api_mut()
.delete_topic({
let mut d = pubsub::api::DeleteTopicRequest::default();
d.topic = topic_name.into();
d
})
.await?;

Expand Down
4 changes: 2 additions & 2 deletions generators/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ harness = false
[dependencies]
anyhow = "1"
flate2 = "1"
prost-build = { version = "0.11", features = ["format"] }
prost-build = { version = "0.12.3", features = ["format"] }
reqwest = { version = "0.11", features = ["blocking"] }
structopt = "0.3"
tar = "0.4"
tempfile = "3"
tonic-build = "0.9"
tonic-build = "0.10"

[dev-dependencies]
criterion = { version = "0.3", features = ["html_reports"] }
Expand Down
23 changes: 19 additions & 4 deletions generators/src/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,36 @@ fn main() -> Result<(), Error> {
// the wire
prost_config.bytes(&["."]);

// The bigtable docs have doc comments that trigger test failures.
// (TODO: in newer versions of prost-build, the `format` option might be enough for this)
prost_config.disable_comments(&[
// Some docs have doc comments that trigger test failures because they're not actually rust.
// Maybe future codegen will be better?
// Try to keep some docs by pretending the fields don't exist during doctests
for ignored_field in [
"bigtable.v2.RowFilter.Interleave.filters",
"bigtable.v2.ReadRowsRequest.reversed",
] {
prost_config.field_attribute(ignored_field, "#[cfg(not(doctest))]");
}

// Other types can't be pretended away because typechecks will fail. just disable the comments
prost_config.disable_comments(&[
"bigtable.v2.RowFilter.sink",
"iam.v1.Policy",
"iam.v1.AuditConfig",
"iam.v1.AuditLogConfig",
"type.Expr",
]);

// the attributes map tend to have a small number of string keys, which are faster to access
// using a btree than a hashmap. See the crate's benchmarks
prost_config.btree_map(&["PubsubMessage.attributes"]);

// Declare all the generated structs and enums as non_exhaustive.
//
// This helps to reconcile two distinct semver conventions:
// 1. in protobuf, adding fields is a semver minor change
// 2. in prost codegen, structs are composed of all-pub fields, therefore adding a new field is
// a semver *major* change
prost_config.type_attribute(".", "#[non_exhaustive]");

tonic_build::configure()
.build_client(true)
.build_server(false)
Expand Down
27 changes: 9 additions & 18 deletions src/auth/grpc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Authorization support for gRPC requests
use crate::auth::Auth;
use futures::future::BoxFuture;
use std::{
sync::Arc,
Expand Down Expand Up @@ -45,15 +46,15 @@ use tonic::client::GrpcService;
/// # };
/// ```
#[derive(Clone)]
pub struct AuthGrpcService<Service, C> {
pub struct AuthGrpcService<Service> {
inner: Service,
auth: Option<crate::Auth<C>>,
scopes: Arc<Vec<String>>,
auth: Option<Auth>,
scopes: Arc<[String]>,
}

impl<Service, C> AuthGrpcService<Service, C> {
impl<Service> AuthGrpcService<Service> {
/// Wrap the given service to add authorization headers to each request
pub fn new<ReqBody>(service: Service, auth: Option<crate::Auth<C>>, scopes: Vec<String>) -> Self
pub fn new<ReqBody>(service: Service, auth: Option<Auth>, scopes: Vec<String>) -> Self
where
// Generic bounds included on the constructor because having them only on the trait impl
// doesn't produce good compiler diagnostics
Expand All @@ -63,7 +64,7 @@ impl<Service, C> AuthGrpcService<Service, C> {
Self {
inner: service,
auth,
scopes: Arc::new(scopes),
scopes: Arc::from(scopes),
}
}
}
Expand Down Expand Up @@ -93,20 +94,11 @@ where
Grpc(ServiceErr),
}

impl<Service, C, ReqBody> GrpcService<ReqBody> for AuthGrpcService<Service, C>
impl<Service, ReqBody> GrpcService<ReqBody> for AuthGrpcService<Service>
where
Service: GrpcService<ReqBody> + Clone + Send + 'static,
Service::Error: std::error::Error + Send + Sync + 'static,
Service::Future: Send,
C: tower::Service<http::Uri> + Clone + Send + Sync + 'static,
C::Response: hyper::client::connect::Connection
+ tokio::io::AsyncRead
+ tokio::io::AsyncWrite
+ Send
+ Unpin
+ 'static,
C::Future: Send + Unpin + 'static,
C::Error: Into<Box<dyn std::error::Error + Send + Sync>>,
ReqBody: Send + 'static,
{
type Error = AuthGrpcError<Service::Error, yup_oauth2::Error>;
Expand Down Expand Up @@ -290,8 +282,7 @@ mod test {
}
}

let mut auth_service =
AuthGrpcService::<_, crate::DefaultConnector>::new(OkService, None, vec![]);
let mut auth_service = AuthGrpcService::<_>::new(OkService, None, vec![]);

let result = auth_service.call(http::request::Request::new(())).await;

Expand Down
4 changes: 4 additions & 0 deletions src/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
#[cfg(feature = "grpc")]
pub mod grpc;

pub(crate) type Auth = yup_oauth2::authenticator::Authenticator<
hyper_rustls::HttpsConnector<hyper::client::HttpConnector>,
>;

/// Add the given authorization token to the given HTTP request
///
/// Returns an error if the token cannot form a valid HTTP header value.
Expand Down
Loading

0 comments on commit 73ed0cb

Please sign in to comment.