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

Type hints for proxyagent #10608

Merged
merged 8 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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 changelog.d/10608.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve type hints for the proxy agent and SRV resolver modules. Contributed by @dklimpel.
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@ files =
synapse/federation,
synapse/groups,
synapse/handlers,
synapse/http/additional_resource.py,
synapse/http/client.py,
synapse/http/federation/matrix_federation_agent.py,
synapse/http/federation/srv_resolver.py,
synapse/http/federation/well_known_resolver.py,
synapse/http/matrixfederationclient.py,
synapse/http/proxyagent.py,
synapse/http/servlet.py,
synapse/http/server.py,
synapse/http/site.py,
Expand Down
13 changes: 10 additions & 3 deletions synapse/http/additional_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import TYPE_CHECKING

from twisted.web.server import Request

from synapse.http.server import DirectServeJsonResource

if TYPE_CHECKING:
from synapse.server import HomeServer


class AdditionalResource(DirectServeJsonResource):
"""Resource wrapper for additional_resources
Expand All @@ -25,22 +32,22 @@ class AdditionalResource(DirectServeJsonResource):
and exception handling.
"""

def __init__(self, hs, handler):
def __init__(self, hs: "HomeServer", handler):
"""Initialise AdditionalResource

The ``handler`` should return a deferred which completes when it has
done handling the request. It should write a response with
``request.write()``, and call ``request.finish()``.

Args:
hs (synapse.server.HomeServer): homeserver
hs: homeserver
handler ((twisted.web.server.Request) -> twisted.internet.defer.Deferred):
function to be called to handle the request.
"""
super().__init__()
self._handler = handler

def _async_render(self, request):
def _async_render(self, request: Request):
# Cheekily pass the result straight through, so we don't need to worry
# if its an awaitable or not.
return self._handler(request)
45 changes: 25 additions & 20 deletions synapse/http/federation/srv_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 List
from typing import Callable, Dict, List

import attr

Expand All @@ -28,35 +28,35 @@

logger = logging.getLogger(__name__)

SERVER_CACHE = {}
SERVER_CACHE: Dict[bytes, List["Server"]] = {}


@attr.s(slots=True, frozen=True)
@attr.s(auto_attribs=True, slots=True, frozen=True)
class Server:
"""
Our record of an individual server which can be tried to reach a destination.

Attributes:
host (bytes): target hostname
port (int):
priority (int):
weight (int):
expires (int): when the cache should expire this record - in *seconds* since
host: target hostname
port:
priority:
weight:
expires: when the cache should expire this record - in *seconds* since
the epoch
"""

host = attr.ib()
port = attr.ib()
priority = attr.ib(default=0)
weight = attr.ib(default=0)
expires = attr.ib(default=0)
host: bytes
port: int
priority: int = 0
weight: int = 0
expires: int = 0


def _sort_server_list(server_list):
def _sort_server_list(server_list: List[Server]) -> List[Server]:
"""Given a list of SRV records sort them into priority order and shuffle
each priority with the given weight.
"""
priority_map = {}
priority_map: Dict[int, List[Server]] = {}

for server in server_list:
priority_map.setdefault(server.priority, []).append(server)
Expand Down Expand Up @@ -103,11 +103,16 @@ class SrvResolver:

Args:
dns_client (twisted.internet.interfaces.IResolver): twisted resolver impl
cache (dict): cache object
get_time (callable): clock implementation. Should return seconds since the epoch
cache: cache object
get_time: clock implementation. Should return seconds since the epoch
"""

def __init__(self, dns_client=client, cache=SERVER_CACHE, get_time=time.time):
def __init__(
self,
dns_client=client,
Copy link
Member

Choose a reason for hiding this comment

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

You'll probably need to import this.

Suggested change
dns_client=client,
dns_client: IResolver = client,

(Also please remove it from the docstring.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this and got new errors. I will reproduce it an post it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With dns_client: IResolver = client,

I get this:

synapse/http/federation/srv_resolver.py:113: error: Incompatible default for argument "dns_client" (default has type Module, argument has type "IResolver")  [assignment]
synapse/http/federation/srv_resolver.py:143: error: Missing positional argument "timeout" in call to "lookupService" of "IResolver"  [call-arg]
synapse/http/federation/srv_resolver.py:143: error: Argument 1 to "lookupService" of "IResolver" has incompatible type "bytes"; expected "str"  [arg-type]

Copy link
Member

Choose a reason for hiding this comment

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

That's unfortunate, it seems that some of the Twisted types around DNS are odd. You can leave it for now and we can take another look in the future.

cache: Dict[bytes, List[Server]] = SERVER_CACHE,
get_time: Callable[[], float] = time.time,
):
self._dns_client = dns_client
self._cache = cache
self._get_time = get_time
Expand All @@ -116,7 +121,7 @@ async def resolve_service(self, service_name: bytes) -> List[Server]:
"""Look up a SRV record

Args:
service_name (bytes): record to look up
service_name: record to look up

Returns:
a list of the SRV records, or an empty list if none found
Expand Down Expand Up @@ -158,7 +163,7 @@ async def resolve_service(self, service_name: bytes) -> List[Server]:
and answers[0].payload
and answers[0].payload.target == dns.Name(b".")
):
raise ConnectError("Service %s unavailable" % service_name)
raise ConnectError(f"Service {service_name!r} unavailable")

servers = []

Expand Down
4 changes: 2 additions & 2 deletions synapse/http/proxyagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def request(
raise ValueError(f"Invalid URI {uri!r}")

parsed_uri = URI.fromBytes(uri)
pool_key = (parsed_uri.scheme, parsed_uri.host, parsed_uri.port)
pool_key = f"{parsed_uri.scheme!r}{parsed_uri.host!r}{parsed_uri.port}"
request_path = parsed_uri.originForm

should_skip_proxy = False
Expand All @@ -199,7 +199,7 @@ def request(
)
# Cache *all* connections under the same key, since we are only
# connecting to a single destination, the proxy:
pool_key = ("http-proxy", self.http_proxy_endpoint)
pool_key = "http-proxy"
endpoint = self.http_proxy_endpoint
request_path = uri
elif (
Expand Down