Uploaded image for project: 'Couchbase Python Client Library'
  1. Couchbase Python Client Library
  2. PYCBC-1056

MutateIn broken for all asyncio, twisted

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 3.0.5
    • 3.0.7
    • library
    • None
    • 1
    • SDK48: FTS Score/Incl, Docs.

    Description

      From investigation of CBSE-9011, if you are using Asyncio, and do something like:

      async def mutate_in():
          coll = bucket.default_collection()
          res = coll.mutate_in("foo", {SD.upsert("other", "path"),})
          return await res
      

      you will see:

      Traceback (most recent call last):
        File "asiominimal.py", line 31, in <module>
          rv2 = loop.run_until_complete(mutate_in())
        File "/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete
          return future.result()
        File "asiominimal.py", line 21, in mutate_in
          res = coll.mutate_in("foo", {SD.upsert("other", "path"),})
        File "/Users/davidkelly/projects/gerrit/couchbase-python-client/couchbase/collection.py", line 258, in wrapped
          return func(self, *args, **kwargs)
        File "/Users/davidkelly/projects/gerrit/couchbase-python-client/acouchbase/cluster.py", line 45, in ret
          rv.set_callbacks(on_ok, on_err)
      AttributeError: 'MutateInResult' object has no attribute 'set_callbacks'
      

      Seems that there is no AsyncMutateInResult, or mechanism to map it similar to AsyncMutationResult. One would think that just changing annotation of collection.mutate_in to be @mutate_result_and_inject instead of @inject_scope_and_collection. would maybe do it, but that insists on a MutationResult or AsyncMutationResult.

      Of course, same issue for twisted. If you do somewhat the same thing, you get:

      	--- <exception caught here> ---
      	  File "/usr/local/lib/python3.7/site-packages/twisted/internet/defer.py", line 654, in _runCallbacks
      	    current.result = callback(current.result, *args, **kw)
      	  File "/Users/davidkelly/projects/gerrit/couchbase-python-client/txcouchbase/cluster.py", line 357, in <lambda>
      	    qop.addCallback(lambda x: f(meth, *args, **kwargs))
      	  File "/Users/davidkelly/projects/gerrit/couchbase-python-client/txcouchbase/cluster.py", line 371, in _wrap
      	    return self.defer(opres)
      	  File "/Users/davidkelly/projects/gerrit/couchbase-python-client/txcouchbase/cluster.py", line 279, in defer
      	    opres.set_callbacks(d.callback, _on_err)
      	builtins.AttributeError: 'MutateInResult' object has no attribute 'set_callbacks'
      	
      

      So lets get this correct. Then, perhaps as a second issue, lets sweep through asyncio and twisted apis and insure the very basics (say, every method in collections) are tested. At a minimum.

      Attachments

        For Gerrit Dashboard: PYCBC-1056
        # Subject Branch Project Status CR V

        Activity

          david.kelly David Kelly added a comment -

          Same thing for lookup_in – 

            Traceback (most recent call last):
            File "/Users/davidkelly/projects/gerrit/couchbase-python-client/asiominimal.py", line 38, in <module>
              r2 = loop.run_until_complete(lookup_in())
            File "/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete
              return future.result()
            File "/Users/davidkelly/projects/gerrit/couchbase-python-client/asiominimal.py", line 27, in lookup_in
              res = coll.lookup_in("foo", {SD.get("other"),})
            File "/Users/davidkelly/projects/gerrit/couchbase-python-client/couchbase/collection.py", line 258, in wrapped
              return func(self, *args, **kwargs)
            File "/Users/davidkelly/projects/gerrit/couchbase-python-client/acouchbase/cluster.py", line 45, in ret
              rv.set_callbacks(on_ok, on_err)
          AttributeError: 'LookupInResult' object has no attribute 'set_callbacks'
          
          

          david.kelly David Kelly added a comment - Same thing for lookup_in –  Traceback (most recent call last): File "/Users/davidkelly/projects/gerrit/couchbase-python-client/asiominimal.py", line 38, in <module> r2 = loop.run_until_complete(lookup_in()) File "/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete return future.result() File "/Users/davidkelly/projects/gerrit/couchbase-python-client/asiominimal.py", line 27, in lookup_in res = coll.lookup_in("foo", {SD.get("other"),}) File "/Users/davidkelly/projects/gerrit/couchbase-python-client/couchbase/collection.py", line 258, in wrapped return func(self, *args, **kwargs) File "/Users/davidkelly/projects/gerrit/couchbase-python-client/acouchbase/cluster.py", line 45, in ret rv.set_callbacks(on_ok, on_err) AttributeError: 'LookupInResult' object has no attribute 'set_callbacks'
          david.kelly David Kelly added a comment - - edited

          I'm sure there are others.  We annotate the functions, and require just a bit of plumbing for the async to work.  A partial fix looks something like the below, with similar (but @_get_result_and_inject instead...

          However, there are surely more like this.  Just fix them all, find any outstanding tickets for others, close 'em all.  No need to do this more than once.  

          Also - note that the tests clearly don't test these.  Adding that is super important - however it may not be simple, as the test infrastructure is needlessly complex.  But we need at least basic tests of all this on the async side.  Perhaps something minimal, for now, and we move to some better infrastructure that makes full testing of this simpler, later. 

          diff --cc couchbase/collection.py
          index 1d0aaee5,1d0aaee5..d7d01f39
          --- a/couchbase/collection.py
          +++ b/couchbase/collection.py
          @@@ -962,7 -962,7 +962,7 @@@ class CBCollection(wrapt.ObjectProxy)
                    final_options = forward_args(kwargs, *options)
                    return LookupInResult(CoreClient.lookup_in(self.bucket, key, spec, **final_options))
           
          --    @_inject_scope_and_collection
          ++    @_mutate_result_and_inject
                def mutate_in(self,  # type: CBCollection
                              key,  # type: str
                              spec,  # type: MutateInSpec
          diff --cc couchbase/result.py
          index e0046e76,e0046e76..c31bed66
          --- a/couchbase/result.py
          +++ b/couchbase/result.py
          @@@ -229,6 -229,6 +229,12 @@@ class MutateInResult(MutationResult)
                    """ Original key of the operation """
                    return self._content.key
           
          ++    def get_mutation_result(result  # type: CoreResult
          ++                            ):
          ++        # type (...)->MutationResult
          ++        orig_result = getattr(result, 'orig_result', result)
          ++        factory_class = AsyncMutateInResult if _is_async(orig_result) else MutateInResult
          ++        return factory_class(orig_result)
           
            class PingResult(object):
                @internal
          @@@ -386,6 -386,6 +392,15 @@@ class AsyncMutationResult(AsyncWrapper.
                    super(AsyncMutationResult, self).__init__(core_result)
           
           
          ++class AsyncMutateInResult(AsyncWrapper.gen_wrapper(MutateInResult)):
          ++    def __init__(self,
          ++                 core_result  # type: CoreResult
          ++                 ):
          ++        # type (...)->None
          ++        super(AsyncMutateInResult, self).__init__(core_result)
          ++
          ++
          ++
            # TODO: eliminate the options shortly.  They serve no purpose
            ResultPrecursor = NamedTuple('ResultPrecursor', [('orig_result', CoreResult), ('orig_options', Mapping[str, Any])])
          

          david.kelly David Kelly added a comment - - edited I'm sure there are others.  We annotate the functions, and require just a bit of plumbing for the async to work.  A partial fix looks something like the below, with similar (but @_get_result_and_inject instead... However, there are surely more like this.  Just fix them all, find any outstanding tickets for others, close 'em all.  No need to do this more than once.   Also - note that the tests clearly don't test these.  Adding that is super important - however it may not be simple, as the test infrastructure is needlessly complex.  But we need at least basic tests of all this on the async side.  Perhaps something minimal, for now, and we move to some better infrastructure that makes full testing of this simpler, later.  diff --cc couchbase/collection.py index 1d0aaee5,1d0aaee5..d7d01f39 --- a/couchbase/collection.py +++ b/couchbase/collection.py @@@ -962,7 -962,7 +962,7 @@@ class CBCollection(wrapt.ObjectProxy) final_options = forward_args(kwargs, *options) return LookupInResult(CoreClient.lookup_in(self.bucket, key, spec, **final_options))   -- @_inject_scope_and_collection ++ @_mutate_result_and_inject def mutate_in(self, # type: CBCollection key, # type: str spec, # type: MutateInSpec diff --cc couchbase/result.py index e0046e76,e0046e76..c31bed66 --- a/couchbase/result.py +++ b/couchbase/result.py @@@ -229,6 -229,6 +229,12 @@@ class MutateInResult(MutationResult) """ Original key of the operation """ return self._content.key   ++ def get_mutation_result(result # type: CoreResult ++ ): ++ # type (...)->MutationResult ++ orig_result = getattr(result, 'orig_result', result) ++ factory_class = AsyncMutateInResult if _is_async(orig_result) else MutateInResult ++ return factory_class(orig_result)   class PingResult(object): @internal @@@ -386,6 -386,6 +392,15 @@@ class AsyncMutationResult(AsyncWrapper. super(AsyncMutationResult, self).__init__(core_result)     ++class AsyncMutateInResult(AsyncWrapper.gen_wrapper(MutateInResult)): ++ def __init__(self, ++ core_result # type: CoreResult ++ ): ++ # type (...)->None ++ super(AsyncMutateInResult, self).__init__(core_result) ++ ++ ++ # TODO: eliminate the options shortly. They serve no purpose ResultPrecursor = NamedTuple('ResultPrecursor', [('orig_result', CoreResult), ('orig_options', Mapping[str, Any])])

          People

            david.kelly David Kelly
            david.kelly David Kelly
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty