Skip to content

Bigtable: Adding Integration Test of BigtableInstanceAdminClient#6186

Merged
igorbernstein2 merged 2 commits intogoogleapis:masterfrom
rahulKQL:instanceAdmintest
Aug 30, 2019
Merged

Bigtable: Adding Integration Test of BigtableInstanceAdminClient#6186
igorbernstein2 merged 2 commits intogoogleapis:masterfrom
rahulKQL:instanceAdmintest

Conversation

@rahulKQL
Copy link
Copy Markdown
Contributor

@rahulKQL rahulKQL commented Aug 28, 2019

This PR contains Integration tests for BigtableInstanceAdminClient.

  • As this client is not supported on emulator So it would be ignored.
  • Also, it will not create any bigtable resource(instance, cluster: considering these resources are expensive) unless createBigtableResource property is set along with other TestEnvRule properties.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2019
@igorbernstein2 igorbernstein2 added the api: bigtable Issues related to the Bigtable API. label Aug 28, 2019
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 29, 2019

Codecov Report

Merging #6186 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6186   +/-   ##
=========================================
  Coverage     47.51%   47.51%           
- Complexity    27460    27475   +15     
=========================================
  Files          2526     2526           
  Lines        274681   274681           
  Branches      31402    31397    -5     
=========================================
  Hits         130521   130521           
  Misses       134528   134528           
  Partials       9632     9632

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cede9f...4e4a1ea. Read the comment docs.

.addCluster(newClusterId, "us-central1-a", 1, StorageType.SSD)
.setDisplayName("Fresh-Instance-Name")
.addLabel("state", "readytodelete")
.setType(Instance.Type.PRODUCTION));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would use a development instance. Also, prod instances require at least 3 nodes, while your cluster only has 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, you should move this out of the try

Copy link
Copy Markdown
Contributor Author

@rahulKQL rahulKQL Aug 30, 2019

Choose a reason for hiding this comment

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

My apologies for the number of nodes. I have switched to Development cluster:

 client.createInstance(
        CreateInstanceRequest.of(newInstanceId)
            .addCluster(newClusterId, "us-central1-a", 0, StorageType.SSD)
            .setDisplayName("Fresh-Instance-Name")
            .addLabel("state", "readytodelete")
            .setType(Instance.Type.DEVELOPMENT));

Q: This was confusing to me, whenever I used 1 for the server nodes it threw an IllegalArgumentException stating server nodes cannot be provided.

com.google.api.gax.rpc.InvalidArgumentException: io.grpc.StatusRuntimeException: INVALID_ARGUMENT: Error in field 'clusters' : Serve nodes cannot be specified for development instances.

This worked fine when the server node is 0(It creates an instance with a cluster combining of 0 nodes). Should we change this behavior or should we update the docs?

- Test case for AppProfile, IAMPolicy, Instance, Clusters.
- BigtableInstanceAdminClient in AbstractTestEnv.
- Added cleanups for stale appProfile/instance/cluster.
- Setup a cleanUpInstance() in case of prod or direct_path env type.
@rahulKQL rahulKQL marked this pull request as ready for review August 30, 2019 09:56
Copy link
Copy Markdown
Contributor

@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.

LGTM!
Thanks!

@igorbernstein2 igorbernstein2 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 30, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 30, 2019
@igorbernstein2 igorbernstein2 merged commit 1cd99a8 into googleapis:master Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants