Spanner: Make rows, consume_all and consume_next private - #4492
Merged
chemelnucfin merged 4 commits intoDec 13, 2017
Merged
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
tseaver
reviewed
Dec 6, 2017
Comment thread
spanner/google/cloud/spanner_v1/streamed.py
Outdated
| 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.
tseaver
suggested changes
Dec 6, 2017
Comment thread
spanner/tests/system/test_system.py
Outdated
| while True: | ||
| try: | ||
| streamed.consume_next() | ||
| streamed._consume_next() |
This comment was marked as spam.
Comment thread
spanner/tests/system/test_system.py
Outdated
| retrieved += len(streamed.rows) | ||
| streamed.rows[:] = () | ||
| retrieved += len(streamed._rows) | ||
| streamed._rows[:] = () |
This comment was marked as spam.
chemelnucfin
force-pushed
the
spanner_rows_rename
branch
from
cfa42c0 to
f1c3cd8
Compare
December 6, 2017 20:20
tseaver
suggested changes
Dec 6, 2017
tseaver
left a comment
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Let's just delete the two system tests: they are redundant with those in other testcases.
Sorry, something went wrong.
chemelnucfin
force-pushed
the
spanner_rows_rename
branch
from
f1c3cd8 to
b4e4551
Compare
December 6, 2017 22:56
vkedia
reviewed
Dec 7, 2017
Comment thread
spanner/google/cloud/spanner_v1/streamed.py
Outdated
|
|
||
| @property | ||
| def rows(self): | ||
| def _rows(self): |
This comment was marked as spam.
vkedia
reviewed
Dec 7, 2017
Comment thread
spanner/google/cloud/spanner_v1/streamed.py
Outdated
| self._merge_values(values) | ||
|
|
||
| def consume_all(self): | ||
| def _consume_all(self): |
This comment was marked as spam.
Copy link
Copy Markdown
Contributor
Author
Sorry, something went wrong.
tseaver
suggested changes
Dec 13, 2017
Comment thread
spanner/tests/unit/test_snapshot.py
Outdated
| 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.
Comment thread
spanner/tests/unit/test_snapshot.py
| # result_set._consume_all() | ||
|
|
||
| self.assertEqual(list(result_set._rows), VALUES) | ||
| self.assertEqual(list(result_set), VALUES) |
This comment was marked as spam.
chemelnucfin
force-pushed
the
spanner_rows_rename
branch
from
d5d3378 to
2ed147a
Compare
December 13, 2017 18:44
Copy link
Copy Markdown
Contributor
Author
|
I just changed the _rows to list. I will put that in another PR. |
Sorry, something went wrong.
chemelnucfin
force-pushed
the
spanner_rows_rename
branch
from
2ed147a to
c34f2c9
Compare
December 13, 2017 19:21
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
Reviewers
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