-
Notifications
You must be signed in to change notification settings - Fork 180
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
Adding changes for listTableSnapshots() in BigtableAsyncAdmin #1945
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good. The whitespace needs to be fixed.
@@ -126,9 +126,7 @@ public void testListSnapshots() throws IOException { | |||
return; | |||
} | |||
final Pattern allSnapshots = Pattern.compile(snapshotName + ".*"); | |||
try (Admin admin = getConnection().getAdmin()) { | |||
createTable(tableName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the space at the beginning of all of the lines that were within the try
block.
@@ -151,7 +149,6 @@ public void testTableSnapshots() throws IOException { | |||
return; | |||
} | |||
final Pattern matchAll = Pattern.compile(".*"); | |||
try (Admin admin = getConnection().getAdmin()) { | |||
createTable(tableName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the whitespace now that the try was removed.
@@ -185,9 +182,22 @@ public void testTableSnapshots() throws IOException { | |||
listTableSnapshotsSize(anotherTableName.getNameAsString(), snapshotName)); | |||
Assert.assertEquals(0, | |||
listTableSnapshotsSize(anotherTableName.getNameAsString(), anotherSnapshotName)); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please get rid of this extra line.
} | ||
|
||
@Test | ||
public void testListTableSnapshotWithSingleArgs() throws Exception { | ||
if (sharedTestEnv.isBigtable() && !enableTestForBigtable()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please only use spaces, and fix the whitespace on this method.
|
||
@Override | ||
protected int listTableSnapshotsSize(Pattern tableNamePattern) throws Exception { | ||
try(Admin admin = getConnection().getAdmin()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the whitespace.
|
||
@Override | ||
protected int listTableSnapshotsSize(Pattern tableNamePattern) throws Exception { | ||
try(Admin admin = getConnection().getAdmin()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the whitespace.
|
||
@Override | ||
protected int listTableSnapshotsSize(Pattern tableNamePattern) throws Exception { | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the whitespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Solomon, Thanks for the review, Have removed white-spaces from all the files.
protected int listTableSnapshotsSize(Pattern tableNamePattern) throws Exception { | ||
try { | ||
return getAsyncAdmin().listTableSnapshots(tableNamePattern).get(60, TimeUnit.SECONDS).size(); | ||
}catch (InterruptedException | ExecutionException e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There needs to be a space between }
and catch
.
Source-Link: googleapis/synthtool@571a091 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:81c3ec554428c8ff6c92f0d58668b7ef52265d053a82284c97a326745e786949
Added Implementation for listTableSnapshots() also adding test case in AbstractTestSnapshot for IntegrationTest.