Skip to content

Bigtable: Add 'PartialRowsData.cancel'. - #8176

Merged
tseaver merged 9 commits into
googleapis:masterfrom
mf2199:bigtable_io
Jun 20, 2019
Merged

Bigtable: Add 'PartialRowsData.cancel'.#8176
tseaver merged 9 commits into
googleapis:masterfrom
mf2199:bigtable_io

Conversation

@mf2199

@mf2199 mf2199 commented May 27, 2019

Copy link
Copy Markdown
Contributor

Fixes #7760.

Introducing a "stop" flag into the iterable PartialRowsData() class. Setting this flag to True results in cancelling of the iteration the next time the iterator is called.

@mf2199
mf2199 requested a review from tseaver as a code owner May 27, 2019 13:27
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 27, 2019

@sduskis sduskis left a comment

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 change needs a unit test, and ideally a system test.

Comment thread bigtable/google/cloud/bigtable/row_data.py Outdated
Comment thread bigtable/google/cloud/bigtable/row_data.py Outdated
@sduskis sduskis changed the title Addressing issue #7760 Bigtable: improve reads rows cancel May 28, 2019
@sduskis

sduskis commented May 31, 2019

Copy link
Copy Markdown
Contributor

@mf2199, I had some comments that need to be addressed regarding this PR.

mf2199 added 2 commits June 1, 2019 21:43
Appending unit-test to assert stop iteration of PartialRowsData() upon calling the cancel() method.
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jun 4, 2019
@sduskis sduskis added the api: bigtable Issues related to the Bigtable API. label Jun 17, 2019
Comment thread bigtable/google/cloud/bigtable/row_data.py Outdated
Comment thread bigtable/tests/unit/test_row_data.py Outdated
@mf2199
mf2199 requested review from crwilcox and frankyn as code owners June 19, 2019 16:42
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2019
@tseaver

tseaver commented Jun 19, 2019

Copy link
Copy Markdown
Contributor

The snippets-2.7 failures seem to be related to an outage (?) on the back-end (or maybe the Bigtable API returns 503 responses instead of using 429 for rate limiting?).

@tseaver tseaver changed the title Bigtable: improve reads rows cancel Bigtable: Add 'PartialRowsData.cancel'. Jun 19, 2019
self._state = self.STATE_NEW_ROW

# Flag to stop iteration, for any reason not related to self.retry()
self._stop = False

@crwilcox crwilcox Jun 19, 2019

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.

Looking over the code, it seems this is marking if the iterator has been cancelled. The comment and name of this sort of directed me to think maybe this did something different. would naming this self._cancelled be accurate?

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.

+1 for _cancelled

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Done.

@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2019
@tseaver

tseaver commented Jun 20, 2019

Copy link
Copy Markdown
Contributor

@tseaver
tseaver merged commit e65044c into googleapis:master Jun 20, 2019
parthea pushed a commit that referenced this pull request Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to cancel Cloud Bigtable reads

7 participants