Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.

tests: harden instance admin samples against timeouts - #452

Merged
tseaver merged 1 commit into
mainfrom
383-434-harden-instanceadmin-samples-vs-timeouts
Oct 20, 2021
Merged

tests: harden instance admin samples against timeouts#452
tseaver merged 1 commit into
mainfrom
383-434-harden-instanceadmin-samples-vs-timeouts

Conversation

@tseaver

Copy link
Copy Markdown
Contributor

Closes#383.
Closes#434.

@tseaver
tseaver requested review from a team, chrisrossi, crwilcox and kolea2October 19, 2021 19:15
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Oct 19, 2021
@product-auto-labelproduct-auto-labelBot added api: bigtable Issues related to the googleapis/python-bigtable API. samples Issues that are directly related to samples. labels Oct 19, 2021
Comment threadsamples/instanceadmin/test_instanceadmin.py Outdated
# 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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
tseaverforce-pushed the 383-434-harden-instanceadmin-samples-vs-timeouts branch from 2952d25 to 1acba53CompareOctober 20, 2021 18:15
@kolea2kolea2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 20, 2021
@yoshi-kokoroyoshi-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 mainOct 20, 2021
@tseaver
tseaver deleted the 383-434-harden-instanceadmin-samples-vs-timeouts branch October 20, 2021 20:16
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigtableIssues related to the googleapis/python-bigtable API.cla: yesThis human has signed the Contributor License Agreement.samplesIssues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

samples.instanceadmin.test_instanceadmin: test_add_and_delete_cluster failed 'test_delete_instance' sample setup flakes with TimeoutError

5 participants

@tseaver@crwilcox@kolea2@dandhlee@yoshi-kokoro