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

Annotate log_function decorator #10943

Merged
merged 21 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
a7f67ac
Annotate `log_function` decorator
reivilibre Sep 29, 2021
fa53832
Fix up type annotations for `get_new_events` on `EventSource` subclasses
reivilibre Sep 29, 2021
fb83e5a
Remove dead assignment with type violation
reivilibre Sep 29, 2021
3376afa
Properly annotate as `Awaitable`
reivilibre Sep 29, 2021
f7e95de
Use `Mapping` in lieu of `Dict` for `resolve_state_groups`
reivilibre Sep 29, 2021
d3a6034
Fix type annotations of `update_remote_profile_cache`
reivilibre Sep 29, 2021
1db0538
Prevent type annotation clash by renaming redefined variable
reivilibre Sep 29, 2021
8bf4b0c
Correct type annotations
reivilibre Sep 29, 2021
721f462
Properly state assertion that transaction data is not None
reivilibre Sep 29, 2021
d761be4
Add needed type annotation to local
reivilibre Sep 29, 2021
d0c46f1
Newsfile
reivilibre Sep 29, 2021
e209c47
Don't force `get_new_events` to take keyword-only arguments
reivilibre Sep 29, 2021
6db34e8
Simplify if conditions
reivilibre Oct 22, 2021
dce957c
Update synapse/federation/federation_server.py
reivilibre Oct 22, 2021
98b340f
Validate transaction_data before indexing into it
reivilibre Oct 22, 2021
3b8738c
Merge remote-tracking branch 'origin/develop' into rei/annotate_log_f…
reivilibre Oct 22, 2021
9d8994d
Use Collection instead of Iterable because we rely on it being falsy …
reivilibre Oct 22, 2021
8c1f8dc
Don't forget backfill in the transport layer
reivilibre Oct 27, 2021
c48eeea
TypeError is marginally more sensible than ValueError
reivilibre Oct 27, 2021
cfef2dd
Add TODO about federation client validation exception type
reivilibre Oct 27, 2021
0149961
Merge remote-tracking branch 'origin/develop' into rei/annotate_log_f…
reivilibre Oct 27, 2021
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/10943.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add type annotations for the `log_function` decorator.
13 changes: 11 additions & 2 deletions synapse/federation/federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ async def claim_client_keys(
)

async def backfill(
self, dest: str, room_id: str, limit: int, extremities: Iterable[str]
self, dest: str, room_id: str, limit: int, extremities: Collection[str]
) -> Optional[List[EventBase]]:
"""Requests some more historic PDUs for the given room from the
given destination server.
Expand All @@ -237,6 +237,8 @@ async def backfill(
room_id: The room_id to backfill.
limit: The maximum number of events to return.
extremities: our current backwards extremities, to backfill from
Must be a Collection that is falsy when empty.
(Iterable is not enough here!)
squahtx marked this conversation as resolved.
Show resolved Hide resolved
"""
logger.debug("backfill extrem=%s", extremities)

Expand All @@ -250,11 +252,18 @@ async def backfill(

squahtx marked this conversation as resolved.
Show resolved Hide resolved
logger.debug("backfill transaction_data=%r", transaction_data)

if not isinstance(transaction_data, dict):
raise ValueError("Backfill transaction_data is not a dict.")
squahtx marked this conversation as resolved.
Show resolved Hide resolved

transaction_data_pdus = transaction_data.get("pdus")
if not isinstance(transaction_data_pdus, list):
raise ValueError("transaction_data.pdus is not a list.")

room_version = await self.store.get_room_version(room_id)

pdus = [
event_from_pdu_json(p, room_version, outlier=False)
for p in transaction_data["pdus"]
for p in transaction_data_pdus
]

# Check signatures and hash of pdus, removing any from the list that fail checks
Expand Down
10 changes: 6 additions & 4 deletions synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,16 @@ async def _handle_incoming_transaction(
Returns:
HTTP response code and body
"""
response = await self.transaction_actions.have_responded(origin, transaction)
existing_response = await self.transaction_actions.have_responded(
origin, transaction
)

if response:
if existing_response:
logger.debug(
"[%s] We've already responded to this request",
transaction.transaction_id,
)
return response
return existing_response

logger.debug("[%s] Transaction is new", transaction.transaction_id)

Expand Down Expand Up @@ -632,7 +634,7 @@ async def on_send_leave_request(

async def on_make_knock_request(
self, origin: str, room_id: str, user_id: str, supported_versions: List[str]
) -> Dict[str, Union[EventBase, str]]:
) -> JsonDict:
"""We've received a /make_knock/ request, so we create a partial knock
event for the room and hand that back, along with the room version, to the knocking
homeserver. We do *not* persist or process this event until the other server has
Expand Down
1 change: 0 additions & 1 deletion synapse/federation/sender/transaction_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ def json_data_cb() -> JsonDict:
)
except HttpResponseException as e:
code = e.code
response = e.response

set_tag(tags.ERROR, True)

Expand Down
15 changes: 13 additions & 2 deletions synapse/federation/transport/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,18 @@

import logging
import urllib
from typing import Any, Callable, Dict, Iterable, List, Mapping, Optional, Tuple, Union
from typing import (
Any,
Awaitable,
Callable,
Dict,
Iterable,
List,
Mapping,
Optional,
Tuple,
Union,
)

import attr
import ijson
Expand Down Expand Up @@ -786,7 +797,7 @@ async def accept_group_invite(
@log_function
def join_group(
self, destination: str, group_id: str, user_id: str, content: JsonDict
) -> JsonDict:
) -> Awaitable[JsonDict]:
"""Attempts to join a group"""
path = _create_v1_path("/groups/%s/users/%s/join", group_id, user_id)

Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ async def get_association(self, room_alias: RoomAlias) -> JsonDict:
servers = result.servers
else:
try:
fed_result = await self.federation.make_query(
fed_result: Optional[JsonDict] = await self.federation.make_query(
destination=room_alias.domain,
query_type="directory",
args={"room_alias": room_alias.to_string()},
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ async def process_remote_join(

@log_function
async def backfill(
self, dest: str, room_id: str, limit: int, extremities: Iterable[str]
self, dest: str, room_id: str, limit: int, extremities: Collection[str]
) -> None:
"""Trigger a backfill request to `dest` for the given `room_id`

Expand Down
2 changes: 2 additions & 0 deletions synapse/handlers/presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
from synapse.api.constants import EventTypes, Membership, PresenceState
from synapse.api.errors import SynapseError
from synapse.api.presence import UserPresenceState
from synapse.appservice import ApplicationService
from synapse.events.presence_router import PresenceRouter
from synapse.logging.context import run_in_background
from synapse.logging.utils import log_function
Expand Down Expand Up @@ -1551,6 +1552,7 @@ async def get_new_events(
is_guest: bool = False,
explicit_room_id: Optional[str] = None,
include_offline: bool = True,
service: Optional[ApplicationService] = None,
) -> Tuple[List[UserPresenceState], int]:
# The process for getting presence events are:
# 1. Get the rooms the user is in.
Expand Down
4 changes: 4 additions & 0 deletions synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,11 @@ async def _update_remote_profile_cache(self) -> None:
continue

new_name = profile.get("displayname")
if not isinstance(new_name, str):
new_name = None
new_avatar = profile.get("avatar_url")
if not isinstance(new_avatar, str):
new_avatar = None

# We always hit update to update the last_check timestamp
await self.store.update_remote_profile_cache(user_id, new_name, new_avatar)
8 changes: 6 additions & 2 deletions synapse/logging/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import logging
from functools import wraps
from inspect import getcallargs
from typing import Callable, TypeVar, cast

_TIME_FUNC_ID = 0

Expand All @@ -41,7 +42,10 @@ def _log_debug_as_f(f, msg, msg_args):
logger.handle(record)


def log_function(f):
F = TypeVar("F", bound=Callable)


def log_function(f: F) -> F:
"""Function decorator that logs every call to that function."""
func_name = f.__name__

Expand Down Expand Up @@ -69,4 +73,4 @@ def format(value):
return f(*args, **kwargs)

wrapped.__name__ = func_name
return wrapped
return cast(F, wrapped)
5 changes: 3 additions & 2 deletions synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
FrozenSet,
Iterable,
List,
Mapping,
Optional,
Sequence,
Set,
Expand Down Expand Up @@ -519,7 +520,7 @@ async def resolve_state_groups(
self,
room_id: str,
room_version: str,
state_groups_ids: Dict[int, StateMap[str]],
state_groups_ids: Mapping[int, StateMap[str]],
event_map: Optional[Dict[str, EventBase]],
state_res_store: "StateResolutionStore",
) -> _StateCacheEntry:
Expand Down Expand Up @@ -703,7 +704,7 @@ def _report_biggest(


def _make_state_cache_entry(
new_state: StateMap[str], state_groups_ids: Dict[int, StateMap[str]]
new_state: StateMap[str], state_groups_ids: Mapping[int, StateMap[str]]
) -> _StateCacheEntry:
"""Given a resolved state, and a set of input state groups, pick one to base
a new state group on (if any), and return an appropriately-constructed
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ async def set_profile_avatar_url(
)

async def update_remote_profile_cache(
self, user_id: str, displayname: str, avatar_url: str
self, user_id: str, displayname: Optional[str], avatar_url: Optional[str]
) -> int:
return await self.db_pool.simple_update(
table="remote_profile_cache",
Expand Down