RPC retries (resubmission) #3811

calpeyser · 2017-08-14T19:01:35Z

Re-submission for PR #3324.

calpeyser · 2017-08-14T20:32:48Z

@dhermes please take a look. Is there anything I should do outside of regular CI to run the tests that failed the last time around?

dhermes · 2017-08-14T22:06:06Z

@calpeyser Sending a PR from your remote will skip the system tests, which is partly why I kept the change around in a branch on the GoogleCloudPlatform remote.

So before we merge this, we should push this branch to the GoogleCloudPlatform remote so the system tests can be run. (FWIW, I have yet to review, though someone else can sign off, I'm not the only one with authority here.)

bigtable/google/cloud/bigtable/table.py

        """
-        request_pb = _create_row_request(
-            self.name, start_key=start_key, end_key=end_key, filter_=filter_,
-            limit=limit, end_inclusive=end_inclusive)


calpeyser · 2017-08-15T17:21:15Z

@dhermes PTAL. Would it be easier to PR against an empty branch, so as not to deal with the backlog of commits? I don't seem to have the privileges to rebase this branch from master.

Looking at the test logs, it seems that the system tests are still being skipped.

googlebot · 2017-08-15T17:22:20Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-08-15T17:23:14Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

dhermes · 2017-08-15T17:42:08Z

@calpeyser I changed the base back to master. I'll just manually push your branch to the other remote so CI can run in parallel with this.

dhermes · 2017-08-15T17:43:59Z

@calpeyser Here is the build and the branch:

https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/2901
https://github.com/GoogleCloudPlatform/google-cloud-python/tree/pr-3811

mbrukman · 2017-08-15T21:21:25Z

@calpeyser – looks like the build/test failed on CircleCI; could you take a look, please?

googlebot · 2017-08-16T22:34:49Z

CLAs look good, thanks!

calpeyser · 2017-08-16T23:13:46Z

@dhermes can you patch the change "encode -> decode for bytestring in bigtable system test" into the branch on the remote? The system tests don't seem to run here.

The current failure is in "Update the docs", and seems unrelated to this change.

calpeyser · 2017-08-21T15:34:10Z

@dhermes ping on pushing the lastest commit to the remote branch so we can run the system tests.

dhermes · 2017-08-21T15:43:49Z

Sorry, doing it now!

dhermes · 2017-08-21T15:44:06Z

(Email is a crappy task list, my bad.)

dhermes · 2017-08-21T15:47:31Z

New build: https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/2951
Non-system-test red build: https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/2917

@calpeyser I see the docs breakage in the last build, you should probably rebase against master due to some moves / nox config changes?

dhermes · 2017-08-21T16:54:40Z

@calpeyser The system tests failed in the build. Do they pass when you run them locally? If you haven't run them locally, is there anything I can do to help you get them running?

$ pip install --upgrade nox-automation
$ cd /path/to/your/git-checkout/of/google-cloud-python/
$ cd bigtable/
$ nox -s "system_tests(python_version='2.7')"
$ nox -s "system_tests(python_version='3.6')"

calpeyser · 2017-10-22T21:05:10Z

@garye what do you think?

garye · 2017-10-31T16:00:34Z

LGTM! Just need @dhermes, @lukesneeringer or someone from the team to pull the trigger I think.

googlebot · 2017-11-10T16:51:18Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-11-14T14:37:08Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

calpeyser · 2017-11-14T15:05:19Z

@tseaver - I've fixed the issues with the merge above - can this PR be merged?

googlebot · 2017-11-14T15:05:26Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

dhermes · 2017-11-15T22:02:02Z

https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/4249

I think this may need to be reverted

dhermes · 2017-11-16T20:50:16Z

@calpeyser @garye Here is another (albeit less severe) failure:

https://circleci.com/gh/GoogleCloudPlatform/google-cloud-python/4256

There have been no other builds on master since the previous failure.

It's worth noting that we have other packages with tests that are flaky (which we tolerate mostly due to lack of developer resources), but this one seems to be a preventable flakiness.

dhermes · 2017-11-16T23:12:45Z

I'm just going to collect a list of broken builds here (for debugging purposes):

I may edit it as more occur / if more occur.

dhermes · 2017-11-21T22:16:35Z

@calpeyser @garye Please weigh in here. I am leaning towards reverting this PR (we need our builds to not fail).

igorbernstein2 · 2017-11-21T22:28:55Z

I can try to fix this tomorrow. After a quick look, I think that the issue is that the test_retry test never cleans up the BIGTABLE_EMULATOR_HOST env var, causing the rest of the tests to connect to a dead port

dhermes · 2017-11-21T22:33:44Z

Thanks @igorbernstein2. I'm happy to help (e.g. if you're unfamiliar with unittest.mock.patch).

calpeyser · 2017-11-22T20:40:18Z

This breakage certainly looks related to this change. @igorbernstein2, thanks a ton - I think you're right about the env var.

This reverts commit 5a0e549 / PR googleapis#3811.

This reverts commit 5a0e549 / PR #3811.

This reverts commit 1c699f36584d8c507118dae9ab8a2a31bc951ce6 / PR #3811.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 14, 2017

calpeyser changed the title ~~Fix retry~~ RPC retries (resubmission) Aug 14, 2017

dhermes reviewed Aug 14, 2017

View reviewed changes

bigtable/google/cloud/bigtable/table.py

"""

request_pb = _create_row_request(

self.name, start_key=start_key, end_key=end_key, filter_=filter_,

limit=limit, end_inclusive=end_inclusive)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

calpeyser changed the base branch from master to pr-3324-to-fix August 15, 2017 16:38

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Aug 15, 2017

dhermes changed the base branch from pr-3324-to-fix to master August 15, 2017 17:41

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Aug 16, 2017

calpeyser requested review from duggelz, liyanhui1228, lukesneeringer, theacodes, tseaver and tswast as code owners September 7, 2017 02:25

fixes for middle-row failures

3d2daeb

Merge branch 'master' into fix-retry

f33f1aa

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 10, 2017

tseaver added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Nov 10, 2017

remove redundant _create_row_request implementation from bigtable

f73b801

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 14, 2017

tseaver merged commit 5a0e549 into googleapis:master Nov 15, 2017

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Dec 4, 2017

Revert "Add RPC retries to Bigtable (googleapis#3811)"

e521fd4

This reverts commit 5a0e549 / PR googleapis#3811.

This was referenced Dec 4, 2017

Revert "Add RPC retries to Bigtable (#3811)" #4524

Merged

BigTable: try resetting BIGTABLE_EMULATOR_HOST to fix bigtable test errors #4443

Closed

dhermes added a commit that referenced this pull request Dec 4, 2017

Revert "Add RPC retries to Bigtable (#3811)"

bd023fe

This reverts commit 5a0e549 / PR #3811.

dhermes added a commit that referenced this pull request Dec 4, 2017

Revert "Add RPC retries to Bigtable (#3811)" (#4524)

dfe8a41

This reverts commit 5a0e549 / PR #3811.

parthea pushed a commit that referenced this pull request Nov 22, 2025

Add RPC retries to Bigtable (#3811)

8a8c305

parthea pushed a commit that referenced this pull request Nov 22, 2025

Revert "Add RPC retries to Bigtable (#3811)" (#4524)

f425409

This reverts commit 1c699f36584d8c507118dae9ab8a2a31bc951ce6 / PR #3811.

RPC retries (resubmission) #3811

RPC retries (resubmission) #3811

Uh oh!

Conversation

calpeyser commented Aug 14, 2017

Uh oh!

calpeyser commented Aug 14, 2017

Uh oh!

dhermes commented Aug 14, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

calpeyser commented Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Aug 15, 2017

Uh oh!

googlebot commented Aug 15, 2017

Uh oh!

dhermes commented Aug 15, 2017

Uh oh!

dhermes commented Aug 15, 2017

Uh oh!

mbrukman commented Aug 15, 2017

Uh oh!

googlebot commented Aug 16, 2017

Uh oh!

calpeyser commented Aug 16, 2017

Uh oh!

calpeyser commented Aug 21, 2017

Uh oh!

dhermes commented Aug 21, 2017

Uh oh!

dhermes commented Aug 21, 2017

Uh oh!

dhermes commented Aug 21, 2017

Uh oh!

dhermes commented Aug 21, 2017

Uh oh!

calpeyser commented Oct 22, 2017

Uh oh!

garye commented Oct 31, 2017

Uh oh!

googlebot commented Nov 10, 2017

Uh oh!

googlebot commented Nov 14, 2017

Uh oh!

calpeyser commented Nov 14, 2017

Uh oh!

googlebot commented Nov 14, 2017

Uh oh!

dhermes commented Nov 15, 2017

Uh oh!

dhermes commented Nov 16, 2017

Uh oh!

dhermes commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhermes commented Nov 21, 2017

Uh oh!

igorbernstein2 commented Nov 21, 2017

Uh oh!

dhermes commented Nov 21, 2017

Uh oh!

calpeyser commented Nov 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

calpeyser commented Aug 15, 2017 •

edited

Loading

dhermes commented Nov 16, 2017 •

edited

Loading