This repository was archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
ObservableDeferred
's API is a mess #11390
Copy link
Copy link
Open
Labels
P3(OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patches(OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patchesT-TaskRefactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Description
... 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 underlyingDeferred
, 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 itscallback
/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, calledAbstractObservableDeferred
, which just exposes theobserve
method:class AbstractObservableDeferred(Generic[T], metaclass=abc.ABCMeta): @abc.abstractmethod def observe(self) -> "Deferred[T]": ...
- Replace consumers (rather than creators) of
ObservableDeferred
withAbstractObservableDeferred
- Create a new class
SimpleObservableDeferred
which implemententsAbstractObservableDeferred
, and just has acallback
, anerrback
, and anobserve
method. Both methods should returnNone
. - Replace uses of
ObservableDeferred
which currently create a brand newDeferred
withSimpleObservableDeferred
- Create a new class
DeferredObserver
which wraps aSimpleObservableDeferred
: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 withDeferredObserver
- Remove now-redundant
ObservableDeferred
Metadata
Metadata
Assignees
Labels
P3(OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patches(OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patchesT-TaskRefactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.