Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bigtable: deprecate the use of typesafe names #4257

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .kokoro/continuous/bigtable-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 -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
}

env_vars: {
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/nightly/bigtable-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 -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
}

env_vars: {
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/presubmit/bigtable-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 -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
}

env_vars: {
Expand Down
4 changes: 3 additions & 1 deletion TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ To use the `prod` environment:
```shell
mvn verify -am -pl google-cloud-bigtable \
-Dbigtable.env=prod \
-Dbigtable.table=projects/my-project/instances/my-instance/tables/my-table
-Dbigtable.project=my-project
-Dbigtable.instance=my-instance
-Dbigtable.table=my-table
```

### Testing code that uses Bigtable Admin
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.api.gax.rpc.ServerStreamingCallSettings;
import com.google.api.gax.rpc.UnaryCallSettings;
import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation;
import com.google.cloud.bigtable.data.v2.models.InstanceName;
import com.google.cloud.bigtable.data.v2.models.KeyOffset;
import com.google.cloud.bigtable.data.v2.models.Query;
import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow;
Expand Down Expand Up @@ -49,7 +48,8 @@
*
* <pre>{@code
* BigtableDataSettings.Builder settingsBuilder = BigtableDataSettings.newBuilder()
* .setInstanceName(InstanceName.of("my-project", "my-instance-id"))
* .setProjectId("my-project")
* .setInstanceId("my-instance-id")
* .setAppProfileId("default");
*
* settingsBuilder.readRowsSettings().setRetryableCodes(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE);
Expand All @@ -67,12 +67,27 @@ public static Builder newBuilder() {
return new Builder();
}

/** Returns the target instance */
public InstanceName getInstanceName() {
/**
* Returns the target instance.
*
* @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
*/
@Deprecated()
public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
return getTypedStubSettings().getInstanceName();
}

/** Returns the configured AppProfile to use */
/** Returns the target project id. */
public String getProjectId() {
return getTypedStubSettings().getProjectId();
}

/** Returns the target instance id. */
public String getInstanceId() {
return getTypedStubSettings().getInstanceId();
}

/** Returns the configured AppProfile id to use. */
public String getAppProfileId() {
return getTypedStubSettings().getAppProfileId();
}
Expand Down Expand Up @@ -142,17 +157,55 @@ private Builder(BigtableDataSettings settings) {
/**
* Sets the target instance. This setting is required. All RPCs will be made in the context of
* this setting.
*
* @deprecated Please use {@link #setProjectId(String)} and {@link #setInstanceId(String)}.
*/
public Builder setInstanceName(@Nonnull InstanceName instanceName) {
@Deprecated
public Builder setInstanceName(
@Nonnull com.google.cloud.bigtable.data.v2.models.InstanceName instanceName) {
getTypedStubSettings().setInstanceName(instanceName);
return this;
}

/** Gets the {@link InstanceName} that was previously set on this Builder. */
public InstanceName getInstanceName() {
/**
* Gets the {@link com.google.cloud.bigtable.data.v2.models.InstanceName} that was previously
* set on this Builder.
*
* @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
*/
@Deprecated
public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
return getTypedStubSettings().getInstanceName();
}

/**
* Sets the target project. This setting is required. All RPCs will be made in the context of
* this setting.
*/
public Builder setProjectId(@Nonnull String projectId) {
getTypedStubSettings().setProjectId(projectId);
return this;
}

/** Gets the project id that was previously set on this Builder. */
public String getProjectId() {
return getTypedStubSettings().getProjectId();
}

/**
* Sets the target instance. This setting is required. All RPCs will be made in the context of
* this setting.
*/
public Builder setInstanceId(@Nonnull String instanceId) {
getTypedStubSettings().setInstanceId(instanceId);
return this;
}

/** Gets the instance id that was previously set on this Builder. */
public String getInstanceId() {
return getTypedStubSettings().getInstanceId();
}

/**
* Sets the AppProfile to use. An application profile (sometimes also shortened to "app
* profile") is a group of configuration parameters for an individual use case. A client will
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.cloud.bigtable.data.v2.internal;

import com.google.api.core.InternalApi;
import javax.annotation.Nonnull;

/**
* Internal helper to compose full resource names.
*
* <p>This class is considered an internal implementation detail and not meant to be used by
* applications.
*/
@InternalApi
public class NameUtil {
public static String formatInstanceName(@Nonnull String projectId, @Nonnull String instanceId) {
return "projects/" + projectId + "/instances/" + instanceId;
}

public static String formatTableName(
@Nonnull String projectId, @Nonnull String instanceId, @Nonnull String tableId) {
return formatInstanceName(projectId, instanceId) + "/tables/" + tableId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.google.api.core.InternalApi;
import com.google.auto.value.AutoValue;
import com.google.cloud.bigtable.data.v2.models.InstanceName;

/**
* Contains information necessary to construct Bigtable protobuf requests from user facing models.
Expand All @@ -32,12 +31,36 @@
@InternalApi
@AutoValue
public abstract class RequestContext {
public static RequestContext create(InstanceName instanceName, String appProfileId) {
return new AutoValue_RequestContext(instanceName, appProfileId);

/** Creates a new instance of the {@link RequestContext}. */
public static RequestContext create(String projectId, String instanceId, String appProfileId) {
return new AutoValue_RequestContext(projectId, instanceId, appProfileId);
}

/** @deprecated Please use {@link #create(String, String, String)}. */
@Deprecated
public static RequestContext create(
com.google.cloud.bigtable.data.v2.models.InstanceName instanceName, String appProfileId) {
return new AutoValue_RequestContext(
instanceName.getProject(), instanceName.getInstance(), appProfileId);
}

/** The instance that the client is configured to target */
public abstract InstanceName getInstanceName();
/** The project id that the client is configured to target. */
public abstract String getProjectId();

/** The instance id that the client is configured to target. */
public abstract String getInstanceId();

/**
* The instance that the client is configured to target.
*
* @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
*/
@Deprecated
public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
return com.google.cloud.bigtable.data.v2.models.InstanceName.of(
getProjectId(), getInstanceId());
}

/** The App Profile to use when processing the current request */
public abstract String getAppProfileId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import com.google.api.core.InternalApi;
import com.google.bigtable.v2.MutateRowsRequest;
import com.google.bigtable.v2.TableName;
import com.google.cloud.bigtable.data.v2.internal.NameUtil;
import com.google.cloud.bigtable.data.v2.internal.RequestContext;
import com.google.common.base.Preconditions;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -91,14 +91,12 @@ public BulkMutation add(@Nonnull ByteString rowKey, @Nonnull Mutation mutation)

@InternalApi
public MutateRowsRequest toProto(RequestContext requestContext) {
TableName tableName =
TableName.of(
requestContext.getInstanceName().getProject(),
requestContext.getInstanceName().getInstance(),
tableId);
String tableName =
NameUtil.formatTableName(
requestContext.getProjectId(), requestContext.getInstanceId(), tableId);

return builder
.setTableName(tableName.toString())
.setTableName(tableName)
.setAppProfileId(requestContext.getAppProfileId())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import com.google.api.core.InternalApi;
import com.google.bigtable.v2.CheckAndMutateRowRequest;
import com.google.bigtable.v2.TableName;
import com.google.cloud.bigtable.data.v2.internal.NameUtil;
import com.google.cloud.bigtable.data.v2.internal.RequestContext;
import com.google.cloud.bigtable.data.v2.models.Filters.Filter;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -128,11 +128,9 @@ public CheckAndMutateRowRequest toProto(RequestContext requestContext) {
Preconditions.checkState(
!builder.getTrueMutationsList().isEmpty() || !builder.getFalseMutationsList().isEmpty(),
"ConditionalRowMutations must have `then` or `otherwise` mutations.");
TableName tableName =
TableName.of(
requestContext.getInstanceName().getProject(),
requestContext.getInstanceName().getInstance(),
tableId);
String tableName =
NameUtil.formatTableName(
requestContext.getProjectId(), requestContext.getInstanceId(), tableId);
return builder
.setTableName(tableName.toString())
.setAppProfileId(requestContext.getAppProfileId())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
import java.util.Map;

// Copied from com.google.bigtable.admin.v2
// TODO: figure out how to unify admin & data resource names
/** Typesafe representation of a fully qualified Bigtable instance name */

/** @deprecated Please use project id and instance id strings instead. */
@Deprecated
public class InstanceName implements ResourceName {
private static final PathTemplate PATH_TEMPLATE =
PathTemplate.createWithoutUrlEncoding("projects/{project}/instances/{instance}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import com.google.bigtable.v2.ReadRowsRequest;
import com.google.bigtable.v2.RowRange;
import com.google.bigtable.v2.RowSet;
import com.google.bigtable.v2.TableName;
import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator;
import com.google.cloud.bigtable.data.v2.internal.NameUtil;
import com.google.cloud.bigtable.data.v2.internal.RequestContext;
import com.google.cloud.bigtable.data.v2.internal.RowSetUtil;
import com.google.cloud.bigtable.data.v2.models.Range.ByteStringRange;
Expand Down Expand Up @@ -248,14 +248,12 @@ public ByteStringRange getBound() {
*/
@InternalApi
public ReadRowsRequest toProto(RequestContext requestContext) {
TableName tableName =
TableName.of(
requestContext.getInstanceName().getProject(),
requestContext.getInstanceName().getInstance(),
tableId);
String tableName =
NameUtil.formatTableName(
requestContext.getProjectId(), requestContext.getInstanceId(), tableId);

return builder
.setTableName(tableName.toString())
.setTableName(tableName)
.setAppProfileId(requestContext.getAppProfileId())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.google.api.core.InternalApi;
import com.google.bigtable.v2.ReadModifyWriteRowRequest;
import com.google.bigtable.v2.ReadModifyWriteRule;
import com.google.bigtable.v2.TableName;
import com.google.cloud.bigtable.data.v2.internal.NameUtil;
import com.google.cloud.bigtable.data.v2.internal.RequestContext;
import com.google.common.base.Preconditions;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -128,13 +128,12 @@ public ReadModifyWriteRow increment(

@InternalApi
public ReadModifyWriteRowRequest toProto(RequestContext requestContext) {
TableName tableName =
TableName.of(
requestContext.getInstanceName().getProject(),
requestContext.getInstanceName().getInstance(),
tableId);
String tableName =
NameUtil.formatTableName(
requestContext.getProjectId(), requestContext.getInstanceId(), tableId);

return builder
.setTableName(tableName.toString())
.setTableName(tableName)
.setAppProfileId(requestContext.getAppProfileId())
.build();
}
Expand Down
Loading