Skip to content
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-8480] Bigtable: 'test_bigtable_create_table' snippet flakes with '504 Deadline Exceeded'. #16

Closed
wants to merge 1 commit into from

Conversation

sangramql
Copy link

@sangramql sangramql commented Jul 30, 2019

Addresses #8480

Preliminary discussion can be found here

@sangramql
Copy link
Author

@mf2199 you have already reviewed, but for further updates plz review.
Opened PR on qlogic repository.

@mf2199 mf2199 requested review from mf2199, emar-kar and IlyaFaer July 31, 2019 01:50
@mf2199 mf2199 changed the title Bigtable: Create table in create_table snippet, deadline exceed error fix [GCP-8480] Bigtable: 'test_bigtable_create_table' snippet flakes with '504 Deadline Exceeded'. Jul 31, 2019
Copy link
Collaborator

@mf2199 mf2199 left a comment

Choose a reason for hiding this comment

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

As we mentioned before, it may be worthwhile including some of the previous discussion into the public PR in order to backup our reasoning.

try:
table.create(column_families={"cf1": max_versions_rule})
break
except Exception as e:

Choose a reason for hiding this comment

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

Maybe that is worth it, if possible, to determine the exception more specifically, instead of rising it through Exception?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree on that, this should be under question. Exception is a base class for all exceptions in Python, so with this statement you'll catch errors of all kinds, while in issue they talk only about google.api_core.exceptions.DeadlineExceeded.

@mf2199
Copy link
Collaborator

mf2199 commented Aug 1, 2019

This can now be forwarded to a public PR.

@mf2199 mf2199 closed this Aug 1, 2019
emar-kar pushed a commit that referenced this pull request Aug 15, 2019
…leapis#8720)

* Checking in staged client helper code

Additional test, docs & proposed cleanup needs to happen on top of this.

* update create_model to allow user to specify included or excluded col… (#16)

* update create_model to allow user to specify included or excluded columns

* made minor changes stylistically and with added ValueError outputs

* Update doc gen & module structure. Add unit & system tests

* added two new func: set time, get table address (#23)

* added two new func: set time, get table address

* changed indentation

* Add system tests

* Address linter & python2.7 import errors

* Passes **kwargs through to client & implements missing methods

* Support BQ as input/output in batch_predict

* Address first round of feedback

* Switch to pytest.raises, fix .rst formatting exception

* Make list system tests more stringent
emar-kar pushed a commit that referenced this pull request Sep 18, 2019
…leapis#8720)

* Checking in staged client helper code

Additional test, docs & proposed cleanup needs to happen on top of this.

* update create_model to allow user to specify included or excluded col… (#16)

* update create_model to allow user to specify included or excluded columns

* made minor changes stylistically and with added ValueError outputs

* Update doc gen & module structure. Add unit & system tests

* added two new func: set time, get table address (#23)

* added two new func: set time, get table address

* changed indentation

* Add system tests

* Address linter & python2.7 import errors

* Passes **kwargs through to client & implements missing methods

* Support BQ as input/output in batch_predict

* Address first round of feedback

* Switch to pytest.raises, fix .rst formatting exception

* Make list system tests more stringent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants