Repository navigation
Feels like we've added a bit more complexity to the datastore API, comments? #647
Description
Hi guys,
I was reviewing the "state of the world" of the datastore API, and found a few things that seem (to me at least) a bit more complicated than they should be.
A bit of this comes from the authentication stuff (which we have another open bug for) but I wanted to bring up a few things for discussion... If this issue gets out of control, we can move the discussion elsewhere, but I wanted to kick things off:
Create a new Person entity in the Datastore
What I have to write
from gcloud import datastore
datastore.set_defaults()
entity = datastore.Entity(key=datastore.Key('Person'))
entity.update({'name': 'Jimmy'})
datastore.put([entity])What I'd like to write
from gcloud import datastore
entity = datastore.Entity('Person')
entity.update({'name': 'Jimmy'})
entity.put()Key differences
- no
set_defaults()(there's another issue open for this I believe) - Create an entity of kind Person without lots more typing (or maybe we get the entity with
datastore.Kind('Person').get_entity()?) .putnot requiring a list for a single entity
Get an existing Person (123) from the Datastore
What I have to write
from gcloud import datastore
datastore.set_defaults()
entity = datastore.get(datastore.Key('Person', 123))What I'd like to write
from gcloud import datastore
datastore.set_defaults()
entity = datastore.Kind('Person').get_by_id(123)Key differences
- No
set_defaults - Equivalent of "drilling down" into a "table" (kind), and then a "row" (id)
(Not hugely concerned about this use case, but wanted to toss it out)
Create a new Person using a specific set of credentials (/home/creds.json) to talk to a specific dataset ('my-dataset') in the Datastore
(Being looked into in PR #641)
What I have to write
(This is a best guess... I don't know if this code is even right... And even so, this code won't do the same thing if you ran it on App Engine because of the order of evaluation here: http://googlecloudplatform.github.io/gcloud-python/latest/gcloud-api.html#gcloud.credentials.get_credentials )
import sys
sys.env['GOOGLE_APPLICATION_CREDENTIALS'] = '/home/creds.json'
from gcloud import datastore
datastore.set_defaults()
entity = datastore.Entity(key=datastore.Key('Person', dataset_id='my-dataset'))
entity.update{'name': 'Jimmy'})
datastore.put(entity)What I'd like to write
from gcloud import datastore
datastore.set_credentials('/home/creds.json')
dataset = datastore.Dataset('my-dataset')
entity = dataset.Entity('Person')
entity.update({'name': 'Jimmy'})
entity.put()Key differences
- Simple in-code way to set which credentials file to use
- High-level concept of a dataset
- Datasets can provide Entities just like
datastorewould
Activity
dhermes commentedon Feb 15, 2015
@jgeewax We spent a lot of work to get rid of the high-level concept of dataset. The philosophy was that we should make the objects and their constructors easier to use and that "learning" another class was overkill when the dataset was a) just a string and b) something that was a single-valued constant for most users.
jgeewax commentedon Feb 15, 2015
That's going to be a really big problem when Datastore exposes multiple datasets per project...
elibixby commentedon Feb 15, 2015
datasetmodule, which isapi. I think it's better to construct adatasetfromdatastoreand leavedatastorefor meta-methods likeset_defaults()now.Entity. It inevitably ends up moving towards ORM which inevitably makes the library more and more heavyweight, and duplicates use-cases from NDB and DB. If anything I think the easiest way forward is to simplify entity. The most lightweight syntax I can think of is:SETUP
Put a single entity
Put a bunch of entities
Get an entity
A couple comments about "what I'd like to write":
entity.put()is confusing. Where is the entity going? Is put really functionality of an entity, or functionality of a dataset on an entity and key. Why ispreferable to
I am completely boggled by this.
datastore.Kind('kind').getById(123)is really really scary. We shouldn't be encouraging people to think of this as relational database. It isn't. We also shouldn't be encouraging them to make 'kind' queries when they know the id, not only is it confusing, it's inefficient.Finally, as an aside, I submitted a PR to remove the list wrapping requirement for single arguments #623 , but if you want to support python 27 and 26 then you need to use
**kwargsand a parser rather than explicit named arguments. Dunno if it's worth it. Python 3 supportsdef func(arg, *args, kw=value, **kwargs)syntax.elibixby commentedon Feb 15, 2015
@jgeewax just wanted to add that I agree with the premise of this issue. I'm writing the samples for the landing page as we speak, and it's awkward to say the least.
jgeewax commentedon Feb 15, 2015
Re "why
.put()"It's similar to the
ext.dbAPI. This is a matter of preference. See https://cloud.google.com/appengine/docs/python/datastore/modelclass#Model_putRe "why
.get_by_id()"It's a helper-method for ultimately constructing a key which reads more clearly (to me). It is also similar to
ext.dbwhich provides aget_by_idmethod. Again, this is a matter of preference.Re the comment about querying
Nowhere here are we talking about relational data.
get_by_idis a shortcut forget(Key(<kind already known>, id)). See https://cloud.google.com/appengine/docs/python/datastore/modelclass#Model_get_by_idI don't see a query here, nor do I see anything related to efficiency. The code I'm talking about would likely look like...
elibixby commentedon Feb 15, 2015
I thought the goal was to move away from the DB implementations, not take queues from their implementation. If the goal is to cover different use cases the syntax is going to be different.
RE:
kind(foo).get_by_id(bar)I mistakenly assumed from your commentthat this was how you meant it, and then it would be implemented by pulling the entities of a certain kind and searching for the right id.
EDIT: If you really wanted to add extra sugar on top of key, you could do something like
jgeewax commentedon Feb 15, 2015
I don't recall ever saying or hearing "we're going to avoid everything done in
ext.db"...?I think the
.put()style on an Entity is a nice thing. I don't have to know anything about the connection, dataset, or authentication -- and.put()will just "save the entity the way it was retrieved". This might seem obvious now, but when multiple datasets are a thing, and different "connections" are floating around, it might be important. It means an Entity on it's own can be enough to persist data.I could be wrong here. Either way, I don't think we should take a hard-line stance of "no stuff that was in
ext.db"...elibixby commentedon Feb 15, 2015
I think that is going to be more of a trouble than a convenience TBH. To quote Zen of Python
explicit is better than implicit.Before the major restructuring
Entityhad all the necessary information (dataset, key, etc) to have all this functionality on it's own, and usage was much more cumbersome IMO. It was the point where I would rather just have used NDB, since it felt like I was being forced to do all the ORM stuff anyway.dhermes commentedon Mar 9, 2015
@jgeewax Can we close this out?
The core issues were
Both of those have been implemented.
Anything remaining can be broken off into a smaller more digestible bug for discussion?
jgeewax commentedon Mar 11, 2015
SGTM
docs: added google.api.field_behavior for some fields in audio_config (…
bug: updates checks for pandas 2.0 to skip certain tests (#647)
bug: updates checks for pandas 2.0 to skip certain tests (#647)
chore: release 0.34.4 (#647)
chore(deps): update all dependencies (#647)
doc: fix Sphinx build for new version of Sphinx (#648)
chore(deps): update all dependencies (#647)
chore: fix typo (#647)
chore(deps): update dependency pytest to v6.2.4 (#647)
fix: remove error msg check from test_decimal_precision_limit for non…
chore(deps): update dependency importlib-metadata to v8.7.0 (#647)
docs: migrate code from googleapis/python-bigquery (#18303)