Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Oct 19, 2021

Closes #383.
Closes #434.

@tseaver tseaver requested a review from a team as a code owner October 19, 2021 19:15
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 19, 2021
@product-auto-label product-auto-label bot added api: bigtable Issues related to the googleapis/python-bigtable API. samples Issues that are directly related to samples. labels Oct 19, 2021
# Get the instance created
instanceadmin.run_instance_operations(PROJECT, INSTANCE, CLUSTER1)
capsys.readouterr() # throw away output
# This won't work, because the instance isn't created yet
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment took a bit to understand. Are you saying we can't add a cluster because INSTANCE doesn't exist (and you ensured that in the line above)?

"get the instance created" also feels weird. Are we getting the instance or creating it? I had similar feelings about those above. I recognize those were carried forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The instance creation gets moved down to line 138 here -- the instance does not exist at the point that we first call instanceadmin.add_cluster.

The diff display is weird -- I don't get why it breaks up. All this PR does is move the "ensure the instance is set up" code inside a nested function (and therefore indented), wrapped in a backoff.

@tseaver tseaver force-pushed the 383-434-harden-instanceadmin-samples-vs-timeouts branch from 2952d25 to 1acba53 Compare October 20, 2021 18:15
@kolea2 kolea2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 20, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 20, 2021
@tseaver tseaver merged commit a189acb into main Oct 20, 2021
@tseaver tseaver deleted the 383-434-harden-instanceadmin-samples-vs-timeouts branch October 20, 2021 20:16
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 googleapis/python-bigtable API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.

6 participants