#514: rip out Connection cruft

[tseaver]

For hygeine:

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

Per #514:

• 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]

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]

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]

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

[dhermes]

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

[tseaver]

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]

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

[tseaver]

Rebased after merging #548.

[coveralls]

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

[coveralls]

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

[dhermes]

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]

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

[coveralls]

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

[tseaver]

@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]

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