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

Commit

Permalink
Simplification to Keyring.wait_for_previous_lookups. (#5250)
Browse files Browse the repository at this point in the history
The list of server names was redundant, since it was equivalent to the keys on
the server_to_deferred map. This reduces the number of large lists being passed
around, and has the benefit of deduplicating the entries in `wait_on`.
  • Loading branch information
richvdh authored May 24, 2019
1 parent dd64b9d commit fa1b293
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog.d/5250.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Simplification to Keyring.wait_for_previous_lookups.
11 changes: 4 additions & 7 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,7 @@ def _start_key_lookups(self, verify_requests):

# We want to wait for any previous lookups to complete before
# proceeding.
yield self.wait_for_previous_lookups(
[rq.server_name for rq in verify_requests], server_to_deferred
)
yield self.wait_for_previous_lookups(server_to_deferred)

# Actually start fetching keys.
self._get_server_verify_keys(verify_requests)
Expand Down Expand Up @@ -215,12 +213,11 @@ def remove_deferreds(res, verify_request):
logger.exception("Error starting key lookups")

@defer.inlineCallbacks
def wait_for_previous_lookups(self, server_names, server_to_deferred):
def wait_for_previous_lookups(self, server_to_deferred):
"""Waits for any previous key lookups for the given servers to finish.
Args:
server_names (list): list of server_names we want to lookup
server_to_deferred (dict): server_name to deferred which gets
server_to_deferred (dict[str, Deferred]): server_name to deferred which gets
resolved once we've finished looking up keys for that server.
The Deferreds should be regular twisted ones which call their
callbacks with no logcontext.
Expand All @@ -233,7 +230,7 @@ def wait_for_previous_lookups(self, server_names, server_to_deferred):
while True:
wait_on = [
(server_name, self.key_downloads[server_name])
for server_name in server_names
for server_name in server_to_deferred.keys()
if server_name in self.key_downloads
]
if not wait_on:
Expand Down
4 changes: 2 additions & 2 deletions tests/crypto/test_keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test_wait_for_previous_lookups(self):
# we run the lookup in a logcontext so that the patched inlineCallbacks can check
# it is doing the right thing with logcontexts.
wait_1_deferred = run_in_context(
kr.wait_for_previous_lookups, ["server1"], {"server1": lookup_1_deferred}
kr.wait_for_previous_lookups, {"server1": lookup_1_deferred}
)

# there were no previous lookups, so the deferred should be ready
Expand All @@ -94,7 +94,7 @@ def test_wait_for_previous_lookups(self):
# set off another wait. It should block because the first lookup
# hasn't yet completed.
wait_2_deferred = run_in_context(
kr.wait_for_previous_lookups, ["server1"], {"server1": lookup_2_deferred}
kr.wait_for_previous_lookups, {"server1": lookup_2_deferred}
)

self.assertFalse(wait_2_deferred.called)
Expand Down

0 comments on commit fa1b293

Please sign in to comment.