Spanner: StreamedResult.rows() is confusing and error prone

[vkedia]

One of Cloud Spanner customers had a bug where they were iterating on StreamedResultSet.rows and were not getting any data back:

result = transaction.execute_sql(query)
        # here result.rows is empty
        for row in result.rows:
            print(row) # prints nothing

This is kind of working as intended because result.rows will only contain data when consume_next or consume_all is called. But it is still a very confusing API and easy to get wrong. It is also not clear to me at any point in time what rows will be present in this list.
I think we should make the rows private and not expose it to the user. We should do it before marking the library as GA as this is a breaking change.

cc @lukesneeringer @tseaver @danoscarmike

[danoscarmike]

@vkedia is this only an issue for Python? Do other languages implement it a better way?

[vkedia]

I believe so. Definitely Java and Go don't do it like this. We will need to check other languages.

[vkedia]

Another user running into the same issue:
https://stackoverflow.com/questions/47311934/read-with-keysets-ranges

[chemelnucfin]

@vkedia I have checked the code and identified a possible problem where the lists are checked for [] and None when it should be checked only for None. However, I'm getting ResourceExhasuted errors and I can't further test the issue.

[vkedia]

That usually means that either you are out of disk or you are sending too much load to your spanner instance indicated by CPU usage. You should be able to look at these details in cloud console

[vkedia]

BTW in this issue I am more concerned with the API itself rather than any bug in its implementation. I don't think we should expose StreamedResultSet.rows at all since users can just iterator over StreamedResultSet directly.

[lukesneeringer]

@chemelnucfin I think all @vkedia is asking for is, "Change the rows attribute to be spelled _rows."