Skip to content

Commit

Permalink
review comments + rebase
Browse files Browse the repository at this point in the history
  • Loading branch information
@dbolduc
dbolduc committed Jan 19, 2022
1 parent 3b1552e commit bbc35f8
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 125 deletions.
5 changes: 4 additions & 1 deletion generator/integration_tests/golden/tests/plumbing_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace golden {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace {

using ::testing::AtLeast;
using ::testing::Return;
using ms = std::chrono::milliseconds;

Expand Down Expand Up @@ -53,7 +54,9 @@ TEST(PlumbingTest, RetryLoopUsesPerCallPolicies) {
auto clone = absl::make_unique<MockRetryPolicy>();
// We will just say the policy is never exhausted, and use a permanent error
// to break out of the loop.
EXPECT_CALL(*clone, IsExhausted).WillRepeatedly(Return(false));
EXPECT_CALL(*clone, IsExhausted)
.Times(AtLeast(1))
.WillRepeatedly(Return(false));
EXPECT_CALL(*clone, OnFailureImpl);
return clone;
});
Expand Down
128 changes: 74 additions & 54 deletions google/cloud/automl/auto_ml_connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,16 +222,16 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultResponse<
google::cloud::automl::v1::Dataset>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->CreateDataset(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->CreateDataset(request), polling_policy(),
__func__);
}

StatusOr<google::cloud::automl::v1::Dataset> GetDataset(
google::cloud::automl::v1::GetDatasetRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->GetDataset(request),
retry_policy(), backoff_policy(),
idempotency_policy()->GetDataset(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::GetDatasetRequest const& request) {
return stub_->GetDataset(context, request);
Expand All @@ -243,11 +243,9 @@ class AutoMlConnectionImpl : public AutoMlConnection {
google::cloud::automl::v1::ListDatasetsRequest request) override {
request.clear_page_token();
auto stub = stub_;
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(
retry_policy_prototype_->clone());
auto backoff = std::shared_ptr<BackoffPolicy const>(
backoff_policy_prototype_->clone());
auto idempotency = idempotency_policy_->ListDatasets(request);
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(retry_policy());
auto backoff = std::shared_ptr<BackoffPolicy const>(backoff_policy());
auto idempotency = idempotency_policy()->ListDatasets(request);
char const* function_name = __func__;
return google::cloud::internal::MakePaginationRange<
StreamRange<google::cloud::automl::v1::Dataset>>(
Expand Down Expand Up @@ -275,8 +273,8 @@ class AutoMlConnectionImpl : public AutoMlConnection {
StatusOr<google::cloud::automl::v1::Dataset> UpdateDataset(
google::cloud::automl::v1::UpdateDatasetRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->UpdateDataset(request),
retry_policy(), backoff_policy(),
idempotency_policy()->UpdateDataset(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::UpdateDatasetRequest const& request) {
return stub_->UpdateDataset(context, request);
Expand Down Expand Up @@ -307,9 +305,9 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->DeleteDataset(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->DeleteDataset(request), polling_policy(),
__func__);
}

future<StatusOr<google::cloud::automl::v1::OperationMetadata>> ImportData(
Expand All @@ -335,9 +333,8 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->ImportData(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->ImportData(request), polling_policy(), __func__);
}

future<StatusOr<google::cloud::automl::v1::OperationMetadata>> ExportData(
Expand All @@ -363,17 +360,16 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->ExportData(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->ExportData(request), polling_policy(), __func__);
}

StatusOr<google::cloud::automl::v1::AnnotationSpec> GetAnnotationSpec(
google::cloud::automl::v1::GetAnnotationSpecRequest const& request)
override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->GetAnnotationSpec(request),
retry_policy(), backoff_policy(),
idempotency_policy()->GetAnnotationSpec(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::GetAnnotationSpecRequest const&
request) {
Expand Down Expand Up @@ -405,16 +401,15 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultResponse<
google::cloud::automl::v1::Model>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->CreateModel(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->CreateModel(request), polling_policy(), __func__);
}

StatusOr<google::cloud::automl::v1::Model> GetModel(
google::cloud::automl::v1::GetModelRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->GetModel(request),
retry_policy(), backoff_policy(),
idempotency_policy()->GetModel(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::GetModelRequest const& request) {
return stub_->GetModel(context, request);
Expand All @@ -426,11 +421,9 @@ class AutoMlConnectionImpl : public AutoMlConnection {
google::cloud::automl::v1::ListModelsRequest request) override {
request.clear_page_token();
auto stub = stub_;
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(
retry_policy_prototype_->clone());
auto backoff = std::shared_ptr<BackoffPolicy const>(
backoff_policy_prototype_->clone());
auto idempotency = idempotency_policy_->ListModels(request);
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(retry_policy());
auto backoff = std::shared_ptr<BackoffPolicy const>(backoff_policy());
auto idempotency = idempotency_policy()->ListModels(request);
char const* function_name = __func__;
return google::cloud::internal::MakePaginationRange<
StreamRange<google::cloud::automl::v1::Model>>(
Expand Down Expand Up @@ -478,16 +471,15 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->DeleteModel(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->DeleteModel(request), polling_policy(), __func__);
}

StatusOr<google::cloud::automl::v1::Model> UpdateModel(
google::cloud::automl::v1::UpdateModelRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->UpdateModel(request),
retry_policy(), backoff_policy(),
idempotency_policy()->UpdateModel(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::UpdateModelRequest const& request) {
return stub_->UpdateModel(context, request);
Expand Down Expand Up @@ -518,9 +510,8 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->DeployModel(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->DeployModel(request), polling_policy(), __func__);
}

future<StatusOr<google::cloud::automl::v1::OperationMetadata>> UndeployModel(
Expand All @@ -546,9 +537,9 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->UndeployModel(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->UndeployModel(request), polling_policy(),
__func__);
}

future<StatusOr<google::cloud::automl::v1::OperationMetadata>> ExportModel(
Expand All @@ -574,17 +565,16 @@ class AutoMlConnectionImpl : public AutoMlConnection {
},
&google::cloud::internal::ExtractLongRunningResultMetadata<
google::cloud::automl::v1::OperationMetadata>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->ExportModel(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->ExportModel(request), polling_policy(), __func__);
}

StatusOr<google::cloud::automl::v1::ModelEvaluation> GetModelEvaluation(
google::cloud::automl::v1::GetModelEvaluationRequest const& request)
override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->GetModelEvaluation(request),
retry_policy(), backoff_policy(),
idempotency_policy()->GetModelEvaluation(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::GetModelEvaluationRequest const&
request) {
Expand All @@ -597,11 +587,9 @@ class AutoMlConnectionImpl : public AutoMlConnection {
google::cloud::automl::v1::ListModelEvaluationsRequest request) override {
request.clear_page_token();
auto stub = stub_;
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(
retry_policy_prototype_->clone());
auto backoff = std::shared_ptr<BackoffPolicy const>(
backoff_policy_prototype_->clone());
auto idempotency = idempotency_policy_->ListModelEvaluations(request);
auto retry = std::shared_ptr<AutoMlRetryPolicy const>(retry_policy());
auto backoff = std::shared_ptr<BackoffPolicy const>(backoff_policy());
auto idempotency = idempotency_policy()->ListModelEvaluations(request);
char const* function_name = __func__;
return google::cloud::internal::MakePaginationRange<
StreamRange<google::cloud::automl::v1::ModelEvaluation>>(
Expand All @@ -628,6 +616,38 @@ class AutoMlConnectionImpl : public AutoMlConnection {
}

private:
std::unique_ptr<AutoMlRetryPolicy> retry_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<AutoMlRetryPolicyOption>()) {
return options.get<AutoMlRetryPolicyOption>()->clone();
}
return retry_policy_prototype_->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<AutoMlBackoffPolicyOption>()) {
return options.get<AutoMlBackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
}

std::unique_ptr<PollingPolicy> polling_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<AutoMlPollingPolicyOption>()) {
return options.get<AutoMlPollingPolicyOption>()->clone();
}
return polling_policy_prototype_->clone();
}

std::unique_ptr<AutoMlConnectionIdempotencyPolicy> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<AutoMlConnectionIdempotencyPolicyOption>()) {
return options.get<AutoMlConnectionIdempotencyPolicyOption>()->clone();
}
return idempotency_policy_->clone();
}

std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<automl_internal::AutoMlStub> stub_;
std::unique_ptr<AutoMlRetryPolicy const> retry_policy_prototype_;
Expand Down
44 changes: 39 additions & 5 deletions google/cloud/automl/prediction_connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ class PredictionServiceConnectionImpl : public PredictionServiceConnection {
StatusOr<google::cloud::automl::v1::PredictResponse> Predict(
google::cloud::automl::v1::PredictRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->Predict(request),
retry_policy(), backoff_policy(),
idempotency_policy()->Predict(request),
[this](grpc::ClientContext& context,
google::cloud::automl::v1::PredictRequest const& request) {
return stub_->Predict(context, request);
Expand Down Expand Up @@ -104,12 +104,46 @@ class PredictionServiceConnectionImpl : public PredictionServiceConnection {
},
&google::cloud::internal::ExtractLongRunningResultResponse<
google::cloud::automl::v1::BatchPredictResult>,
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->BatchPredict(request),
polling_policy_prototype_->clone(), __func__);
retry_policy(), backoff_policy(),
idempotency_policy()->BatchPredict(request), polling_policy(),
__func__);
}

private:
std::unique_ptr<PredictionServiceRetryPolicy> retry_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<PredictionServiceRetryPolicyOption>()) {
return options.get<PredictionServiceRetryPolicyOption>()->clone();
}
return retry_policy_prototype_->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<PredictionServiceBackoffPolicyOption>()) {
return options.get<PredictionServiceBackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
}

std::unique_ptr<PollingPolicy> polling_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<PredictionServicePollingPolicyOption>()) {
return options.get<PredictionServicePollingPolicyOption>()->clone();
}
return polling_policy_prototype_->clone();
}

std::unique_ptr<PredictionServiceConnectionIdempotencyPolicy>
idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<PredictionServiceConnectionIdempotencyPolicyOption>()) {
return options.get<PredictionServiceConnectionIdempotencyPolicyOption>()
->clone();
}
return idempotency_policy_->clone();
}

std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<automl_internal::PredictionServiceStub> stub_;
std::unique_ptr<PredictionServiceRetryPolicy const> retry_policy_prototype_;
Expand Down
29 changes: 27 additions & 2 deletions google/cloud/policytroubleshooter/iam_checker_connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ class IamCheckerConnectionImpl : public IamCheckerConnection {
google::cloud::policytroubleshooter::v1::
TroubleshootIamPolicyRequest const& request) override {
return google::cloud::internal::RetryLoop(
retry_policy_prototype_->clone(), backoff_policy_prototype_->clone(),
idempotency_policy_->TroubleshootIamPolicy(request),
retry_policy(), backoff_policy(),
idempotency_policy()->TroubleshootIamPolicy(request),
[this](grpc::ClientContext& context,
google::cloud::policytroubleshooter::v1::
TroubleshootIamPolicyRequest const& request) {
Expand All @@ -76,6 +76,31 @@ class IamCheckerConnectionImpl : public IamCheckerConnection {
}

private:
std::unique_ptr<IamCheckerRetryPolicy> retry_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<IamCheckerRetryPolicyOption>()) {
return options.get<IamCheckerRetryPolicyOption>()->clone();
}
return retry_policy_prototype_->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<IamCheckerBackoffPolicyOption>()) {
return options.get<IamCheckerBackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
}

std::unique_ptr<IamCheckerConnectionIdempotencyPolicy> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<IamCheckerConnectionIdempotencyPolicyOption>()) {
return options.get<IamCheckerConnectionIdempotencyPolicyOption>()
->clone();
}
return idempotency_policy_->clone();
}

std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<policytroubleshooter_internal::IamCheckerStub> stub_;
std::unique_ptr<IamCheckerRetryPolicy const> retry_policy_prototype_;
Expand Down
Loading

0 comments on commit bbc35f8

Please sign in to comment.