googlecloudplatform / datastore-ndb-python Goto Github PK

PolyModel is a notably missing feature that is in old db but not in ndb.

If you have a cached entity and mutate its key, subsequent retrievals will 
retrieve the entity even though its key no longer matches. E.g.

e = Employee(key=Key(Employee, 'joe'))
e.put()
e.key = Key(Employee, 'fred')
f = Key(Employee, 'joe').get()
# Now f is e; f.key == Key(Employee, 'fred')

What steps will reproduce the problem?
1. Create query with IN() filter and pass set() to the filter:

Article.query().filter(Article.name.IN(set('name1', 'name2')))

What is the expected output? What do you see instead?
raised BadArgumentError(Expected list or tuple, got set('name1', 'name2'))

What version of the product are you using? On what operating system?
0.6 from PYPI.

Please provide any additional information below.

Or at least make 'Rollback' available for convenience. It's annoying to have to 
import two things just to have the common set of exceptions.

from ndb import model

class Test(model.Model):
    test = model.BlobProperty()

data = 'X5\xfes4\xc4=w\x83G\xc68G\xf1\xa2\xe6'
q = Test.query(Test.test == data)
q.get()

WARNING:root:initial generator run_to_queue(query.py:707) raised 
UnicodeDecodeError('ascii' codec can't decode byte 0xfe in position 2: ordinal 
not in range(128))
Traceback (most recent call last):
  File "/Users/guido/appengine-ndb-experiment/ndb/tasklets.py", line 284, in _help_tasklet_along
    value = gen.send(val)
  File "/Users/guido/appengine-ndb-experiment/ndb/query.py", line 717, in run_to_queue
    dsquery = self._get_query(conn)
  File "/Users/guido/appengine-ndb-experiment/ndb/query.py", line 696, in _get_query
    filters = filters._to_filter(bindings)
  File "/Users/guido/appengine-ndb-experiment/ndb/query.py", line 399, in _to_filter
    self.__opsymbol, value)
  File "/usr/local/google_appengine/google/appengine/datastore/datastore_query.py", line 107, in make_filter
    properties = datastore_types.ToPropertyPb(name, values)
  File "/usr/local/google_appengine/google/appengine/api/datastore_types.py", line 1672, in ToPropertyPb
    pbvalue = pack_prop(name, v, pb.mutable_value())
  File "/usr/local/google_appengine/google/appengine/api/datastore_types.py", line 1485, in PackString
    pbvalue.set_stringvalue(unicode(value).encode('utf-8'))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xfe in position 2: ordinal 
not in range(128)

I'm encountering an error with the following code *only on production servers* 
it works fine on the SDK. Using default code branch up to date as of 10 minutes 
ago. Please see attached file for tracebacks.

    import binascii
    from ndb import model

    class Test(model.Model):
        test = model.BlobProperty(indexed=True)

    data = binascii.unhexlify("5835FE7334C43D778347C63847F1A2E6")
    t = Test(test=data)
    t.put()
    # Remember: eventually consistent queries...
    q = Test.query(Test.test == data).get()

We just had a user complain about an app [not using NDB] whose index space was 
10x the data space use; the cause was Expandos with 100s of dynamic properties 
and always growing.

Namespace support appears incomplete.

Hello,

I think the context module has changed.

For example: 
To get the default context, we need to use the module "tasklet" with 
tasklet.get_context()

In the doc section "Context" i read: "context.get_default_context() -> Context 
object"

I don't think NDB currently works right when a non-trivial MultiRpc (i.e. one 
with 2 or more sub-rpcs) is generated by async_get() or friends.

It would be cool if an ancestor query could be specified using filter notation, 
e.g.

Greeting.query(greeting.ancestor == some_key)

instead of

Greeting.query(ancestor=some_key)

It would seem that this is more verbose, but the advantage is that it can be 
used as part of an OR expression.

(Though Alfred has a different syntax suggestion in the context of GQL.)

Future App Engine Python runtimes may support multiple threads handling 
concurrent requests.  NDB should work well in such an environment.  There are 
currently a few places where a single-threaded environment is assumed.

Noah McIlraith wrote: "I'm finding Appstats really difficult to use with NDB. 
The call traces are a lot less useful then they were when using the old DB 
module. I don't even know what kind of entities queries are for."

I think it should be possible to modify ndb so that this works:
ModelName.query(ModelName.key <= key_obj). It would require making
'key' into some kind of special Property subclass that maps the vallue
to the entity's key and gives the name as '__key__'.

Transactional add() from the taskqueue API is broken with NDB. Here is a 
workaround (though untested):

from google.appengine.api import datastore
save = datastore._GetConnection()
try:
  datastore._SetConnection(tasklets.get_context()._conn)
  <put your add() call here>
finally:
  datastore._SetConnection(save)

But this should be done by NDB when creating a transaction.

These are new in release 0.2.1 but not yet documented.

If you do str(entity) and the model contains a DateTimeProperty, you get:

AttributeError: type object 'DateTimeProperty' has no attribute '_auto_now'

Must check all properties for the missing explicit attributes which may cause 
problems with __repr__. '_auto_now' and '_auto_now_add' in the datetime related 
properties are two missing ones.

Right now it only takes keys. If you pass in a model you get this error:

AttributeError: 'MyKind' object has no attribute 'pairs'

For convenience, all public methods that accept a Key should also accept a 
Model instance as alternative.

Here's a normalization function that could be used:

def normalize_key(key_or_model):
  """Convenience function to normalize a key or model instance to a key."""
  if key_or_model is not None:
    if isinstance(key_or_model, Key):
      return key_or_model
    if isinstance(key_or_model, Model):
      key = key_or_model.key
      if key is None:
        raise datastore_errors.BadValueError('%s instance must have a '
                                             'complete key' %
                                             (key_or_model.GetKind(),))
      return key
    raise datastore_errors.BadValueError('Expected Key or Model instance, '
                                         'got %r' % (key_or_model,))

Following the TODO notes, Model.SetAttributes is set to be renamed 
_set_attributes and become a private method. It will be a good idea to keep it 
public instead, as a convenience method to fill an existing entity with data 
when doing updates. It could be called populate(), following a common name used 
by ORM and form libraries. 

Here's a usage example compared to the alternatives:

data = {
  'name': 'foo',
  'city': 'bar',
  'active': True,
}
ent = MyModel.get_by_id(id)
ent.populate(**data)

vs.

ent = MyModel.get_by_id(id)
ent.name = 'foo'
ent.city = 'bar'
ent.active = True

vs.

data = {
  'name': 'foo',
  'city': 'bar',
  'active': True,
}
ent = MyModel.get_by_id(id)
for name, value in data.iteritems():
  setattr(ent, name, value)

I've noticed that my structured properties include both the Kind name and the 
corresponding App ID in their encoded format. That seems unnecessary to me and 
a waste of space in serialization. The whole Key technically should be missing, 
since the LocalStructuredProperty already knows the kind of the entity that was 
encoded.

My sensitivity comes from the fact that the proto encoding is actually kinda 
physically big in bytes, more so than JSON would be for these cases. So I'm 
trying to see if we can get closer to the size of JSON.

The two classes deriving from threading.local should just derive from object 
until we actually have a threadsafe runtime...

This is slightly tricky because it requires reversing both the cursor and the 
orders on the query. So what should the API look like?

There is no API to get a cursor out of an iterator.

Retrieving id or name from a key is too commonly used so Key should provide a 
shortcut for it. My clone has an implementation for Key.id(), Key.name() and 
Key.id_or_name():

http://code.google.com/r/rodrigomoraes-ndb/source/diff?spec=svnc751a0455bf6c7ccd
e92de07c6fdeead9fdf7404&r=b56a5919bac4d28406367dc7fb9ba5b5b23c8979&format=side&p
ath=/ndb/key.py&old_path=/ndb/key.py&old=f2769eb8aca50e024f11ff51193c987762c1f55
e

I am relying on overriding _from_pb() method to be able to check if instance 
have properties with changed values: 
http://groups.google.com/group/appengine-ndb-discuss/browse_thread/thread/1b00cb
8dce9d4560

But I found that NDB prefer to return cached entity instead of entity, that I 
just queried. Code that does this:

            if key in self._cache:
              # Assume the cache is more up to date.
              if self._cache[key] is None:
                # This is a weird case.  Apparently this entity was
                # deleted concurrently with the query.  Let's just
                # pretend the delete happened first.
                logging.info('Conflict: entity %s was deleted', key)
                continue
              # Replace the entity the callback will see with the one
              # from the cache.
              if ent != self._cache[key]:
                logging.info('Conflict: entity %s was modified', key)
              ent = self._cache[key]

Can you provide options to disable this behavior without completely disabling 
caching for model?

We need an API to control the memcache timeout for each key written.

Apologies for the names here, I encountered this bug working on a different 
structure and tried to reproduce it using the simplest possible case:

class TestModel(model.Model):
    first_name = model.StringProperty()
    last_name = model.StringProperty()

class TestNestedProperty(model.Model):
    person = model.StructuredProperty(TestModel)
    phone = model.StringProperty()

class TestRepeatedProperty(model.Model):
    test = model.StructuredProperty(TestNestedProperty, repeated=True)

...

        ctx = tasklets.get_context() 
        ctx.set_cache_policy(False)
        ctx.set_memcache_policy(False)

        repeat = TestRepeatedProperty.get_or_insert('test')
        person = TestModel(first_name="John", last_name='Smith')
        phone = TestNestedProperty(person=person, phone='phone')
        repeat.test.append(phone)
        repeat.put()

        repeat = TestRepeatedProperty.get_by_id('test')

        print
        print repeat.test

With the in process and memcache turned off, I get the following output:

[TestNestedProperty(person=TestProperty(first_name=u'John'), phone=u'phone'), 
TestNestedProperty(person=TestProperty(last_name=u'Smith'))]

The Datastore view only shows one item in each list for test.person.first_name, 
test.person.last_name, test.phone

I would expect to see:
[TestNestedProperty(person=TestProperty(first_name='John', last_name='Smith'), 
phone='phone')]

This happens on both devserver and live environment

Switching the nested SP to a LocalStructuredProperty fixes this problem

I'd like to be able to set a KeyProperty's value to None in the model's 
__init__ constructor to indicate that the reference should not be saved. 
Obviously this should only work for optional fields.

Using pdb with tasklets is pretty horrid. The main problem is that you can't 
step over a yield -- the pdb command 'next' returns to the eventloop logic, 
which you have to trace all the way until the generator is resumed, much later.

To match old db / datastore.py.

See http://code.google.com/p/googleappengine/issues/detail?id=4883

(The reason is that the production logging handler keeps *all* logging output, 
regardless of what logging level is configured.)

It is possible to add hooks to PUT operations by overriding put_async() for a 
specific Model subclass. But this is not so easy for DELETE (or GET) 
operations. I propose adding a factory function to the Model class which is 
called to create a Key instance, so this can be overridden to substitute a Key 
subclass with a hook on delete_async() or even get_async().

See http://code.google.com/p/googleappengine/issues/detail?id=322

In ext/db, there are public APIs that render entities as XML. These internally 
use serialization. The serialization has (or had, until the bug got fixed) as a 
side-effect that the auto_now and auto_now_add properties were updated.

In NDB, _to_pb() has the same side-effect. We might consider fixing this. 
Although the question then becomes, when *do* we update auto_now* properties? I 
suppose when _put_async() is called.

I'm just leaving this as a note, there's no immediate action needed.

Both get_or_insert() and allocate_ids() are broken. Here are the fixes with 
some more tests.

Model.get_or_insert():

http://code.google.com/r/rodrigomoraes-ndb/source/detail?r=38955da52ea56eeedaa8d
5ba76a2bf1e640a7f47&path=/ndb/model.py

Model.allocate_ids():

http://code.google.com/r/rodrigomoraes-ndb/source/detail?r=6f6493a198b8734d49368
6a018e8b40a265234fe&path=/ndb/model.py

When writing an entity, there are failure modes that cause memcache to be 
inconsistent with the datastore. We should minimize these.

Possibly LocalStructuredProperty would be faster if it could delay the 
decompress and deserialization until an attribute of the value is actually used.

This might require some kind of lazy access mix-in class that can be 
dynamically mixed in with the given modelclass, or a generic mechanism in Model.

I would like to get_or_insert a model based on the integer_id of a parent model.

class ParentModel(model.Model):
    pass

class ChildModel(model.Model):

    @classmethod
    def get_or_insert_for(cls, parent_key):
        parent_id = int(parent_key.id())
        entity = cls.get_or_insert(parent_id, parent=parent_key)
        return entity

Can the assert at:

     http://code.google.com/p/appengine-ndb-experiment/source/browse/ndb/context.py#573

be safely changed to:

    assert isinstance(name, (basestring, int)) and name

to allow for integer keys?

Once an app is debugged, collecting stack frames and other info in get_stack() 
and code_info() is a waste of time. There should be a way to disable these 
calls.

Especially StructuredProperty doesn't work out of the box.

For more details see 
https://groups.google.com/group/appengine-ndb-discuss/browse_thread/thread/541e2
559bb3eaa42

esp. the solution in https://gist.github.com/1145809

import unittest
from ndb import model
from google.appengine.ext import testbed

class TestModel(model.Model):
    foo = model.StringProperty()

class SavingTest(unittest.TestCase):
    def setUp(self):
       self.testbed = testbed.Testbed()
       self.testbed.activate()
       self.testbed.init_user_stub()
       self.testbed.init_datastore_v3_stub()
       self.testbed.init_memcache_stub()

    def testSaving(self):
        entity = TestModel(foo = 'bar')
        entity.put()
        entity.foo = 'bang!'

        entity_from_db = TestModel.query().get()

        self.assertNotEqual(entity.foo, entity_from_db.foo)

This test fails, though it should pass. It looks like entity_from_db and entity 
are actually sharing the same object ref. I don't know if this is intended 
behavior, but it certainly makes testing if an entity has been saved very 
difficult. 

Basically, MultiQuery is a toy.

See http://paste.pocoo.org/show/383007/ for an example.

BTW the agreed-upon return value would be -> results, cursor, more.

Several exceptions are reported as deadlocks, apparently because the error 
happens in another tasklet. Example:

transaction(lambda: Employee.query().fetch(10))

prints


WARNING:root:initial generator run_to_queue(query.py:718) raised 
BadRequestError(Only ancestor queries are allowed inside a transaction.)
WARNING:root:suspended generator transaction(context.py:376) raised 
RuntimeError(Deadlock waiting for <Future 1481dd0 created by 
tasklet_wrapper(tasklets.py:718) for tasklet 
positional_wrapper(datastore_rpc.py:82); suspended generator 
fetch_async(query.py:907); pending>)
[...traceback...]

RuntimeError: Deadlock waiting for <Future 1481dd0 created by 
tasklet_wrapper(tasklets.py:718) for tasklet 
positional_wrapper(datastore_rpc.py:82); suspended generator 
fetch_async(query.py:907); pending>

The background: my task processes information from another web site and loads 
data into the datastore.  One phase of this ends up inserting many records of 
the same kind into the datastore.

NDB quietly tries to batch all of these together.

The issue is the batch size is unbounded; it seems to collect items together 
until a different object is written.

In my app, the batch ended up being about 17000 objects.

In the end all the objects got inserted, so there is not a "correctness" issue 
here.  But the app did end up taking a huge amount of memory, because it kept 
all those objects around for a long time.

IMHO the incremental value of batching goes way down after 100 or so; there 
should be a default max-batch size (perhaps: the amount that will fit in an RPC 
call) that should be changeable.

class MyModel(Model):
  name = StringProperty()
MyModel.get_or_insert('foo', name='bar')

The get_or_insert() call fails with a TypeError because the positional 'foo' 
argument is also called 'name'. We should rename this to _name and force it to 
be positional. Possibly some other keywords need to be renamed as well, in 
context.py's get_or_insert().

Add Model.delete() and Model.delete_async() as convenience methods that mirror 
ext.db functionality. Here's the implementation:

http://code.google.com/r/rodrigomoraes-ndb/source/browse/ndb/model.py?spec=svnce
4b8c83631a5dec1ab6099663a74a43eba77ed7&r=ce4b8c83631a5dec1ab6099663a74a43eba77ed
7#451

Maybe I'm missing something, but the implementation of Key.kind() seems to be 
doing unnecessary work. Currently it is:

kind = None
for elem in self.__reference.path().element_list():
  kind = elem.type()
return kind

Since a Key always have at least 1 pair, this could be replaced by:

return self.__reference.path().element(-1).type()

Not sure what's the right API for this. Maybe Context methods to get and set 
the Configuration for the underlying Connection? That currently requires using 
a __private variable on the Connection (__config) but we could promote that to 
a public API on the Connection.

Context.get_or_insert() disallows saving an entity with parent app and 
namespace that differ from current app and namespace. Unless there's a strong 
reason for this, this restriction should be removed for consistency as no other 
methods enforce it and it is considered a feature from ext.db (you save 
entities with parent from the "global" namespace, avoiding a namespace switch).

Besides, it becomes much simpler and the same expectation doesn't need to be 
repeated in other methods. See:

http://code.google.com/r/rodrigomoraes-ndb/source/diff?spec=svn7188dce6129a0def6
69c0ead7af512dfcc365d8c&r=7188dce6129a0def669c0ead7af512dfcc365d8c&format=side&p
ath=/ndb/context.py