From 326b3dace77aeb36e516ea9b04ba1baa171bcb47 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 30 Oct 2019 11:35:46 +0000 Subject: [PATCH 1/3] Make ObservableDeferred.observe() always return deferred. This makes it easier to use in an async/await world. Also fixes a bug where cache descriptors would occaisonally return a raw value rather than a deferred. --- synapse/util/async_helpers.py | 7 ++----- tests/storage/test__base.py | 2 +- tests/util/caches/test_descriptors.py | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index 7659eaeb4234..fd75ba27adad 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -86,11 +86,8 @@ def errback(f): deferred.addCallbacks(callback, errback) - def observe(self): + def observe(self) -> defer.Deferred: """Observe the underlying deferred. - - Can return either a deferred if the underlying deferred is still pending - (or has failed), or the actual value. Callers may need to use maybeDeferred. """ if not self._result: d = defer.Deferred() @@ -105,7 +102,7 @@ def remove(r): return d else: success, res = self._result - return res if success else defer.fail(res) + return defer.succeed(res) if success else defer.fail(res) def observers(self): return self._observers diff --git a/tests/storage/test__base.py b/tests/storage/test__base.py index dd49a1452418..9b81b536f546 100644 --- a/tests/storage/test__base.py +++ b/tests/storage/test__base.py @@ -197,7 +197,7 @@ def func(self, key): a.func.prefill(("foo",), ObservableDeferred(d)) - self.assertEquals(a.func("foo"), d.result) + self.assertEquals(a.func("foo").result, d.result) self.assertEquals(callcount[0], 0) @defer.inlineCallbacks diff --git a/tests/util/caches/test_descriptors.py b/tests/util/caches/test_descriptors.py index f907903511d0..39e360fe2400 100644 --- a/tests/util/caches/test_descriptors.py +++ b/tests/util/caches/test_descriptors.py @@ -310,14 +310,14 @@ def fn(self, arg1, arg2): obj.mock.return_value = ["spam", "eggs"] r = obj.fn(1, 2) - self.assertEqual(r, ["spam", "eggs"]) + self.assertEqual(r.result, ["spam", "eggs"]) obj.mock.assert_called_once_with(1, 2) obj.mock.reset_mock() # a call with different params should call the mock again obj.mock.return_value = ["chips"] r = obj.fn(1, 3) - self.assertEqual(r, ["chips"]) + self.assertEqual(r.result, ["chips"]) obj.mock.assert_called_once_with(1, 3) obj.mock.reset_mock() From 1de28183cb92a2967d527c175123251514f58e69 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 30 Oct 2019 11:37:56 +0000 Subject: [PATCH 2/3] Newsfile --- changelog.d/6291.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6291.misc diff --git a/changelog.d/6291.misc b/changelog.d/6291.misc new file mode 100644 index 000000000000..7b1bb4b6794f --- /dev/null +++ b/changelog.d/6291.misc @@ -0,0 +1 @@ +Change cache descriptors to always return deferreds. From 6e677403b7c71511374b1532b56e0c411840f87b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 30 Oct 2019 11:52:04 +0000 Subject: [PATCH 3/3] Clarify docstring --- synapse/util/async_helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index fd75ba27adad..b60a604474bd 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -88,6 +88,10 @@ def errback(f): def observe(self) -> defer.Deferred: """Observe the underlying deferred. + + This returns a brand new deferred that is resolved when the underlying + deferred is resolved. Interacting with the returned deferred does not + effect the underdlying deferred. """ if not self._result: d = defer.Deferred()