Skip to content

Commit 460b606

Browse files
chemelnucfinchemelnucfin
authored
Spanner: Make rows, consume_all and consume_next private (googleapis#4492)
* Spanner: Make rows, consume_all and consume_next private * review changes * Spanner: remove _consume_all and _rows methods * Spanner: delete unnecessary tests
1 parent 73d979d commit 460b606

4 files changed

Lines changed: 38 additions & 135 deletions

File tree

‎spanner/google/cloud/spanner_v1/streamed.py‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,6 @@ def __init__(self, response_iterator, source=None):
4747
self._pending_chunk = None # Incomplete value
4848
self._source = source # Source snapshot
4949

50-
@property
51-
def rows(self):
52-
"""Fully-processed rows.
53-
54-
:rtype: list of row-data lists.
55-
:returns: list of completed row data, from proceesd PRS responses.
56-
"""
57-
return self._rows
58-
5950
@property
6051
def fields(self):
6152
"""Field descriptors for result set columns.
@@ -115,7 +106,7 @@ def _merge_values(self, values):
115106
self._rows.append(self._current_row)
116107
self._current_row = []
117108

118-
def consume_next(self):
109+
def _consume_next(self):
119110
"""Consume the next partial result set from the stream.
120111
121112
Parse the result set into new/existing rows in :attr:`_rows`
@@ -142,19 +133,11 @@ def consume_next(self):
142133

143134
self._merge_values(values)
144135

145-
def consume_all(self):
146-
"""Consume the streamed responses until there are no more."""
147-
while True:
148-
try:
149-
self.consume_next()
150-
except StopIteration:
151-
break
152-
153136
def __iter__(self):
154137
iter_rows, self._rows[:] = self._rows[:], ()
155138
while True:
156139
if not iter_rows:
157-
self.consume_next() # raises StopIteration
140+
self._consume_next() # raises StopIteration
158141
iter_rows, self._rows[:] = self._rows[:], ()
159142
while iter_rows:
160143
yield iter_rows.pop(0)

‎spanner/tests/system/test_system.py‎

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -859,26 +859,6 @@ def test_multiuse_snapshot_read_isolation_exact_staleness(self):
859859
after = list(exact.read(self.TABLE, self.COLUMNS, self.ALL))
860860
self._check_row_data(after, all_data_rows)
861861

862-
def test_read_w_manual_consume(self):
863-
ROW_COUNT = 3000
864-
session, committed = self._set_up_table(ROW_COUNT)
865-
866-
snapshot = session.snapshot(read_timestamp=committed)
867-
streamed = snapshot.read(self.TABLE, self.COLUMNS, self.ALL)
868-
869-
retrieved = 0
870-
while True:
871-
try:
872-
streamed.consume_next()
873-
except StopIteration:
874-
break
875-
retrieved += len(streamed.rows)
876-
streamed.rows[:] = ()
877-
878-
self.assertEqual(retrieved, ROW_COUNT)
879-
self.assertEqual(streamed._current_row, [])
880-
self.assertEqual(streamed._pending_chunk, None)
881-
882862
def test_read_w_index(self):
883863
ROW_COUNT = 2000
884864
# Indexed reads cannot return non-indexed columns
@@ -1187,17 +1167,11 @@ def test_execute_sql_w_manual_consume(self):
11871167

11881168
snapshot = session.snapshot(read_timestamp=committed)
11891169
streamed = snapshot.execute_sql(self.SQL)
1170+
keyset = KeySet(all_=True)
1171+
rows = list(session.read(self.TABLE, self.COLUMNS, keyset))
1172+
items = [item for item in iter(streamed)]
11901173

1191-
retrieved = 0
1192-
while True:
1193-
try:
1194-
streamed.consume_next()
1195-
except StopIteration:
1196-
break
1197-
retrieved += len(streamed.rows)
1198-
streamed.rows[:] = ()
1199-
1200-
self.assertEqual(retrieved, ROW_COUNT)
1174+
self.assertEqual(items, rows)
12011175
self.assertEqual(streamed._current_row, [])
12021176
self.assertEqual(streamed._pending_chunk, None)
12031177

‎spanner/tests/unit/test_snapshot.py‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,7 @@ def _read_helper(self, multi_use, first=True, count=0):
252252
else:
253253
self.assertIsNone(result_set._source)
254254

255-
result_set.consume_all()
256-
257-
self.assertEqual(list(result_set.rows), VALUES)
255+
self.assertEqual(list(result_set), VALUES)
258256
self.assertEqual(result_set.metadata, metadata_pb)
259257
self.assertEqual(result_set.stats, stats_pb)
260258

@@ -392,9 +390,7 @@ def _execute_sql_helper(self, multi_use, first=True, count=0):
392390
else:
393391
self.assertIsNone(result_set._source)
394392

395-
result_set.consume_all()
396-
397-
self.assertEqual(list(result_set.rows), VALUES)
393+
self.assertEqual(list(result_set), VALUES)
398394
self.assertEqual(result_set.metadata, metadata_pb)
399395
self.assertEqual(result_set.stats, stats_pb)
400396

‎spanner/tests/unit/test_streamed.py‎

Lines changed: 30 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def test_ctor_defaults(self):
3333
streamed = self._make_one(iterator)
3434
self.assertIs(streamed._response_iterator, iterator)
3535
self.assertIsNone(streamed._source)
36-
self.assertEqual(streamed.rows, [])
36+
self.assertEqual(streamed._rows, [])
3737
self.assertIsNone(streamed.metadata)
3838
self.assertIsNone(streamed.stats)
3939

@@ -43,7 +43,7 @@ def test_ctor_w_source(self):
4343
streamed = self._make_one(iterator, source=source)
4444
self.assertIs(streamed._response_iterator, iterator)
4545
self.assertIs(streamed._source, source)
46-
self.assertEqual(streamed.rows, [])
46+
self.assertEqual(streamed._rows, [])
4747
self.assertIsNone(streamed.metadata)
4848
self.assertIsNone(streamed.stats)
4949

@@ -484,7 +484,7 @@ def test_merge_values_empty_and_empty(self):
484484
streamed._metadata = self._make_result_set_metadata(FIELDS)
485485
streamed._current_row = []
486486
streamed._merge_values([])
487-
self.assertEqual(streamed.rows, [])
487+
self.assertEqual(streamed._rows, [])
488488
self.assertEqual(streamed._current_row, [])
489489

490490
def test_merge_values_empty_and_partial(self):
@@ -500,7 +500,7 @@ def test_merge_values_empty_and_partial(self):
500500
VALUES = [self._make_value(bare) for bare in BARE]
501501
streamed._current_row = []
502502
streamed._merge_values(VALUES)
503-
self.assertEqual(streamed.rows, [])
503+
self.assertEqual(streamed._rows, [])
504504
self.assertEqual(streamed._current_row, BARE)
505505

506506
def test_merge_values_empty_and_filled(self):
@@ -516,7 +516,7 @@ def test_merge_values_empty_and_filled(self):
516516
VALUES = [self._make_value(bare) for bare in BARE]
517517
streamed._current_row = []
518518
streamed._merge_values(VALUES)
519-
self.assertEqual(streamed.rows, [BARE])
519+
self.assertEqual(streamed._rows, [BARE])
520520
self.assertEqual(streamed._current_row, [])
521521

522522
def test_merge_values_empty_and_filled_plus(self):
@@ -536,7 +536,7 @@ def test_merge_values_empty_and_filled_plus(self):
536536
VALUES = [self._make_value(bare) for bare in BARE]
537537
streamed._current_row = []
538538
streamed._merge_values(VALUES)
539-
self.assertEqual(streamed.rows, [BARE[0:3], BARE[3:6]])
539+
self.assertEqual(streamed._rows, [BARE[0:3], BARE[3:6]])
540540
self.assertEqual(streamed._current_row, BARE[6:])
541541

542542
def test_merge_values_partial_and_empty(self):
@@ -553,7 +553,7 @@ def test_merge_values_partial_and_empty(self):
553553
]
554554
streamed._current_row[:] = BEFORE
555555
streamed._merge_values([])
556-
self.assertEqual(streamed.rows, [])
556+
self.assertEqual(streamed._rows, [])
557557
self.assertEqual(streamed._current_row, BEFORE)
558558

559559
def test_merge_values_partial_and_partial(self):
@@ -570,7 +570,7 @@ def test_merge_values_partial_and_partial(self):
570570
MERGED = [42]
571571
TO_MERGE = [self._make_value(item) for item in MERGED]
572572
streamed._merge_values(TO_MERGE)
573-
self.assertEqual(streamed.rows, [])
573+
self.assertEqual(streamed._rows, [])
574574
self.assertEqual(streamed._current_row, BEFORE + MERGED)
575575

576576
def test_merge_values_partial_and_filled(self):
@@ -589,7 +589,7 @@ def test_merge_values_partial_and_filled(self):
589589
MERGED = [42, True]
590590
TO_MERGE = [self._make_value(item) for item in MERGED]
591591
streamed._merge_values(TO_MERGE)
592-
self.assertEqual(streamed.rows, [BEFORE + MERGED])
592+
self.assertEqual(streamed._rows, [BEFORE + MERGED])
593593
self.assertEqual(streamed._current_row, [])
594594

595595
def test_merge_values_partial_and_filled_plus(self):
@@ -613,19 +613,19 @@ def test_merge_values_partial_and_filled_plus(self):
613613
TO_MERGE = [self._make_value(item) for item in MERGED]
614614
VALUES = BEFORE + MERGED
615615
streamed._merge_values(TO_MERGE)
616-
self.assertEqual(streamed.rows, [VALUES[0:3], VALUES[3:6]])
616+
self.assertEqual(streamed._rows, [VALUES[0:3], VALUES[3:6]])
617617
self.assertEqual(streamed._current_row, VALUES[6:])
618618

619619
def test_one_or_none_no_value(self):
620620
streamed = self._make_one(_MockCancellableIterator())
621-
with mock.patch.object(streamed, 'consume_next') as consume_next:
621+
with mock.patch.object(streamed, '_consume_next') as consume_next:
622622
consume_next.side_effect = StopIteration
623623
self.assertIsNone(streamed.one_or_none())
624624

625625
def test_one_or_none_single_value(self):
626626
streamed = self._make_one(_MockCancellableIterator())
627627
streamed._rows = ['foo']
628-
with mock.patch.object(streamed, 'consume_next') as consume_next:
628+
with mock.patch.object(streamed, '_consume_next') as consume_next:
629629
consume_next.side_effect = StopIteration
630630
self.assertEqual(streamed.one_or_none(), 'foo')
631631

@@ -644,7 +644,7 @@ def test_one_or_none_consumed_stream(self):
644644
def test_one_single_value(self):
645645
streamed = self._make_one(_MockCancellableIterator())
646646
streamed._rows = ['foo']
647-
with mock.patch.object(streamed, 'consume_next') as consume_next:
647+
with mock.patch.object(streamed, '_consume_next') as consume_next:
648648
consume_next.side_effect = StopIteration
649649
self.assertEqual(streamed.one(), 'foo')
650650

@@ -653,7 +653,7 @@ def test_one_no_value(self):
653653

654654
iterator = _MockCancellableIterator(['foo'])
655655
streamed = self._make_one(iterator)
656-
with mock.patch.object(streamed, 'consume_next') as consume_next:
656+
with mock.patch.object(streamed, '_consume_next') as consume_next:
657657
consume_next.side_effect = StopIteration
658658
with self.assertRaises(exceptions.NotFound):
659659
streamed.one()
@@ -662,7 +662,7 @@ def test_consume_next_empty(self):
662662
iterator = _MockCancellableIterator()
663663
streamed = self._make_one(iterator)
664664
with self.assertRaises(StopIteration):
665-
streamed.consume_next()
665+
streamed._consume_next()
666666

667667
def test_consume_next_first_set_partial(self):
668668
TXN_ID = b'DEADBEEF'
@@ -679,8 +679,8 @@ def test_consume_next_first_set_partial(self):
679679
iterator = _MockCancellableIterator(result_set)
680680
source = mock.Mock(_transaction_id=None, spec=['_transaction_id'])
681681
streamed = self._make_one(iterator, source=source)
682-
streamed.consume_next()
683-
self.assertEqual(streamed.rows, [])
682+
streamed._consume_next()
683+
self.assertEqual(streamed._rows, [])
684684
self.assertEqual(streamed._current_row, BARE)
685685
self.assertEqual(streamed.metadata, metadata)
686686
self.assertEqual(source._transaction_id, TXN_ID)
@@ -700,8 +700,8 @@ def test_consume_next_first_set_partial_existing_txn_id(self):
700700
iterator = _MockCancellableIterator(result_set)
701701
source = mock.Mock(_transaction_id=TXN_ID, spec=['_transaction_id'])
702702
streamed = self._make_one(iterator, source=source)
703-
streamed.consume_next()
704-
self.assertEqual(streamed.rows, [])
703+
streamed._consume_next()
704+
self.assertEqual(streamed._rows, [])
705705
self.assertEqual(streamed._current_row, BARE)
706706
self.assertEqual(streamed.metadata, metadata)
707707
self.assertEqual(source._transaction_id, TXN_ID)
@@ -719,8 +719,8 @@ def test_consume_next_w_partial_result(self):
719719
iterator = _MockCancellableIterator(result_set)
720720
streamed = self._make_one(iterator)
721721
streamed._metadata = self._make_result_set_metadata(FIELDS)
722-
streamed.consume_next()
723-
self.assertEqual(streamed.rows, [])
722+
streamed._consume_next()
723+
self.assertEqual(streamed._rows, [])
724724
self.assertEqual(streamed._current_row, [])
725725
self.assertEqual(streamed._pending_chunk, VALUES[0])
726726

@@ -741,8 +741,8 @@ def test_consume_next_w_pending_chunk(self):
741741
streamed = self._make_one(iterator)
742742
streamed._metadata = self._make_result_set_metadata(FIELDS)
743743
streamed._pending_chunk = self._make_value(u'Phred ')
744-
streamed.consume_next()
745-
self.assertEqual(streamed.rows, [
744+
streamed._consume_next()
745+
self.assertEqual(streamed._rows, [
746746
[u'Phred Phlyntstone', BARE[1], BARE[2]],
747747
[BARE[3], BARE[4], BARE[5]],
748748
])
@@ -767,60 +767,11 @@ def test_consume_next_last_set(self):
767767
iterator = _MockCancellableIterator(result_set)
768768
streamed = self._make_one(iterator)
769769
streamed._metadata = metadata
770-
streamed.consume_next()
771-
self.assertEqual(streamed.rows, [BARE])
770+
streamed._consume_next()
771+
self.assertEqual(streamed._rows, [BARE])
772772
self.assertEqual(streamed._current_row, [])
773773
self.assertEqual(streamed._stats, stats)
774774

775-
def test_consume_all_empty(self):
776-
iterator = _MockCancellableIterator()
777-
streamed = self._make_one(iterator)
778-
streamed.consume_all()
779-
780-
def test_consume_all_one_result_set_partial(self):
781-
FIELDS = [
782-
self._make_scalar_field('full_name', 'STRING'),
783-
self._make_scalar_field('age', 'INT64'),
784-
self._make_scalar_field('married', 'BOOL'),
785-
]
786-
metadata = self._make_result_set_metadata(FIELDS)
787-
BARE = [u'Phred Phlyntstone', 42]
788-
VALUES = [self._make_value(bare) for bare in BARE]
789-
result_set = self._make_partial_result_set(VALUES, metadata=metadata)
790-
iterator = _MockCancellableIterator(result_set)
791-
streamed = self._make_one(iterator)
792-
streamed.consume_all()
793-
self.assertEqual(streamed.rows, [])
794-
self.assertEqual(streamed._current_row, BARE)
795-
self.assertEqual(streamed.metadata, metadata)
796-
797-
def test_consume_all_multiple_result_sets_filled(self):
798-
FIELDS = [
799-
self._make_scalar_field('full_name', 'STRING'),
800-
self._make_scalar_field('age', 'INT64'),
801-
self._make_scalar_field('married', 'BOOL'),
802-
]
803-
metadata = self._make_result_set_metadata(FIELDS)
804-
BARE = [
805-
u'Phred Phlyntstone', 42, True,
806-
u'Bharney Rhubble', 39, True,
807-
u'Wylma Phlyntstone', 41, True,
808-
]
809-
VALUES = [self._make_value(bare) for bare in BARE]
810-
result_set1 = self._make_partial_result_set(
811-
VALUES[:4], metadata=metadata)
812-
result_set2 = self._make_partial_result_set(VALUES[4:])
813-
iterator = _MockCancellableIterator(result_set1, result_set2)
814-
streamed = self._make_one(iterator)
815-
streamed.consume_all()
816-
self.assertEqual(streamed.rows, [
817-
[BARE[0], BARE[1], BARE[2]],
818-
[BARE[3], BARE[4], BARE[5]],
819-
[BARE[6], BARE[7], BARE[8]],
820-
])
821-
self.assertEqual(streamed._current_row, [])
822-
self.assertIsNone(streamed._pending_chunk)
823-
824775
def test___iter___empty(self):
825776
iterator = _MockCancellableIterator()
826777
streamed = self._make_one(iterator)
@@ -841,7 +792,7 @@ def test___iter___one_result_set_partial(self):
841792
streamed = self._make_one(iterator)
842793
found = list(streamed)
843794
self.assertEqual(found, [])
844-
self.assertEqual(streamed.rows, [])
795+
self.assertEqual(streamed._rows, [])
845796
self.assertEqual(streamed._current_row, BARE)
846797
self.assertEqual(streamed.metadata, metadata)
847798

@@ -869,7 +820,7 @@ def test___iter___multiple_result_sets_filled(self):
869820
[BARE[3], BARE[4], BARE[5]],
870821
[BARE[6], BARE[7], BARE[8]],
871822
])
872-
self.assertEqual(streamed.rows, [])
823+
self.assertEqual(streamed._rows, [])
873824
self.assertEqual(streamed._current_row, [])
874825
self.assertIsNone(streamed._pending_chunk)
875826

@@ -902,7 +853,7 @@ def test___iter___w_existing_rows_read(self):
902853
[BARE[3], BARE[4], BARE[5]],
903854
[BARE[6], BARE[7], BARE[8]],
904855
])
905-
self.assertEqual(streamed.rows, [])
856+
self.assertEqual(streamed._rows, [])
906857
self.assertEqual(streamed._current_row, [])
907858
self.assertIsNone(streamed._pending_chunk)
908859

@@ -952,11 +903,10 @@ def _match_results(self, testcase_name, assert_equality=None):
952903
partial_result_sets, expected = self._load_json_test(testcase_name)
953904
iterator = _MockCancellableIterator(*partial_result_sets)
954905
partial = self._make_one(iterator)
955-
partial.consume_all()
956906
if assert_equality is not None:
957-
assert_equality(partial.rows, expected)
907+
assert_equality(list(partial), expected)
958908
else:
959-
self.assertEqual(partial.rows, expected)
909+
self.assertEqual(list(partial), expected)
960910

961911
def test_basic(self):
962912
self._match_results('Basic Test')

0 commit comments

Comments
 (0)