Skip to content

Commit

Permalink
Bigtable admin: deprecate typesafe names part 2 (#4258)
Browse files Browse the repository at this point in the history
* Bigtable admin: deprecate typesafe names

This is a followup on #4257:
Unfortunately typesafe names create more issues for bigtable than they solve. A lot of the time users need to use both bigtable data & admin apis. Unfortunately, those apis define the same names in different namespaces. This forces users to use fully qualified names which is ugly. Also, absolute names don't benefit the bigtable client because the client is anchored at the instance and all names end up being relative

* format code

* fix year
  • Loading branch information
igorbernstein2 committed Jan 4, 2019
1 parent ba4ba16 commit 28720b5
Show file tree
Hide file tree
Showing 26 changed files with 580 additions and 358 deletions.
2 changes: 1 addition & 1 deletion .kokoro/continuous/bigtableadmin-it.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ env_vars: {

env_vars: {
key: "INTEGRATION_TEST_ARGS"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.instance=projects/gcloud-devel/instances/google-cloud-bigtable"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable"
}

env_vars: {
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/nightly/bigtableadmin-it.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ env_vars: {

env_vars: {
key: "INTEGRATION_TEST_ARGS"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.instance=projects/gcloud-devel/instances/google-cloud-bigtable"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable"
}

env_vars: {
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/presubmit/bigtableadmin-it.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ env_vars: {

env_vars: {
key: "INTEGRATION_TEST_ARGS"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.instance=projects/gcloud-devel/instances/google-cloud-bigtable"
value: "google-cloud-clients/google-cloud-bigtable-admin -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable"
}

env_vars: {
Expand Down
3 changes: 2 additions & 1 deletion TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ To run the tests:
created earlier. Example:
```shell
mvn verify -am -pl google-cloud-bigtable-admin \
-Dbigtable.instance=projects/my-project/instances/my-instance
-Dbigtable.project=my-project
-Dbigtable.instance=my-instance
```

### Testing code that uses Compute
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,13 @@
/**
* Settings class to configure an instance of {@link BigtableInstanceAdminClient}.
*
* <p>It must be configured with a {@link ProjectName} and can be used to change default RPC
* settings.
* <p>It must be configured with a project id and can be used to change default RPC settings.
*
* <p>Example usage:
*
* <pre>{@code
* BigtableInstanceAdminSettings.Builder settingsBuilder = BigtableInstanceAdminSettings.newBuilder()
* .setProjectName(ProjectName.of("my-project"));
* .setProjectId("my-project");
*
* settingsBuilder.stubSettings().createInstanceSettings()
* .setRetrySettings(
Expand All @@ -45,21 +44,32 @@
* }</pre>
*/
public final class BigtableInstanceAdminSettings {
private final ProjectName projectName;
private final String projectId;
private final BigtableInstanceAdminStubSettings stubSettings;

private BigtableInstanceAdminSettings(Builder builder) throws IOException {
Preconditions.checkNotNull(builder.projectName, "ProjectName must be set");
Preconditions.checkNotNull(builder.projectId, "Project ud must be set");
Verify.verifyNotNull(builder.stubSettings, "stubSettings should never be null");

this.projectName = builder.projectName;
this.projectId = builder.projectId;
this.stubSettings = builder.stubSettings.build();
}

/** Gets the anme of the project whose instances the client will manager. */
/** Gets the id of the project whose instances the client will manage. */
@Nonnull
public ProjectName getProjectName() {
return projectName;
public String getProjectId() {
return projectId;
}

/**
* Gets the name of the project whose instances the client will manager.
*
* @deprecated Please use {@link #getProjectId()}.
*/
@Deprecated
@Nonnull
public com.google.bigtable.admin.v2.ProjectName getProjectName() {
return ProjectName.of(projectId);
}

/** Gets the underlying RPC settings. */
Expand All @@ -80,29 +90,53 @@ public static Builder newBuilder() {

/** Builder for BigtableInstanceAdminSettings. */
public static final class Builder {
@Nullable private ProjectName projectName;
@Nullable private String projectId;
private final BigtableInstanceAdminStubSettings.Builder stubSettings;

private Builder() {
stubSettings = BigtableInstanceAdminStubSettings.newBuilder();
}

private Builder(BigtableInstanceAdminSettings settings) {
this.projectName = settings.projectName;
this.projectId = settings.projectId;
this.stubSettings = settings.stubSettings.toBuilder();
}

/** Sets the name of instance whose tables the client will manage. */
public Builder setProjectName(@Nonnull ProjectName projectName) {
Preconditions.checkNotNull(projectName);
this.projectName = projectName;
/** Sets the id of the project whose instances the client will manage. */
public Builder setProjectId(@Nonnull String projectId) {
Preconditions.checkNotNull(projectId);
this.projectId = projectId;
return this;
}

/** Gets the name of the project whose instances the client will manage. */
/** Gets the id of the project whose instances the client will manage. */
@Nullable
public String getProjectId() {
return projectId;
}

/**
* Sets the name of instance whose tables the client will manage.
*
* @deprecated Please use {@link #setProjectId(String)}.
*/
@Deprecated
public Builder setProjectName(@Nonnull com.google.bigtable.admin.v2.ProjectName projectName) {
return setProjectId(projectName.getProject());
}

/**
* Gets the name of the project whose instances the client will manage.
*
* @deprecated Please use {@link #getProjectId()}.
*/
@Deprecated
@Nullable
public ProjectName getProjectName() {
return projectName;
if (projectId != null) {
return ProjectName.of(projectId);
}
return null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@
import com.google.bigtable.admin.v2.DeleteTableRequest;
import com.google.bigtable.admin.v2.DropRowRangeRequest;
import com.google.bigtable.admin.v2.GetTableRequest;
import com.google.bigtable.admin.v2.InstanceName;
import com.google.bigtable.admin.v2.ListTablesRequest;
import com.google.bigtable.admin.v2.TableName;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPage;
import com.google.cloud.bigtable.admin.v2.BaseBigtableTableAdminClient.ListTablesPagedResponse;
import com.google.cloud.bigtable.admin.v2.internal.NameUtil;
import com.google.cloud.bigtable.admin.v2.models.CreateTableRequest;
import com.google.cloud.bigtable.admin.v2.models.ModifyColumnFamiliesRequest;
import com.google.cloud.bigtable.admin.v2.models.Table;
Expand All @@ -52,7 +51,7 @@
* <p>Sample code to get started:
*
* <pre>{@code
* try(BigtableTableAdminClient client = BigtableTableAdminClient.create(InstanceName.of("[PROJECT]", "[INSTANCE]"))) {
* try(BigtableTableAdminClient client = BigtableTableAdminClient.create("[PROJECT]", "[INSTANCE]")) {
* CreateTable request =
* CreateTableRequest.of("my-table")
* .addFamily("cf1")
Expand All @@ -73,7 +72,8 @@
*
* <pre>{@code
* BigtableTableAdminSettings tableAdminSettings = BigtableTableAdminSettings.newBuilder()
* .setInstanceName(InstanceName.of("[PROJECT]", "[INSTANCE]"))
* .setProjectId("[PROJECT]")
* .setInstanceId("[INSTANCE]")
* .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
* .build();
*
Expand All @@ -85,47 +85,86 @@
*
* <pre>{@code
* BigtableTableAdminSettings tableAdminSettings = BigtableTableAdminSettings.newBuilder()
* .setInstanceName(InstanceName.of("[PROJECT]", "[INSTANCE]"))
* .setProjectId("[PROJECT]")
* .setInstanceId("[INSTANCE]")
* .setEndpoint(myEndpoint).build();
*
* BigtableTableAdminClient client = BigtableTableAdminClient.create(tableAdminSettings);
* }</pre>
*/
public final class BigtableTableAdminClient implements AutoCloseable {

private final EnhancedBigtableTableAdminStub stub;
private final InstanceName instanceName;
private final String projectId;
private final String instanceId;

/** Constructs an instance of BigtableTableAdminClient with the given instanceName. */
public static BigtableTableAdminClient create(@Nonnull InstanceName instanceName)
throws IOException {
return create(BigtableTableAdminSettings.newBuilder().setInstanceName(instanceName).build());
/** Constructs an instance of BigtableTableAdminClient with the given project and instance ids. */
public static BigtableTableAdminClient create(
@Nonnull String projectId, @Nonnull String instanceId) throws IOException {
return create(
BigtableTableAdminSettings.newBuilder()
.setProjectId(projectId)
.setInstanceId(instanceId)
.build());
}

/**
* Constructs an instance of BigtableTableAdminClient with the given instanceName.
*
* @deprecated Please {@link #create(String, String)}.
*/
@Deprecated
public static BigtableTableAdminClient create(
@Nonnull com.google.bigtable.admin.v2.InstanceName instanceName) throws IOException {
return create(instanceName.getProject(), instanceName.getInstance());
}

/** Constructs an instance of BigtableTableAdminClient with the given settings. */
public static BigtableTableAdminClient create(@Nonnull BigtableTableAdminSettings settings)
throws IOException {
EnhancedBigtableTableAdminStub stub =
EnhancedBigtableTableAdminStub.createEnhanced(settings.getStubSettings());
return create(settings.getInstanceName(), stub);
return create(settings.getProjectId(), settings.getInstanceId(), stub);
}

/** Constructs an instance of BigtableTableAdminClient with the given instanceName and stub. */
public static BigtableTableAdminClient create(
@Nonnull InstanceName instanceName, @Nonnull EnhancedBigtableTableAdminStub stub) {
return new BigtableTableAdminClient(instanceName, stub);
@Nonnull String projectId,
@Nonnull String instanceId,
@Nonnull EnhancedBigtableTableAdminStub stub) {
return new BigtableTableAdminClient(projectId, instanceId, stub);
}

private BigtableTableAdminClient(
@Nonnull InstanceName instanceName, @Nonnull EnhancedBigtableTableAdminStub stub) {
Preconditions.checkNotNull(instanceName);
@Nonnull String projectId,
@Nonnull String instanceId,
@Nonnull EnhancedBigtableTableAdminStub stub) {
Preconditions.checkNotNull(projectId);
Preconditions.checkNotNull(instanceId);
Preconditions.checkNotNull(stub);
this.instanceName = instanceName;
this.projectId = projectId;
this.instanceId = instanceId;
this.stub = stub;
}

/** Gets the instanceName this client is associated with. */
public InstanceName getInstanceName() {
return instanceName;
/** Gets the project id of the instance whose tables this client manages. */
public String getProjectId() {
return projectId;
}

/** Gets the id of the instance whose tables this client manages. */
public String getInstanceId() {
return instanceId;
}

/**
* Gets the instanceName this client is associated with.
*
* @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
*/
@Deprecated
public com.google.bigtable.admin.v2.InstanceName getInstanceName() {
return com.google.bigtable.admin.v2.InstanceName.of(projectId, instanceId);
}

@Override
Expand Down Expand Up @@ -183,7 +222,7 @@ public Table createTable(CreateTableRequest request) {
@SuppressWarnings("WeakerAccess")
public ApiFuture<Table> createTableAsync(CreateTableRequest request) {
return transformToTableResponse(
this.stub.createTableCallable().futureCall(request.toProto(instanceName)));
this.stub.createTableCallable().futureCall(request.toProto(projectId, instanceId)));
}

/**
Expand Down Expand Up @@ -275,7 +314,9 @@ public Table modifyFamilies(ModifyColumnFamiliesRequest request) {
@SuppressWarnings("WeakerAccess")
public ApiFuture<Table> modifyFamiliesAsync(ModifyColumnFamiliesRequest request) {
return transformToTableResponse(
this.stub.modifyColumnFamiliesCallable().futureCall(request.toProto(instanceName)));
this.stub
.modifyColumnFamiliesCallable()
.futureCall(request.toProto(projectId, instanceId)));
}

/**
Expand Down Expand Up @@ -500,7 +541,9 @@ public List<String> listTables() {
@SuppressWarnings("WeakerAccess")
public ApiFuture<List<String>> listTablesAsync() {
ListTablesRequest request =
ListTablesRequest.newBuilder().setParent(instanceName.toString()).build();
ListTablesRequest.newBuilder()
.setParent(NameUtil.formatInstanceName(projectId, instanceId))
.build();

// TODO(igorbernstein2): try to upstream pagination spooling or figure out a way to expose the
// paginated responses while maintaining the wrapper facade.
Expand Down Expand Up @@ -550,7 +593,7 @@ public ApiFuture<List<com.google.bigtable.admin.v2.Table>> apply(
public List<String> apply(List<com.google.bigtable.admin.v2.Table> protos) {
List<String> results = Lists.newArrayListWithCapacity(protos.size());
for (com.google.bigtable.admin.v2.Table proto : protos) {
results.add(TableName.parse(proto.getName()).getTable());
results.add(NameUtil.extractTableIdFromTableName(proto.getName()));
}
return results;
}
Expand Down Expand Up @@ -718,8 +761,10 @@ public ApiFuture<Void> dropAllRowsAsync(String tableId) {
*/
@SuppressWarnings("WeakerAccess")
public void awaitReplication(String tableId) {
TableName tableName =
TableName.of(instanceName.getProject(), instanceName.getInstance(), tableId);
// TODO(igorbernstein2): remove usage of typesafe names
com.google.bigtable.admin.v2.TableName tableName =
com.google.bigtable.admin.v2.TableName.of(projectId, instanceId, tableId);

ApiExceptions.callAndTranslateApiException(
stub.awaitReplicationCallable().futureCall(tableName));
}
Expand Down Expand Up @@ -752,8 +797,9 @@ public void awaitReplication(String tableId) {
*/
@SuppressWarnings("WeakerAccess")
public ApiFuture<Void> awaitReplicationAsync(final String tableId) {
TableName tableName =
TableName.of(instanceName.getProject(), instanceName.getInstance(), tableId);
// TODO(igorbernstein2): remove usage of trypesafe names
com.google.bigtable.admin.v2.TableName tableName =
com.google.bigtable.admin.v2.TableName.of(projectId, instanceId, tableId);
return stub.awaitReplicationCallable().futureCall(tableName);
}

Expand All @@ -762,7 +808,7 @@ public ApiFuture<Void> awaitReplicationAsync(final String tableId) {
* projects/{project}/instances/{instance}/tables/{tableId}
*/
private String getTableName(String tableId) {
return TableName.of(instanceName.getProject(), instanceName.getInstance(), tableId).toString();
return NameUtil.formatTableName(projectId, instanceId, tableId);
}

// TODO(igorbernstein): rename methods to make clear that they deal with futures.
Expand Down
Loading

0 comments on commit 28720b5

Please sign in to comment.