Spanner: Make rows, consume_all and consume_next private#4492
Merged
chemelnucfin merged 4 commits intoDec 13, 2017
Conversation
Contributor
Author
|
@tseaver PTAL, thanks. |
tseaver
reviewed
Dec 6, 2017
| def __init__(self, response_iterator, source=None): | ||
| self._response_iterator = response_iterator | ||
| self._rows = [] # Fully-processed rows | ||
| self._processed_rows = [] # Fully-processed rows |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
tseaver
suggested changes
Dec 6, 2017
| while True: | ||
| try: | ||
| streamed.consume_next() | ||
| streamed._consume_next() |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| retrieved += len(streamed.rows) | ||
| streamed.rows[:] = () | ||
| retrieved += len(streamed._rows) | ||
| streamed._rows[:] = () |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
cfa42c0 to
f1c3cd8
Compare
tseaver
suggested changes
Dec 6, 2017
tseaver
left a comment
Contributor
There was a problem hiding this comment.
Let's just delete the two system tests: they are redundant with those in other testcases.
f1c3cd8 to
b4e4551
Compare
vkedia
reviewed
Dec 7, 2017
|
|
||
| @property | ||
| def rows(self): | ||
| def _rows(self): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
vkedia
reviewed
Dec 7, 2017
| self._merge_values(values) | ||
|
|
||
| def consume_all(self): | ||
| def _consume_all(self): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Contributor
Author
tseaver
suggested changes
Dec 13, 2017
| self.assertIsNone(result_set._source) | ||
|
|
||
| result_set._consume_all() | ||
| # result_set._consume_all() |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| # result_set._consume_all() | ||
|
|
||
| self.assertEqual(list(result_set._rows), VALUES) | ||
| self.assertEqual(list(result_set), VALUES) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
d5d3378 to
2ed147a
Compare
Contributor
Author
|
I just changed the _rows to list. I will put that in another PR. |
2ed147a to
c34f2c9
Compare
tseaver
approved these changes
Dec 13, 2017
parthea
pushed a commit
that referenced
this pull request
Mar 9, 2026
* Spanner: Make rows, consume_all and consume_next private * review changes * Spanner: remove _consume_all and _rows methods * Spanner: delete unnecessary tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #4414 and #4204