Skip to content

Commit f15fabc

Browse files
dbolducdbolduc
authored
cleanup(bigtable): default options like the generator does (#9636)
1 parent a508bb6 commit f15fabc

5 files changed

Lines changed: 28 additions & 47 deletions

File tree

‎google/cloud/bigtable/data_connection.cc‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -156,21 +156,3 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
156156
} // namespace bigtable
157157
} // namespace cloud
158158
} // namespace google
159-
160-
namespace google {
161-
namespace cloud {
162-
namespace bigtable_internal {
163-
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
164-
165-
std::shared_ptr<bigtable::DataConnection> MakeDataConnection(
166-
std::shared_ptr<BigtableStub> stub, Options options) {
167-
options = bigtable::internal::DefaultDataOptions(std::move(options));
168-
auto background = internal::MakeBackgroundThreadsFactory(options)();
169-
return std::make_shared<DataConnectionImpl>(
170-
std::move(background), std::move(stub), std::move(options));
171-
}
172-
173-
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
174-
} // namespace bigtable_internal
175-
} // namespace cloud
176-
} // namespace google

‎google/cloud/bigtable/data_connection.h‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,4 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
165165
} // namespace cloud
166166
} // namespace google
167167

168-
namespace google {
169-
namespace cloud {
170-
namespace bigtable_internal {
171-
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
172-
173-
std::shared_ptr<bigtable::DataConnection> MakeDataConnection(
174-
std::shared_ptr<BigtableStub> stub, Options options);
175-
176-
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
177-
} // namespace bigtable_internal
178-
} // namespace cloud
179-
} // namespace google
180-
181168
#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_DATA_CONNECTION_H

‎google/cloud/bigtable/internal/data_connection_impl.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,8 @@ DataConnectionImpl::DataConnectionImpl(
5555
std::shared_ptr<BigtableStub> stub, Options options)
5656
: background_(std::move(background)),
5757
stub_(std::move(stub)),
58-
options_(internal::MergeOptions(
59-
std::move(options),
60-
bigtable::internal::DefaultDataOptions(DataConnection::options()))) {}
58+
options_(internal::MergeOptions(std::move(options),
59+
DataConnection::options())) {}
6160

6261
Status DataConnectionImpl::Apply(std::string const& table_name,
6362
bigtable::SingleRowMutation mut) {

‎google/cloud/bigtable/internal/data_connection_impl_test.cc‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/bigtable/internal/data_connection_impl.h"
16+
#include "google/cloud/bigtable/data_connection.h"
17+
#include "google/cloud/bigtable/internal/defaults.h"
1618
#include "google/cloud/bigtable/testing/mock_bigtable_stub.h"
1719
#include "google/cloud/bigtable/testing/mock_policies.h"
1820
#include "google/cloud/common_options.h"
21+
#include "google/cloud/credentials.h"
1922
#include "google/cloud/grpc_options.h"
2023
#include "google/cloud/internal/async_streaming_read_rpc_impl.h"
2124
#include "google/cloud/testing_util/mock_backoff_policy.h"
25+
#include "google/cloud/testing_util/scoped_environment.h"
2226
#include "google/cloud/testing_util/status_matchers.h"
2327
#include <google/protobuf/text_format.h>
2428
#include <gmock/gmock.h>
@@ -184,11 +188,11 @@ ExponentialBackoffPolicy TestBackoffPolicy() {
184188

185189
std::shared_ptr<DataConnectionImpl> TestConnection(
186190
std::shared_ptr<BigtableStub> mock) {
187-
auto options =
191+
auto options = bigtable::internal::DefaultDataOptions(
188192
Options{}
189193
.set<GrpcCredentialOption>(grpc::InsecureChannelCredentials())
190194
.set<DataRetryPolicyOption>(TestRetryPolicy().clone())
191-
.set<DataBackoffPolicyOption>(TestBackoffPolicy().clone());
195+
.set<DataBackoffPolicyOption>(TestBackoffPolicy().clone()));
192196
auto background = internal::MakeBackgroundThreadsFactory(options)();
193197
return std::make_shared<DataConnectionImpl>(
194198
std::move(background), std::move(mock), std::move(options));
@@ -232,14 +236,6 @@ TEST(TransformReadModifyWriteRowResponse, Basic) {
232236
MatchCell(c3), MatchCell(c4)));
233237
}
234238

235-
TEST(DataConnectionTest, Options) {
236-
auto mock = std::make_shared<MockBigtableStub>();
237-
auto conn = TestConnection(std::move(mock));
238-
auto options = conn->options();
239-
EXPECT_TRUE(options.has<GrpcCredentialOption>());
240-
ASSERT_TRUE(options.has<EndpointOption>());
241-
}
242-
243239
/**
244240
* Verify that call options (provided via an OptionsSpan) take precedence over
245241
* connection options (provided to the DataConnection constructor). We will only
@@ -1636,6 +1632,24 @@ TEST(DataConnectionTest, AsyncReadRowFailure) {
16361632
EXPECT_THAT(resp, StatusIs(StatusCode::kPermissionDenied));
16371633
}
16381634

1635+
TEST(MakeDataConnection, DefaultsOptions) {
1636+
using ::google::cloud::testing_util::ScopedEnvironment;
1637+
ScopedEnvironment emulator_host("BIGTABLE_EMULATOR_HOST", "localhost:1");
1638+
1639+
auto conn = bigtable::MakeDataConnection(
1640+
Options{}
1641+
.set<bigtable::AppProfileIdOption>("user-supplied")
1642+
// Disable channel refreshing, which is not under test.
1643+
.set<bigtable::MaxConnectionRefreshOption>(ms(0))
1644+
// Create the minimum number of stubs.
1645+
.set<GrpcNumChannelsOption>(1));
1646+
auto options = conn->options();
1647+
EXPECT_TRUE(options.has<bigtable::DataRetryPolicyOption>())
1648+
<< "Options are not defaulted in MakeDataConnection()";
1649+
EXPECT_EQ(options.get<bigtable::AppProfileIdOption>(), "user-supplied")
1650+
<< "User supplied Options are overridden in MakeDataConnection()";
1651+
}
1652+
16391653
} // namespace
16401654
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
16411655
} // namespace bigtable_internal

‎google/cloud/bigtable/table.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,8 @@ class Table {
208208
: table_(std::move(tr)),
209209
table_name_(table_.FullName()),
210210
connection_(std::move(conn)),
211-
options_(google::cloud::internal::MergeOptions(
212-
std::move(options),
213-
internal::DefaultDataOptions(connection_->options()))),
211+
options_(google::cloud::internal::MergeOptions(std::move(options),
212+
connection_->options())),
214213
metadata_update_policy_(bigtable_internal::MakeMetadataUpdatePolicy(
215214
table_name_, app_profile_id())) {}
216215

0 commit comments

Comments
 (0)