Skip to content

connecting to Bigtable emulator using environment variable #6067

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.

Sign up for GitHub

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

Merged
igorbernstein2 merged 4 commits into from
Aug 19, 2019
Merged

connecting to Bigtable emulator using environment variable #6067

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35c4d5b
connecting to Bigtable emulator using environment variable
rahulKQL Aug 12, 2019
db553d3
Address feedback comment for emulator configs
rahulKQL Aug 19, 2019
4ddac32
Address feedback commets
rahulKQL Aug 19, 2019
5b9688b
Addressing more comments
rahulKQL Aug 19, 2019
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
10 changes: 6 additions & 4 deletions ...bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
import com.google.cloud.bigtable.admin.v2.stub.BigtableTableAdminStubSettings;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.base.Verify;
import io.grpc.ManagedChannelBuilder;
import java.io.IOException;
Expand Down Expand Up @@ -97,14 +98,14 @@ public Builder toBuilder() {
*/
public static Builder newBuilder() {
String hostAndPort = System.getenv(BIGTABLE_EMULATOR_HOST_ENV_VAR);
if (hostAndPort != null && !hostAndPort.isEmpty()) {
if (!Strings.isNullOrEmpty(hostAndPort)) {
int port;
try {
port = Integer.parseInt(hostAndPort.substring(hostAndPort.lastIndexOf(":") + 1));
return newBuilderForEmulator(hostAndPort.substring(0, hostAndPort.lastIndexOf(":")), port);
} catch (NumberFormatException ex) {
} catch (NumberFormatException | IndexOutOfBoundsException ex) {
throw new RuntimeException(
"Invalid port in "
"Invalid host/port in "
+ BIGTABLE_EMULATOR_HOST_ENV_VAR
+ " environment variable: "
+ hostAndPort);
Expand All @@ -119,7 +120,8 @@ public static Builder newBuilderForEmulator(int port) {
}

/**
* Create a new builder preconfigured to connect to the Bigtable emulator with host & port number.
* Creates a new builder preconfigured to connect to the Bigtable emulator with host name and port
* number.
*/
public static Builder newBuilderForEmulator(String hostname, int port) {
Builder builder = new Builder().setProjectId("fake-project").setInstanceId("fake-instance");
Copy link

@igorbernstein2 igorbernstein2 Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be setting the project instance ids to stay consistent with other languages

Copy link
Author

@rahulKQL rahulKQL Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Java, we have projectId & instanceId required. Shall I remove the Preconditions checks for projectId & InstanceId?
I would vote to keep these checks and continue with "fake-project" & "fake-instance".

Copy link

@igorbernstein2 igorbernstein2 Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by that. I'm picturing something along the lines of:

BigtableTableAdminSettings.newBuilderForEmulator("localhost", 1234).setProjectId("blah").setInstanceId("blah2").build()

I think the other client languages require something similar

Copy link
Author

@rahulKQL rahulKQL Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, I mistook it as emulator without any projectId & instanceId. Have updated with the suggestion. please have a look.

Expand Down
14 changes: 7 additions & 7 deletions ...-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.cloud.bigtable.data.v2.models.Query;
import com.google.cloud.bigtable.data.v2.models.Row;
import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings;
import com.google.common.base.Strings;
import io.grpc.ManagedChannelBuilder;
import java.util.logging.Logger;
import javax.annotation.Nonnull;
Expand Down Expand Up @@ -74,14 +75,14 @@ private BigtableDataSettings(Builder builder) {
*/
public static Builder newBuilder() {
String hostAndPort = System.getenv(BIGTABLE_EMULATOR_HOST_ENV_VAR);
if (hostAndPort != null && !hostAndPort.isEmpty()) {
if (!Strings.isNullOrEmpty(hostAndPort)) {
try {
int lastIndexOfCol = hostAndPort.lastIndexOf(":");
int port = Integer.parseInt(hostAndPort.substring(lastIndexOfCol + 1));
return newBuilderForEmulator(hostAndPort.substring(0, lastIndexOfCol), port);
} catch (NumberFormatException ex) {
} catch (NumberFormatException | IndexOutOfBoundsException ex) {
throw new RuntimeException(
"Invalid port in "
"Invalid host/port in "
+ BIGTABLE_EMULATOR_HOST_ENV_VAR
+ " environment variable: "
+ hostAndPort);
Expand All @@ -96,15 +97,14 @@ public static Builder newBuilderForEmulator(int port) {
}

/**
* Create a new builder preconfigured to connect to the Bigtable emulator with o host & port name.
* Creates a new builder preconfigured to connect to the Bigtable emulator with a host name and
* port number.
*/
public static Builder newBuilderForEmulator(String hostname, int port) {
Builder builder = new Builder();
Builder builder = new Builder().setProjectId("fake-project").setInstanceId("fake-instance");

builder
.stubSettings()
.setProjectId("fake-project")
.setInstanceId("fake-instance")
.setCredentialsProvider(NoCredentialsProvider.create())
.setEndpoint(hostname + ":" + port)
.setTransportChannelProvider(
Expand Down
8 changes: 6 additions & 2 deletions ...le-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/env/TestEnvRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
*/
package com.google.cloud.bigtable.data.v2.it.env;

import com.google.common.truth.TruthJUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.junit.Assume;
import org.junit.rules.ExternalResource;

/**
Expand Down Expand Up @@ -47,7 +47,11 @@ public class TestEnvRule extends ExternalResource {

@Override
protected void before() throws Throwable {
Assume.assumeTrue(System.getenv(BIGTABLE_EMULATOR_HOST_ENV_VAR) == null);
TruthJUnit.assume()
.withMessage(
"Integration tests can't run with the BIGTABLE_EMULATOR_HOST_ENV_VAR environment variable set. Please use the emulator-it maven profile instead")
.that(System.getenv())
.doesNotContainKey(BIGTABLE_EMULATOR_HOST_ENV_VAR);
String env = System.getProperty(ENV_PROPERTY, "emulator");

switch (env) {
Expand Down