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

Convert the well known resolver to async #8214

Merged
merged 4 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ files =
synapse/handlers/saml_handler.py,
synapse/handlers/sync.py,
synapse/handlers/ui_auth,
synapse/http/federation/well_known_resolver.py,
synapse/http/server.py,
synapse/http/site.py,
synapse/logging/,
Expand Down
25 changes: 15 additions & 10 deletions synapse/http/federation/well_known_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import logging
import random
import time
from typing import Tuple
from typing import Callable, Dict, Optional, Tuple

import attr

Expand Down Expand Up @@ -125,7 +125,9 @@ async def get_well_known(self, server_name: bytes) -> WellKnownLookupResult:
# requests for the same server in parallel?
try:
with Measure(self._clock, "get_well_known"):
result, cache_period = await self._fetch_well_known(server_name)
result, cache_period = await self._fetch_well_known(
server_name
) # type: Tuple[Optional[bytes], float]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this is Optional[bytes], whereas _fetch_well_known supposedly returns bytes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because result can be set to None below, so without it you get:

synapse/http/federation/well_known_resolver.py:139: error: Incompatible types in assignment (expression has type "None", variable has type "bytes") [assignment]

This isn't saying the type of the return value of _fetch_well_known, but the types of the variables result and cache_period.


except _FetchWellKnownFailure as e:
if prev_result and e.temporary:
Expand Down Expand Up @@ -154,7 +156,7 @@ async def get_well_known(self, server_name: bytes) -> WellKnownLookupResult:

return WellKnownLookupResult(delegated_server=result)

async def _fetch_well_known(self, server_name: bytes) -> Tuple[bytes, int]:
async def _fetch_well_known(self, server_name: bytes) -> Tuple[bytes, float]:
"""Actually fetch and parse a .well-known, without checking the cache

Args:
Expand Down Expand Up @@ -268,18 +270,21 @@ async def _make_well_known_request(
await self._clock.sleep(0.5)


def _cache_period_from_headers(headers, time_now=time.time):
def _cache_period_from_headers(
headers: Headers, time_now: Callable[[], float] = time.time
) -> Optional[float]:
cache_controls = _parse_cache_control(headers)

if b"no-store" in cache_controls:
return 0

if b"max-age" in cache_controls:
try:
max_age = int(cache_controls[b"max-age"])
return max_age
except ValueError:
pass
max_age = cache_controls[b"max-age"]
if max_age:
Comment on lines +282 to +283
Copy link
Member Author

@clokep clokep Aug 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra check was to make mypy happy that we weren't doing int(None). This case was actually not being handled properly anyway since it throws a TypeError, not a ValueError.

try:
return int(max_age)
except ValueError:
pass

expires = headers.getRawHeaders(b"expires")
if expires is not None:
Expand All @@ -295,7 +300,7 @@ def _cache_period_from_headers(headers, time_now=time.time):
return None


def _parse_cache_control(headers):
def _parse_cache_control(headers: Headers) -> Dict[bytes, Optional[bytes]]:
cache_controls = {}
for hdr in headers.getRawHeaders(b"cache-control", []):
for directive in hdr.split(b","):
Expand Down