#825: Allow passing explicit connection to dataset API methods.

[tseaver]

See #825.

[coveralls]

Coverage remained the same at 100.0% when pulling 9130c03 on tseaver:825-datastore-dataset_explicit_connection into 8b5c5b9 on GoogleCloudPlatform:master.

[dhermes]

I agree with the spirit of #825 but I think the Dataset was already doing the right thing. Maybe we should just make the connection required in the constructor?

[tseaver]

ISTM that it would be better to just delete the connection attribute altogether from Dataset, because the methods it proxies to already DTRT if pass connection=None.

[dhermes]

If we go the "clients everywhere" route, then Dataset needs to hold the same properties as _DefaultsContainer.

[tseaver]

I'm actually opposed to the "client" notion at this point: I think the "use a connection as a context manager" bit would work better. We don't need to weld the project ID together with the connection, since it is only passed in a few cases.

[tseaver]

I think @jgeewax was actually in agreement with that notion at the end of the last call, too.

[jgeewax]

I'm a fan of the Client pattern that Danny is tossing around -- take a look at #861? Maybe we can discuss it there?

[dhermes]

It would be nice to avoid churn if we could.

I think making connections constructable with one-liner factories that also pull in credentials will make things easier (in addition to making them have long-lived contexts). If we need clients, we can go that route, but if we don't we can move forward instead of chasing our tail.

[tseaver]

@dhermes a671672 removes the Dataset.connection attribute altogether -- I think it makes for a better DX.

[dhermes]

@tseaver How does it help? What is better? It certainly doesn't capture the idea of a pre-loaded object. And after the change Dataset just becomes a string with a bunch of methods.

[coveralls]

Coverage remained the same at 100.0% when pulling a671672 on tseaver:825-datastore-dataset_explicit_connection into 8b5c5b9 on GoogleCloudPlatform:master.

[tseaver]

The goal of the Dataset class was to ease the process of constructing many-many objects which needed to be passed a dataset_id in their ctors. Bundling in yet-another-way-to-spell-a-non-default-connection adds neither clarity for the user, who is likely to be confused about how to signal it, nor in straightforwardness of implementation.

Except for Batch and Transaction objects, which necessarily wrap a connection object, I would say that any connection attribute in a user-visible object is a misfeature, across all the APIs.

[dhermes]

This really just brings up the question: do all config defaults matter as a bundle or just the some of them.

[tseaver]

ISTM we can't move on this until #861 settles out.

[dhermes]

Indeed

[dhermes]

@tseaver this doesn't exactly fit the pattern of #910. I image you'll be binding clients to make Transaction work anyhow?