#514: rip out `Connection` cruft #550

tseaver · 2015-01-14T21:06:05Z

For hygeine:

Hide batch._BATCHES.top behind batch.Batch.current() classmethod.

Remove `Connection.save_entity()``
Remove Connection.delete_entities()
Remove Connection.mutation
Remove Connection.transaction
- Make Connection.lookup and Connection.run_query depend on transaction
  on top of the _BATCHES stack.

dhermes · 2015-01-14T21:10:51Z

Does this depend on #548? I have yet to view the code, but is

Make Connection.lookup and Connection.run_query depend on transaction on top
of the _BATCHES stack.

meant to be a temporary replacement for the interaction of Transaction and Connection or is is meant to be permanent?

I was imagining a world in which Connection was "stateless" and relied on the callers to tell it how to send the RPCs. Really just a stand-alone mapping for DatastoreService'.

tseaver · 2015-01-14T21:18:42Z

Yes, based on #548.

To accomplish what you are asking, we would need to have Connection.lookup and Connection.run_query take a transaction_id argument (like Connection.commit).

coveralls · 2015-01-14T21:20:06Z

Coverage remained the same when pulling b1935fe on tseaver:514-rip_out_connection_cruft into f980aad on GoogleCloudPlatform:master.

dhermes · 2015-01-14T21:25:51Z

That doesn't seem like too big an issue. Are you opposed to that?

tseaver · 2015-01-14T21:33:27Z

It means moving the logic we currently have in Connection._set_read_options somewhere else (maybe a helper called by a new top-level lookup() API, as well as from the query iterator?)

coveralls · 2015-01-14T21:46:11Z

Coverage remained the same when pulling 76a918e on tseaver:514-rip_out_connection_cruft into f980aad on GoogleCloudPlatform:master.

tseaver · 2015-01-14T22:18:37Z

Rebased after merging #548.

coveralls · 2015-01-14T22:18:58Z

Changes Unknown when pulling c449af6 on tseaver:514-rip_out_connection_cruft into * on GoogleCloudPlatform:master*.

coveralls · 2015-01-14T22:21:27Z

Coverage remained the same when pulling c449af6 on tseaver:514-rip_out_connection_cruft into 64aaed4 on GoogleCloudPlatform:master.

gcloud/datastore/helpers.py

dhermes · 2015-01-14T23:54:36Z

OK I did a commit by commit review and even managed to put my comments on the PR instead of the commits (pats self on back).

This mostly looks good except I think you should refactor Connection.lookup, Connection.run_query and connection._set_read_options to take a transaction ID and then make sure that api.get passes that ID to Connection.lookup and query.Iterator.next_page passes that ID to Connection.run_query.

coveralls · 2015-01-15T00:56:14Z

Coverage remained the same when pulling 0aa8804 on tseaver:514-rip_out_connection_cruft into 64aaed4 on GoogleCloudPlatform:master.

- Folds in changes from #404 / #512 to avoid reintroducing #403. See: https://github.com/GoogleCloudPlatform/gcloud-python/pull/550/files#r22982145.

Avoid having callers depend on private singleton, _BATCHES.

Returns top of stack IFF it is a transaction, or None.

coveralls · 2015-01-15T01:36:48Z

Coverage remained the same when pulling f10620e on tseaver:514-rip_out_connection_cruft into 64aaed4 on GoogleCloudPlatform:master.

tseaver · 2015-01-15T02:12:52Z

@dhermes the following two commits do as you recommend:

301498d adds a Transaction.current() class method, which returns the top-most item on the batch
stack IFF it is a tranaction (otherwise, None).
a4a65a2 makes the Connection methods take an explicit transaction_id, and uses the Transaction.current() bit to find it in the callers (datasore.get() and QueryIterator.next_page())

dhermes · 2015-01-15T02:22:31Z

LGTM w00t! (Sorry I am reviewing from phone.)

#514: rip out `Connection` cruft

Source-Link: https://togithub.com/googleapis/synthtool/commit/0ddbff8012e47cde4462fe3f9feab01fbc4cdfd6 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bced5ca77c4dda0fd2f5d845d4035fc3c5d3d6b81f245246a36aee114970082b

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Source-Link: googleapis/synthtool@352b9d4 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:3e3800bb100af5d7f9e810d48212b37812c1856d20ffeafb99ebe66461b61fc7 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* chore(deps): update all dependencies * fix(deps): allow pyarrow < 10 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: update dependency google-cloud-bigquery==3.3.1 Co-authored-by: Anthonios Partheniou <[email protected]> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Source-Link: googleapis/synthtool@06e8279 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:b3500c053313dc34e07b1632ba9e4e589f4f77036a7cf39e1fe8906811ae0fce

Otherwise this file is excluded from generation. Partially addresses #437

* chore: update templated files * remove obsolete replacements in owlbot.py * update replacements in owlbot.py * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Kevin Zheng <[email protected]>

* build(python): use release-publish app for notifying GitHub of release status * fix: re-add pypi password Source-Author: Bu Sun Kim <[email protected]> Source-Date: Wed Sep 16 08:46:42 2020 -0600 Source-Repo: googleapis/synthtool Source-Sha: 257fda18168bedb76985024bd198ed1725485488 Source-Link: googleapis/synthtool@257fda1 * build(python): add secret manager in kokoro Source-Author: Bu Sun Kim <[email protected]> Source-Date: Wed Sep 16 10:24:40 2020 -0600 Source-Repo: googleapis/synthtool Source-Sha: dba48bb9bc6959c232bec9150ac6313b608fe7bd Source-Link: googleapis/synthtool@dba48bb

dhermes mentioned this pull request Jan 14, 2015

Decouple Transaction from Connection #514

Closed

8 tasks

tseaver mentioned this pull request Jan 14, 2015

#514: add datastore.put API, remove `Entity.save' #548

Merged

dhermes reviewed Jan 14, 2015
View reviewed changes

gcloud/datastore/helpers.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

tseaver added 8 commits January 14, 2015 20:31

Skip empty lists.

de03993

- Folds in changes from #404 / #512 to avoid reintroducing #403. See: https://github.com/GoogleCloudPlatform/gcloud-python/pull/550/files#r22982145.

#514: rip out 'Connection.save_entity' and 'Connection.delete_entities'.

3527809

Add 'Batch.current' static method.

e68ce53

Avoid having callers depend on private singleton, _BATCHES.

Rip out 'Connection.mutation'.

82a97e5

No longer store the transaction on the connection.

dd7a598

Add 'Transaction.current' staticmethod.

301498d

Returns top of stack IFF it is a transaction, or None.

Add explicit 'transaction_id' parm to 'Connection.{lookup,run_query}'.

a4a65a2

De-lint continuations.

f10620e

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 15, 2015

tseaver added a commit that referenced this pull request Jan 15, 2015

Merge pull request #550 from tseaver/514-rip_out_connection_cruft

12ac983

#514: rip out `Connection` cruft

tseaver merged commit 12ac983 into googleapis:master Jan 15, 2015

tseaver deleted the 514-rip_out_connection_cruft branch January 15, 2015 03:14

This was referenced Jan 15, 2015

Adding one-time RPC to find unaliased / true dataset ID. #528

Closed

Transaction does not get rolled back if exception happens during commit. #457

Closed

Sniff out and fix duplicated code replaced by _set_protobuf_property #540

Closed

vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023

samples: update logs for knowledge base management sample (#550)

6b0347e

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore(main): release 3.18.0 (#550)

24d18de

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

parthea pushed a commit that referenced this pull request Aug 21, 2025

chore(deps): update dependency pytest to v7.2.1 (#550)

f729448

parthea pushed a commit that referenced this pull request Sep 16, 2025

chore(deps): update dependency pytest to v7.2.1 (#550)

595f2de

parthea pushed a commit that referenced this pull request Nov 24, 2025

fix: rename __init__.py to __init__.py.j2 (#550)

173ad90

Otherwise this file is excluded from generation. Partially addresses #437

parthea pushed a commit that referenced this pull request Nov 24, 2025

feat: support with_call for wrapped rpcs (#550)

1748010

#514: rip out Connection cruft #550

#514: rip out Connection cruft #550

Uh oh!

Conversation

tseaver commented Jan 14, 2015

Uh oh!

dhermes commented Jan 14, 2015

Uh oh!

tseaver commented Jan 14, 2015

Uh oh!

coveralls commented Jan 14, 2015

Uh oh!

dhermes commented Jan 14, 2015

Uh oh!

tseaver commented Jan 14, 2015

Uh oh!

coveralls commented Jan 14, 2015

Uh oh!

tseaver commented Jan 14, 2015

Uh oh!

coveralls commented Jan 14, 2015

Uh oh!

coveralls commented Jan 14, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Jan 14, 2015

Uh oh!

coveralls commented Jan 15, 2015

Uh oh!

coveralls commented Jan 15, 2015

Uh oh!

tseaver commented Jan 15, 2015

Uh oh!

dhermes commented Jan 15, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

#514: rip out `Connection` cruft #550

#514: rip out `Connection` cruft #550