mszb commented on a change in pull request #11210:
URL: https://github.com/apache/beam/pull/11210#discussion_r422280614
##########
File path: sdks/python/apache_beam/io/gcp/experimental/spannerio_test.py
##########
@@ -499,6 +499,7 @@ def test_batch_byte_size(
# and each bach should contains 25 mutations.
res = (
p | beam.Create(mutation_group)
+ | 'combine to list' >> beam.combiners.ToList()
Review comment:
The user does not have to add ToList transform in the production
pipeline. I only added this to test the batch process.
The previous implementation of batching (without ToList transform) was as
per the java implementation but without the sorting of the transactions by
table and primary key (this is also documented as a feature to be added later).
##########
File path: sdks/python/apache_beam/io/gcp/experimental/spannerio.py
##########
@@ -1008,31 +1007,30 @@ def _reset_count(self):
self._cells = 0
def process(self, element):
- mg_info = element.info
+ for elem in element:
Review comment:
Make sense, in that case, we don't need to alter the connector code
anymore, it was working as expected. Thanks, @chamikaramj for the feedback as
it is always helpful.
I'll remove the changes from the spanner io connector and update the IT test
code for the assertion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
