Address second part of 477: Remove query(), entity(), and transaction() methods from Dataset.

[dhermes]

NOTE: Has #478 as diffbase.

[coveralls]

Coverage remained the same when pulling 56e9c05 on dhermes:fix-477-part2 into b79d862 on GoogleCloudPlatform:master.

[elibixby]

I'm not sure about removing transaction from dataset (and dataset/transactions in general). Is it not valuable to support working on multiple transactions simultaneously? Transactions are not unique to a dataset much less a connection.

[dhermes]

@elibixby This is part of removing the Dataset class (#477), so we are just removing the methods on that class piece-by-piece. The functionality isn't going away, just the factory which duplicates a constructor from elsewhere as a convenience.

[elibixby]

@dhermes and it was determined that editing multiple datasets simultaneously was not a valuable enough use case to justify a class which stores dataset-id and forwards methods to connection (so users don't have to track string literals?) it does seem like an edge case, but also I kinda like the abstraction of connection.

EDIT: This was addressed briefly in the design doc, but it seems like we are moving towards splitting methods across Key and Entity (e.g. #480 ). I can't see how this is preferable to more unified access through a dataset object. If all methods go through a dataset object, the user does not need to be aware of a connection object, and the number of classes they must learn remains the same (or is fewer if Key and Entity eventually disappear). I gave example usage of this in #438

[dhermes]

RE: "Tracking string literals". Carrying around dataset_id = 'foo' is no easier or harder than carrying around dataset = Dataset('foo'). Maybe I'm missing something?

RE: "Multiple datasets". This support is not being removed.

RE: "Splitting methods across Key and Entity". I had hoped to put them all on Key but .save() depends on the actual data as does .reload(). (I am not sure reload() is needed.) The datastore team asked us to take get off the Entity but maybe putting them all in datastore.__init__ is the right way to go.

All this notwithstanding, the Dataset class is just a wrapper around a single string and a Connection which does all the work.

[elibixby]

Other than the fact that dataset.save(stuff) feels more natural to me than connection.save(stuff, dataset_id) (which I have no rational argument for):

Eliminating dataset prevents us from allowing users to associate persistent settings on a per dataset basis, for example, eventually consistent reads, or forced mutations. Instead they have to specify these options each time they call the method in connection.

In my mind a dataset was an immutable wrapper for all the optional arguments to mutations and fetches. So the interface for save, delete, etc can be much cleaner, while the API still has full functionality. In my api mock up, dataset needed to do a lot of work, not just keeping track of an id:

• keep track of read_options
• automatically attach partition_ids to keys (to support namespacing)
• keep track of mutation options
• super-class for transaction (which behaving similarly, in addition to keeping track of a transaction_id and sanity checking read/write options)
• factory for transactions and bulk mutators (clones over option fields to the new objects)

None of this functionality should be merged into connection, as it all depends on options which could reasonably be different depending on the dataset.

All of this could be done with optional arguments to connection methods but it feels much cleaner to have them wrapped in an object.

[dhermes]

These seem like great ideas, can we track elsewhere?

RE: Methods on connections, the idea is for people to execute something like

from gcloud import datastore
datastore.set_defaults()

and then from then forward

datastore.get_entities(keys)
datastore.allocate_ids(incomplete_key, num_keys)
...

etc.

[tseaver]

@dhermes, you you want to rebase, or should we just merge it manually.

[tseaver]

LGTM, by the way.

[elibixby]

@dhermes

with that datastore syntax how do users interact with multiple datasets/namespaces?

Also would you like me to create an issue? Alternatively I can try to sketch out a basic implementation and make a pull request for discussion. Whichever you prefer.

[dhermes]

@elibixby Please file an issue. Let's discuss please before any implementation.

[coveralls]

Coverage remained the same when pulling 6ef0156 on dhermes:fix-477-part2 into 8727c0d on GoogleCloudPlatform:master.