test(samples): add backoff to cluster creation sample#362
Conversation
| print("\nCluster not created, as {} already exists.".format(cluster_id)) | ||
| else: | ||
| operation = cluster.create() | ||
| # Avoid failing for "instance is currently being changed" by |
There was a problem hiding this comment.
Does this belong here or wrapping the add_cluster call in test_instanceadmin.py? https://github.com/googleapis/python-bigtable/blob/master/samples/instanceadmin/test_instanceadmin.py#L118-L122. Might be a better fit there, WDYT?
There was a problem hiding this comment.
Wrapping inside the testrunner would certainly leave the sample itself more pristine: the issue here is that the back-end is raising a 503 UNAVAILABLE for the cluster.create() call, which is (presumably) occurring "too soon" after the instance.create call. Re-running the whole sample doesn't really address the root cause of the flakiness. It might happen to delay running the sample until the back-end load dropped (or whatever is causing the current error to be raised on the back-end).
There was a problem hiding this comment.
instance.create occurs outside of this sample. This seems like a synchronization problem of how we run our tests, and therefore are a bit confusing to put inside the sample itself. To me, it makes more sense to handle issues with our testing in the test class instead.
9370f85 to
b0267cd
Compare
Closes #353.
@kolea2 Note that this PR injects "repeatability" noise into the sample, which may be undesirable from the perspective of documentation.