Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

ObservableDeferred's API is a mess #11390

@richvdh

Description

@richvdh

... and I hate it.

In particular:

  • it doesn't consumeErrors by default, which in 99% of cases leads to log spam about unhandled failures
  • it does some awful __getattr__ / __setattr__ hijinks to "transparently" pass through the methods on the underlying Deferred, which makes it hard to type correctly and generally leads to magical behaviour.
  • I find it weird that you can fire an ObservableDeferred either by directly calling its callback/errback methods, or by calling them on the deferred you pass into it.

I would like to replace it with a much simpler implementation. Here is a proposal for how we can do it without rewriting everything at once.

  • Define an abstract base class which ObservableDeferred implements, called AbstractObservableDeferred, which just exposes the observe method:
    class AbstractObservableDeferred(Generic[T], metaclass=abc.ABCMeta):
        @abc.abstractmethod
        def observe(self) -> "Deferred[T]": ...
  • Replace consumers (rather than creators) of ObservableDeferred with AbstractObservableDeferred
  • Create a new class SimpleObservableDeferred which implementents AbstractObservableDeferred, and just has a callback, an errback, and an observe method. Both methods should return None.
  • Replace uses of ObservableDeferred which currently create a brand new Deferred with SimpleObservableDeferred
  • Create a new class DeferredObserver which wraps a SimpleObservableDeferred:
    class DeferredObserver(AbstractObservableDeferred[T]):
        def __init__(self): 
            self._observable = SimpleObservableDeferred[T]()
       def observe(self) -> Deferred[T]:
            return self._observable.observe()
       def chain_to_deferred(self, deferred: Deferred[T]) -> None:
            deferred.addCallbacks(self._observable.callback, self._observable.errback)
  • Replace uses of ObservableDeferred which chain to an existing Deferred with DeferredObserver
  • Remove now-redundant ObservableDeferred

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3(OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patchesT-TaskRefactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions