-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GCP-8479] Bigtable: 'test_bigtable_list_tables' snippet flakes with '504 Deadline Exceeded'. #15
Conversation
@mf2199 you have already reviewed, but for further updates plz review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks fine but the questions are whether the flakes were reproduced, how positive are we about this way of solving them, and if enough testing has been done afterwards to ensure they're gone.
instance = client.instance(INSTANCE_ID) | ||
table = instance.table("to_list") | ||
max_versions_rule = column_family.MaxVersionsGCRule(2) | ||
table.create(column_families={"cf1": max_versions_rule}) | ||
|
||
# [START bigtable_list_tables] | ||
from google.cloud.bigtable import Client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, not sure why this duplicate import was there before, in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, not sure why this duplicate import was there before, in the first place.
These are snippets which are included in doc string of actual functions, so for example codes we need to have imports for users to understand.
cluster = instance.cluster( | ||
"clus-my-123", | ||
"clus-to-delete" + UNIQUE_SUFFIX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nit-picking, but I'd use somewhat more informative names, such as "snippet-instance", "cluster-to-delete" etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This instance is created for only 'instance delete' test, hence the names.
Already tried naming that, but got string length error.
from_value = <_Rendezvous of RPC that terminated with: status = StatusCode.INVALID_ARGUMENT details = "Error in field 'clusters' ...lection clusters : Length should be between [6,30], but found 31 'cluster-to-delete-1564545637619'","grpc_status":3}" ???
E google.api_core.exceptions.InvalidArgument: 400 Error in field 'clusters' : Cluster Id : Invalid id for collection clusters : Length should be between [6,30], but found 31 'cluster-to-delete-1564545637619'
This issue is not consistent in production i suppose. And I was not able to reproduce even once.
|
Closing this PR as this opened in googleapis#8879 |
Addresses #8479
Preliminary discussion can be found here