googleapis / google-cloud-cpp

C++ Client Libraries for Google Cloud Services
https://cloud.google.com/
Apache License 2.0
554 stars 373 forks source link

Add per-call configuration options to Client APIs #7666

Closed dbolduc closed 2 years ago

dbolduc commented 2 years ago

Overview

This would offer a general framework for implementing a solution to something like #4926. (In addition to timeouts, it could be used to add Metadata, serve some service specific function, etc.).

The Client calls would all gain an Options parameter:

// calls like:
Status Foo(FooRequest const& request);
// ...become:
Status Foo(FooRequest const& request, Options const& options = {});

The Connection calls would also gain an Options parameter.

Details / Open Questions

Jotting down some thoughts from a previous discussion with the team:

While we probably have enough direction to start implementing the API surface, I think we need a clearer picture of what a per-call option implementation looks like.

Appendix

Just to give an idea of what I currently have in mind (setting deadlines) (for gRPC):

/// FILE: google/cloud/common_options.h
namespace google::cloud {
struct DeadlineOption {
  using Type = std::chrono::time_point<std::chrono::system_clock>;
};
}  // namespace

/// FILE: google/cloud/grpc_options.h
namespace google::cloud::internal {
struct GrpcSetupOption {
  using Type = std::function<void(grpc::ClientContext&)>;
};

// TODO : Whether this alters a ClientContext& or returns a new, configured
//              ClientContext can be figured out at a later date.
void ConfigureContext(grpc::ClientContext& context, Options const& opts) {
  // TODO : Which option takes precedence, and whether they are exclusive
  //        can be decided later.
  if (opts.has<GrpcSetupOption>()) {
    opts.get<GrpcSetupOption>()(context);
  } else if (opts.has<DeadlineOption>()) {
    context.set_deadline(opts.get<DeadlineOption>());
  }
}
}  // namespace
coryan commented 2 years ago

This looks like a duplicate of #7198, I will let @devjgm and @dbolduc decide which one should be closed.

dbolduc commented 2 years ago

Whoops. I did not do my homework before writing up this issue. I am going to close this one, but copy the text over to the original issue as a comment.