Skip to content

Conversation

wchargin
Copy link
Contributor

Summary:
These docs used to be correct, but Session.run_in_transaction was
changed from returning the commit timestamp to returning the function
value in #3753, and the upstream docs were not updated.

The docs for run_in_transaction are now identical across Database
and Session.

The tests for run_in_transaction (in test_database.py) are wrong, as
they mock out the session object instead of using a real session, and
they also were not updated in #3753. This change does not fix them.

Test Plan:

$ virtualenv -q -p python3.6 ./ve
$ . ./ve/bin/activate
(ve) $ pip install -q google-cloud-spanner==1.10.0
(ve) $ pip freeze | grep google-cloud-spanner
google-cloud-spanner==1.10.0
(ve) $ cat test.py; echo
from google.cloud import spanner_v1

client = spanner_v1.Client()
instance = client.instance("my-instance-name")
database = instance.database("my-database-name")
result = database.run_in_transaction(lambda txn: "ahoy")
print(result)

(ve) $ python test.py
ahoy

wchargin-branch: run-in-transaction-rval

Summary:
These docs used to be correct, but `Session.run_in_transaction` was
changed from returning the commit timestamp to returning the function
value in googleapis#3753, and the upstream docs were not updated.

The docs for `run_in_transaction` are now identical across `Database`
and `Session`.

The tests for `run_in_transaction` (in `test_database.py`) are wrong, as
they mock out the session object instead of using a real session, and
they also were not updated in googleapis#3753. This change does not fix them.

Test Plan:

```
$ virtualenv -q -p python3.6 ./ve
$ . ./ve/bin/activate
(ve) $ pip install -q google-cloud-spanner==1.10.0
(ve) $ pip freeze | grep google-cloud-spanner
google-cloud-spanner==1.10.0
(ve) $ cat test.py; echo
from google.cloud import spanner_v1

client = spanner_v1.Client()
instance = client.instance("my-instance-name")
database = instance.database("my-database-name")
result = database.run_in_transaction(lambda txn: "ahoy")
print(result)

(ve) $ python test.py
ahoy
```

wchargin-branch: run-in-transaction-rval
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 21, 2019
@plamut plamut added api: spanner Issues related to the Spanner API. type: docs Improvement to the documentation for an API. labels Sep 23, 2019
@plamut plamut changed the title Fix run_in_transaction return value docs Spanner: Fix run_in_transaction return value docs Sep 23, 2019
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 23, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 23, 2019
@tseaver tseaver merged commit 4540b23 into googleapis:master Sep 23, 2019
@tseaver
Copy link
Contributor

tseaver commented Sep 23, 2019

@wchargin Thanks for the patch!

@wchargin wchargin deleted the wchargin-run-in-transaction-rval branch September 23, 2019 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants