ReadOptions support for 'Connection.lookup', 'Connection.run_query'

[tseaver]

The protobuf specs for the lookup RPC and the runQuery RPC show an optional 'read_options' field, which denominate the consistency level and optional transaction to be used. Do we need to support this field?

[tseaver]

FWIW, the protobuf spec for the ReadOptions message says:

// The read consistency to use.
// Cannot be set when transaction is set.
// Lookup and ancestor queries default to STRONG, global queries default to
// EVENTUAL and cannot be set to STRONG.

Connection.lookup() would thus resemble:

def lookup(self, dataset_id, key_pbs, transaction=None, eventual=False):
    ...

• If transaction was passed, we would copy it onto the ReadOpttions.transaction field.
• Otherwise, if eventual were True, we would set the enum value onto the ReadOptions.read_consistency field.

We would need to update Dataset.get_entities to allow passing transaction and eventual through to Connection.lookup.

Likewise, Connection.run_query would resemble:

def run_query(self, dataset_id, query_pb, namespace=None,
              transaction=None, read_consistency=None):
    ...

• If transaction was passed, we would copy it onto the ReadOpttions.transaction field.
• Otherwise, if read_consistency were not-none (one of "STRONG" or "EVENTUAL"), we would set the corresponding enum value onto the ReadOptions.read_consistency field. We would not try to pre-enforce the semantics described in the protobuf spec.

We would need to update Query.fetch to allow passing transaction and read_consistency through to Connection.run_query.

[silvolu]

Sounds good to me. 2 questions:

• In Connection.lookup and Connection.run_query, should the default value for transaction be the value of the Connection's object transaction, so that if we are already inside a transaction, it's automatically applied ?
• In Dataset.get_entities and Query.fetch, should we raise an error if a transaction and eventual=True are passed, or silently ignore it and favor one of the two options?

[tseaver]

IRT the first question, that sounds sensible to me (and most like what a user would expect).

For the second, we might need to rely on having the Connection methods raise an exception (since the allowed values would depend on whether a transaction was active).

[silvolu]

Cool, SGTM.

[tseaver]

I think maybe we leave transaction out of the signature of Connection.lookup and Query.fetch: if the connection already has a transaction, then we just use it (and raise an error if eventual is passed). Otherwise, if eventual is passed we set the flag on the request.

I don't know if we should be trying to pre-enforce that STRONG is incompatible with queries that are not ancestor-based. I'm assuming that the back-end returns an error if the QueryRequest protobuf contains it with a non-ancestor filter.