Skip to content

Conversation

@rahulKQL
Copy link

@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

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));

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

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

@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