Smooth out API surfaces before M1

[dhermes]

Current offenders:

1. There is no way to allocate_ids without explicitly using protobufs. I have partially addressed (Implemented in Adding a non-protobuf allocate_ids method on Dataset. #436).

2. Transactions don't give any indication of success or failure (i.e. commit or rollback in __exit__). (Filed DISCUSSION: Is there a way for a Transaction to indicate success/failure from __exit__ #496)

3. Query (and other classes) could be more Pythonic by allowing us to pass values to constructor that also have their own setters. For example:

   query = dataset.query('Character').ancestor(
       ANCESTOR_KEY).limit(10).order('appearances')

   becomes

   query = dataset.query('Character', ancestor=ANCESTOR_KEY,
                         limit=10, order='appearances')

   We don't have to go as extreme as DISCUSSION: Are setter methods which clone and return object type confusing to end-users? #188 by wiping away the return self behavior in the setters, but could use them in conjuction with more Pythonic constructors. (Implemented in Fix #439: split query vs. iterator apart. #487)

4. Query.cursor() should not raise an error before being used. Using query.cursor() is None gives a way to check progress in a query. For example, in paging: (addressed in Removing cursor and more results from query object. #432)

   query = dataset.query(kind=kind).limit(FETCH_MAX)
   results = []
   more_results = True
   while more_results:
       # Make new query.
       if query._cursor is not None:
           query = query.with_cursor(query._cursor)
   
       curr_results = query.fetch()
       results.extend(curr_results)
   
       more_results = len(curr_results) == FETCH_MAX

5. Query should give access to start_cursor and end_cursor. It is currently only set on the pb. (see Removing cursor and more results from query object. #432)

6. Comparing Entity values should be easier. Current workaround is

   arya_dict = dict(arya_entity.items())
   self.assertEqual(arya_dict, {'name': 'Arya', 'family': 'Stark'})

7. Paging with offsets should be a bit more automatic, as it is in gcloud-node. Currently we have to

   query = query.offset(offset).limit(limit)
   entities = page_query.fetch()
   # Offset needs to be reset to 0 since we want to start exactly at `cursor`.
   next_query = query.with_cursor(cursor).offset(0)

   (done in Fix #439: split query vs. iterator apart. #487, since offset always defaults to 0)

8. I'm sure there will be plenty more to come after Implementing storage regression tests to match gcloud-node. #319

9. (UPDATED): Consider making bulk-adding properties to Entity more intuitive. See update() override for datastore entities #389. (dict.update() is Pythonic and simple to use / multi-type signature)

10. (UPDATED): "Creating a new datastore.entity with a key name is a pain." See Creating a new datastore.entity with a key name is a pain #82

[dhermes]

Suggestion from @tseaver (paraphrased):

We might extend Query to accomodate a fetch_keys() which handles setting the
projection and then returning only the Keys (instead of the Entitys that were returned)
in the projection='__key__' query.

[elibixby]

I recently wrote https://github.com/GoogleCloudPlatform/gcloud-python-todos/tree/precodereview

I'll try to contribute some bite-sized feedback here =)

The first thing is I'd like to be able to send json data to the datastore without having to learn a new class. In the case of:

d = dataset()
key = Key.from_path('todo','root','todo',id)
data = json.loads(request.data)
#do stuff with data
entity.from_key(key, dataset=d)
entity.update(data)
entity.save()

I would rather just call

data = json.loads(request.data)
#do stuff with data
d.put(('todo','root','todo','id), data)

[dhermes]

@elibixby This was somewhat part of the discussion in #410.

I think a happy middle ground would be:

data = json.loads(request.data)
# do stuff
entity = datastore.Entity(key=('todo', 'root', 'todo', 'id), data=data)
entity.save()

This somewhat fits behind the discussion we previously had (with @elibixby's previous account) in #396 which seems to have fallen by the wayside.

---

In general, sophisticated applications would likely prefer an ORM, so this is kind of moot in some sense.

I'd prefer to have a high-quality mapping to the API and then begin a process of making ndb work on top of it. I tried to articulate this in a previous discussion.

This also means making a "getting started" app would be for people that build libraries rather than people that use libraries. It now occurs to me that @proppy did this with the googledatastore library and I'm curious if that is being actively maintained.

If it is, we need to coordinate and take the mix of three (datastore package here, googledatastore and ndb) and find a path to make it two.

[dhermes]

Asking users to carry around untyped data and keep track of what it all means is a tall task to do well. By providing an Entity class which is not only a data container but also has methods for interacting with the API (save, reload, delete) we provide a consistent way to do easy things and to do complex things.

Jack's talk is good (I just re-watched it); adding complexity is not always a good thing. But here the class allows for doing simple things easily (though as noted above, it could be easier) as well as migrating nicely from a simple "hello world" use case to more complex ones.

(FWIW I'm not the creator of this class and am open to other approaches, though the codebase is fairly mature and throwing it all away seems the wrong move.)

[elibixby]

Totally agree about the two library thing. I think the point I'm trying to make is that those two libraries should be far apart in usage in order to best support the user-base.

I understood gcloud-python as a library facing developers of applications, rather than developers of libraries. If we were two develop two libraries on-top of gcloud-python, one which is more or less classless and one which is a full-blown ORM that would make more sense to me.

[elibixby]

One option that would be not totally classless but not use entity is to keep a Key class around but not the entity.

so something like

dataset.put(Key('todo','root','todo',id), data)

EDIT: fundamentally it doesn't feel like there is much of an internal state, or immutable objecty-ness tied up in the entity class. Keys are a bit more complex and have some formatting defined on them. Sorry I'm not being so helpful/constructive. Having played with the library a bit, it's harder than I expected to make my frustrations concrete.

[dhermes]

@elibixby It's clear your preference is a simple interface that doesn't require much other than the data itself (which must adhere to simple proto types like int, string, bytes, etc.).

It's not clear to me that this is a general thing that people want or expect and I hesitate to make such a large decision based on such a small sample size.

ISTM that by just carrying around the basic types, we provide no way to iterate on or update data through the lifetime of an application. Instead, that burden falls completely on our users, which doesn't seem like the right answer.

[elibixby]

I understand that perspective. I think in that case, that Entity having a mutable key breaks the illusion of consistency. Although it would be a terrible practice a user could theoretically change an entity object into a completely different entity object by saying:

entity = dataset.Entity(key=old_key, data)
entity.key(new_key)
entity.update()
#confusion ensues

This ties back in with your comment about idiomatic libraries not just encouraging good practice, but enforcing it.

[dhermes]

Yes the immutability of an Entity key was posed (by the original author and @proppy) in #3, one of the earliest questions, and is still unanswered. You may be right that it should return a new Entity just like most other setters do.

Or instead we should disallow changing the key ever and enable copying data over to a new Entity in a simple way.

[elibixby]

It does seem like there are no good options there ='(

[proppy]

I think the analysis about the layering of library is spot-on, I can think of the following level:
0/ lowest level API that only expose a dataset type, with one method per RPC call that takes proto, ideally generated
1/ low level API that factor those methods into python idioms, works well with basic type and is flexible and non intrusive with existing codebase
2/ dynamic model API similar to the current Entity class or NDB expando model, doesn't enforce any schema but provide base object that you subclass when targeting datastore first.
3/ a full blown ORM, with user defined property schema on Model like NDB

that burden falls completely on our users, which doesn't seem like the right answer.

I think it's also worth noting that some existing application might already have some existing data model for their app, with its own hierarchy of classes. And that this model is very likely to already have a way to serialized itself to a dict, should they need to send it over somewhere else in JSON.

Asking application to subclass Entity in order to store anything in the datastore is intrusive for those kind of application. And creating the Entity object on the fly seems noisy compared to just pass a dict variable around.

/cc @pcostell

[elibixby]

Would it be too abnormal to build immutability into the Key class except in the case of a onetime transition from partial key to full key?

entity = dataset.entity(Key('a','partial','key'), data)
entity.key.name = 'name'
entity.key.name = 'blah'
#fails

Just a thought, there is probably much more idiomatic syntax out there for it. But making keys immutable seems to make a lot of sense except in this one case.

[dhermes]

There's no doubt that keys and the other types need a clearer contract to enable valid __eq__ methods to be defined (discussed in #293) or even better for == to "just work" without having to define anything.

I think non-idempotent operations like

entity.key.name = 'name'
entity.key.name = 'blah'

surprise and (don't) amuse the user, but maybe that can lead to somewhere useful.

[elibixby]

The presence of _dataset_key is one of the major reasons I was confused as to why you need to attach a key to an entity, and an entity to a dataset, and following that, why you need to have an entity at all.

Either a Key is really just a wrapper for a path, or it is all of the information needed to locate an object in the datastore, and includes dataset_id and namespace. I lean towards the latter, which is also what pushes me towards a put(Key, dict) syntax.

[dhermes]

Closing this out as all have been addressed. Last (not so important) placed in #533