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.

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

Jump to bottom

Exists method added #3769

Merged
merged 12 commits into from
Oct 23, 2018
Merged

Exists method added #3769

merged 12 commits into from
Oct 23, 2018

Conversation

elisheva-qlogic
Copy link
Contributor

@elisheva-qlogic elisheva-qlogic commented Oct 4, 2018 •
edited
Loading

exists method added in BigtableTableAdminClient to check if table exists #3636

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 4, 2018
@chingor13 chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 4, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 4, 2018
igorbernstein2
igorbernstein2 requested changes Oct 4, 2018
View reviewed changes
...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
* }</pre>
*/
public boolean exists(String tableId) {
List<TableName> tableNames = this.listTables();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
String table = tableName.toString();
String separator = "/";
int index = table.lastIndexOf(separator);
String name = table.substring(index + separator.length());

This comment was marked as spam.

Sign in to view

...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Outdated
boolean found = adminClient.exists(TABLE_NAME.getTable());

// Verify
Assert.assertTrue(found);

This comment was marked as spam.

Sign in to view

...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Show resolved Hide resolved
@elisheva-qlogic elisheva-qlogic requested a review from a team as a code owner October 11, 2018 16:26
igorbernstein2
igorbernstein2 requested changes Oct 11, 2018
View reviewed changes
...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
return ApiFutures.transform(listTablesAsync(), new ApiFunction<List<TableName>, Boolean>() {
@Override public Boolean apply(List<TableName> tableNames) {
for(TableName tableName : tableNames) {
if(tableName.getTable().equalsIgnoreCase(tableId)) {

This comment was marked as spam.

Sign in to view

...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
return listTablesAsync(com.google.bigtable.admin.v2.Table.View.NAME_ONLY);
}

public ApiFuture<List<TableName>> listTablesAsync(com.google.bigtable.admin.v2.Table.View view) {

This comment was marked as spam.

Sign in to view

...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
}

public ApiFuture<Boolean> existsAsync(final String tableId) {
return ApiFutures.transform(listTablesAsync(), new ApiFunction<List<TableName>, Boolean>() {

This comment was marked as spam.

Sign in to view

@sduskis sduskis added the api: bigtable Issues related to the Bigtable API. label Oct 12, 2018
igorbernstein2
igorbernstein2 requested changes Oct 17, 2018
View reviewed changes
Copy link

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

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

This looks pretty good, just needs some stylistic fixes

...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Show resolved Hide resolved
...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Outdated
.setName(TABLE_NAME.toString())
.build();

Mockito.when(mockGetTableCallable.futureCall(Matchers.any(GetTableRequest.class))).thenReturn(ApiFutures.immediateFuture(expectedResponse));

This comment was marked as spam.

Sign in to view

sduskis
sduskis suggested changes Oct 18, 2018
View reviewed changes
...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Outdated
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.*;

This comment was marked as spam.

Sign in to view

...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Outdated
@@ -184,6 +182,7 @@ public void testGetTable() {
// Setup
GetTableRequest expectedRequest = GetTableRequest.newBuilder()
.setName(TABLE_NAME.toString())
.setView(com.google.bigtable.admin.v2.Table.View.SCHEMA_VIEW)

This comment was marked as spam.

Sign in to view

...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Show resolved Hide resolved
igorbernstein2
igorbernstein2 reviewed Oct 19, 2018
View reviewed changes
Copy link

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

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

One last nit about sample code, but this LGTM

...igtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
* found,
* new ApiFutureCallback<Boolean>() {
* public void onSuccess(Boolean found) {
* System.out.println("Table exists");

This comment was marked as spam.

Sign in to view

@igorbernstein2 igorbernstein2 mentioned this pull request Oct 19, 2018
Merged
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
igorbernstein2
igorbernstein2 requested changes Oct 23, 2018
View reviewed changes
...ble-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClientTest.java Outdated
new NotFoundException("fake error", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false);

Mockito.when(mockGetTableCallable.futureCall(Matchers.any(GetTableRequest.class)))
.thenReturn(ApiFutures.immediateFailedFuture(exception));

This comment was marked as spam.

Sign in to view

@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@sduskis
Copy link
Contributor

sduskis commented Oct 23, 2018

@elisheva-qlogic, there's a compilation failure:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.5.1:compile (default-compile) on project google-cloud-bigtable-admin: Compilation failure
[ERROR] /tmpfs/src/github/google-cloud-java/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java:[339,12] cannot find symbol
[ERROR] symbol: method awaitFuture(com.google.api.core.ApiFuture<java.lang.Boolean>)
[ERROR] location: class com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient

It looks like a merge conflict with PR #3809

@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@igorbernstein2
Copy link

I think I fixed the issue

@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@igorbernstein2 igorbernstein2 merged commit 0e1bc68 into googleapis:master Oct 23, 2018
@JesseLovelace JesseLovelace mentioned this pull request Oct 25, 2018
Merged
@elisheva-qlogic elisheva-qlogic deleted the exist_method branch November 8, 2018 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sduskis sduskis sduskis approved these changes

@igorbernstein2 igorbernstein2 igorbernstein2 approved these changes

@pongad pongad Awaiting requested review from pongad

Assignees
No one assigned
Labels
api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@elisheva-qlogic @sduskis @igorbernstein2 @chingor13 @googlebot @kokoro-team @JustinBeckwith