ioflo / hio

Apache License 2.0
8 stars 8 forks source link

Doize Decorator on Class Instance Methods Missing Attribute `done` #2

Closed rosenbrockc closed 3 years ago

rosenbrockc commented 3 years ago

I have this method on a Doer that tests the DHT client interaction with the server. It adds DHT puts to a deque so the client (that is a different DoDoer) can process them independently (trying to simulate real life as best we can). I include it here in case the contents of the method are doing something wrong.

@doing.doize()
    def test_scheduler(self, tymist=None, tock=0.25, **opts):
        """ Returns Doist compatibile generator method (doer dog) to add new
        DHT put operations to :attr:`puts` so that they will be processed by
        the scheduler and :meth:`client_puts`. Note that once a put request
        completes, the :meth:`result_checker` will then queue the appropriate
        get requests.
        """
        while True and not self._shutting_down.is_set() and self.test_count < self.maxtests:
            # See how many to schedule this round; don't want to do too many or the server
            # and client doers won't get to process any tx/rx.
            process_count = random.randint(1, 2)
            client_ids = random.choices(range(len(self.clients)), k=process_count)

            for i in client_ids:
                key = random_key(self.test_count)
                # Put methods take arguments of (key, payload)
                r = {
                    "client_id": i, 
                    "method": random.choice(TestClient.PUT_METHODS),
                    "args": (key, random_payload()),
                    "key": key
                }

                self.puts.append(r)
                self.test_count += 1

            yield

        return True

Once we are finished with all the tests, we finally hit the return True and I get this error:

            if retyme <= tyme:  # run it now
                try:  # tock == 0.0 means re-run asap
                    tock = dog.send(tyme)  # send tyme. yield tock
                except StopIteration as ex:  # returned instead of yielded
>                   self.doers[index].done = ex.value if ex.value else False  # assign done state
E                   AttributeError: 'method' object has no attribute 'done'

../../../envs/keri/lib/python3.8/site-packages/hio/base/doing.py:833: AttributeError

It seems that despite the doize decorator, the instance method is not getting extra attributes added. My suspicion is that it may have to do with the hard-coded FunctionType here: https://github.com/ioflo/hio/blob/da964ad2f1072e432474ab7fc163bbb783b8b5ae/src/hio/help/helping.py#L24. But I haven't done any real digging.

rosenbrockc commented 3 years ago

A little more background. I added this supposing it would temporarily allow things to continue:

 if hasattr(self.doers[index], "done"):
     self.doers[index].done = ex.value if ex.value else False  # assign done state

And, if I do a dir on the object, the decorator did actually add the done attribute. So here we have an explicit check for the attribute, which passes, and then the error: AttributeError: 'method' object has no attribute 'done'

So, there is something non-trivial going on...

rosenbrockc commented 3 years ago

It seems that despite the decorator, python still views this as a bound method which has explicit restrictions on adding attributes. So the decorator has the appropriate extensions (functions can have attributes), but the underlying bound method is not assignable.

SmithSamuelM commented 3 years ago

Python generator objects (not generator functions which return generator objects) do not support bespoke attributes. So there is no .done attribute on the generator itself, only on the generator function that returns the generator. The way Hio manages this is that Doist and DoDoer instances keep the original list of doers (.doers) each of which is a generator creating instance, function, or method. The index into that list is included in the the deeds deque entry (tuple, generator, tock, index) for each generator. Therefore upon generator (deed) completion that index can be used to find the originating generator function in the doers list and set its .done attribute. This means that one does not check the generator itself, this should be opaque anyway, but checks the orginating generator function's .done, e.g doer.done of the doer that was passed into the Doist or DoDoer. How the associated generator communicates is done status is via its return not via any attribute. The Doist is responsible for grabbing that return and then assigning it to the doer.done when the generator raises StopIteration upon an unforced return.

Note that .done is only set to True if the generator completes on its own. If the generator is forced closed because of the thrown GeneratorExit or some uncaught exception then its done is False. So the purpose of .done is for use with generators that are not meant to run forever but have a fixed finite set of operations that they complete and the .done shows that completion status. A run forever generator will never complete or return on its own so its done state will always be False even after it is exited and is not longer running. Done does not mean running or stopped but means, ran to successful completion and then stopped on its own as a result. I.e. only when running to completion has meaning. So if you stage some finite tasks in a generator, you can check that the reason the generator stopped is because it completed successfully vs. it stopped due to being aborted by an exception or being force closed.

SmithSamuelM commented 3 years ago

Whoops misunderstood your problem. You are applying doize to a bound instance method not a function. Instance methods do not expose the attributes on their wrapped functions. One has to access the dunder method .__func__ instead. This is fixed in hio 0.3.0 (actually earlier but 0.3.0 is the latest. There are some breaking changes in 0.3.0 where instead of injecting tymist reference it just injects a closure around the tymist.tyme.

But I ran into the same problem last week when using doize with a method not a function. The fix was added and is shown here.

doer = self.doers[index]
if ismethod(doer):  # when using bound method for generator function
       doer.__func__.done = False  # forced close
 else:
       doer.done = False  # forced close
SmithSamuelM commented 3 years ago

So it should work fine for you once you update to hio 0.3.0 (and fix the breaking changes =( )

SmithSamuelM commented 3 years ago

See kerypy keri.base.directing.Reactor for example of using instance methods as generator functions with doize.

SmithSamuelM commented 3 years ago

I did some more thorough testing and find on place I missed for checking for bound method creating generator. This is now fixed in version hio 0.3.1 eb9e8ea778a69cd7fce112a28eec7a62531291ab

rosenbrockc commented 3 years ago

Excellent! Thank-you.

SmithSamuelM commented 3 years ago

Just to avoid confusion. Normally when using a class the intent is that the instance of the class is a callable via its .__call__ method and that callable returns a generator. Usually the .__call__ references the .do method. In this case the instance itself has a .done attribute that just works. This is the intended use case. No need to use the '@doize' decorator on its .do method. Its a special case to instead use some other method for returning a generator on an instance that is not its callable method and to apply @doize decorator to that other method. This is the special case that created the bug. This comment is intended to clarify if indeed you were using the special case but intended to do the normal case but misunderstood how to do it.