GoogleCloudPlatform / appengine-python-standard

Google App Engine services SDK for Python 3
Apache License 2.0
70 stars 47 forks source link

NDB transaction() read_only property does not work #114

Open troberti opened 7 months ago

troberti commented 7 months ago

Expected Behavior

The NDB transaction() function has a read_only property to indicate that the transaction will only perform entity reads, potentially improving throughput. I expect that it will start a read only transaction when I invoke it as such: ndb.transaction(txn, read_only=True).

Actual Behavior

Sadly, this flag does not work in the current SDK. Instead, it fails with an TypeError: Unknown configuration option ('read_only') error when you specify it (regardless whether it is True of False).

Full stack trace:

Traceback (most recent call last):
  File "/Users/tijmen/dev/ndb-read-only/readonly_test.py", line 22, in test_readonly_transaction
    ndb.transaction(txn, read_only=False)  # Does not work
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/utils.py", line 182, in positional_wrapper
    return wrapped(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/model.py", line 3902, in transaction
    return fut.get_result()
           ^^^^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/tasklets.py", line 397, in get_result
    self.check_success()
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/tasklets.py", line 394, in check_success
    six.reraise(self._exception.__class__, self._exception, self._traceback)
  File "/Users/tijmen/dev/ndb-read-only/lib/six.py", line 719, in reraise
    raise value
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/tasklets.py", line 444, in _help_tasklet_along
    value = gen.send(val)
            ^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/context.py", line 976, in transaction
    options = _make_ctx_options(ctx_options, TransactionOptions)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/ext/ndb/context.py", line 145, in _make_ctx_options
    return config_cls(**ctx_options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tijmen/dev/ndb-read-only/lib/google/appengine/datastore/datastore_rpc.py", line 392, in __new__
    raise TypeError('Unknown configuration option (%s)' % err)
TypeError: Unknown configuration option ('read_only')

Steps to Reproduce the Problem

Run the following test:

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

class ExampleModel(ndb.Model):
    pass

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

    def tearDown(self):
        self.testbed.deactivate()

    def test_readonly_transaction(self):
        def txn():
            model = ExampleModel.get_by_id("test")
        ndb.transaction(txn, read_only=True)  # Does not work

    def test_normal_transaction(self):
        def txn():
            model = ExampleModel.get_by_id("test")
        ndb.transaction(txn)  # Works.

if __name__ == '__main__':
    unittest.main()

Specifications

Version: appengine-python-standard 1.1.6

troberti commented 7 months ago

The problem is in google/appengine/ext/ndb/context.py:976 where the kwargs **ctx_options is passed to the _make_ctx_options function . This function parses the options but does not accept the read_only keyword argument. This argument is used separately a few lines later to set the correct TransactionMode, but never gets there.

  @tasklets.tasklet
  def transaction(self, callback, **ctx_options):

    options = _make_ctx_options(ctx_options, TransactionOptions)
    propagation = TransactionOptions.propagation(options)
    if propagation is None:
      propagation = TransactionOptions.NESTED

    mode = datastore_rpc.TransactionMode.READ_WRITE
    if ctx_options.get('read_only', False):
      mode = datastore_rpc.TransactionMode.READ_ONLY

Swapping the order around and removing the read_only key from ctx_options fixes the issue:

  @tasklets.tasklet
  def transaction(self, callback, **ctx_options):

    mode = datastore_rpc.TransactionMode.READ_WRITE
    if ctx_options.get('read_only', False):
      mode = datastore_rpc.TransactionMode.READ_ONLY
    ctx_options.pop('read_only')

    options = _make_ctx_options(ctx_options, TransactionOptions)
    propagation = TransactionOptions.propagation(options)
    if propagation is None:
      propagation = TransactionOptions.NESTED