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

Add type hints to synapse.storage.databases.main.client_ips #10972

Merged
merged 7 commits into from
Oct 12, 2021
Merged

Add type hints to synapse.storage.databases.main.client_ips #10972

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
80c73c2
Fix error in `get_user_ip_and_agents` when fetching from the database
Oct 1, 2021
cf20c86
Fix inconsistent behavior of `get_last_client_by_ip`
Oct 1, 2021
744756d
Add type hints to `synapse.storage.databases.main.client_ips`
Oct 1, 2021
b452f8b
Use classy TypedDicts and give them better names
Oct 4, 2021
7637353
Merge branch 'develop' into squah/add_type_hints_to_client_ip_store
squahtx Oct 12, 2021
519bb6b
Clean up diff for tests/storage/test_client_ips.py
Oct 12, 2021
0acd03f
Clean up diff
Oct 12, 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/10968.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix `/admin/whois/{user_id}` endpoint, which was broken in v1.44.0rc1.
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions changelog.d/10970.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix inconsistent behavior of `get_last_client_by_ip` when reporting data that has not been stored in the database yet.
1 change: 1 addition & 0 deletions changelog.d/10972.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add type hints to `synapse.storage.databases.main.client_ips`.
4 changes: 4 additions & 0 deletions mypy.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ files =
synapse/storage/_base.py,
synapse/storage/background_updates.py,
synapse/storage/databases/main/appservice.py,
synapse/storage/databases/main/client_ips.py,
synapse/storage/databases/main/events.py,
synapse/storage/databases/main/keys.py,
synapse/storage/databases/main/pusher.py,
Expand Down Expand Up @@ -99,6 +100,9 @@ disallow_untyped_defs = True
[mypy-synapse.rest.*]
disallow_untyped_defs = True

[mypy-synapse.storage.databases.main.client_ips]
disallow_untyped_defs = True
Copy link
Contributor

Choose a reason for hiding this comment

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

<3


[mypy-synapse.util.batching_queue]
disallow_untyped_defs = True

Expand Down
15 changes: 13 additions & 2 deletions synapse/handlers/device.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING, Collection, Dict, Iterable, List, Optional, Set, Tuple
from typing import (
TYPE_CHECKING,
Any,
Collection,
Dict,
Iterable,
List,
Mapping,
Optional,
Set,
Tuple,
)

from synapse.api import errors
from synapse.api.constants import EventTypes
Expand Down Expand Up @@ -595,7 +606,7 @@ async def rehydrate_device(


def _update_device_from_client_ips(
device: JsonDict, client_ips: Dict[Tuple[str, str], JsonDict]
device: JsonDict, client_ips: Mapping[Tuple[str, str], Mapping[str, Any]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

client_ips comes from the return of get_last_client_ip_by_device. I downgraded the Dicts to Mappings to make mypy happy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was mypy unhappy here, sorry---something to do with a Dict being mutable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to do with the inner JsonDict not being the same as LastClientIpByDeviceEntry:
Argument 2 ... has incompatible type "Dict[Tuple[str, str], LastClientIpByDeviceEntry]"; expected "Dict[Tuple[str, str], Dict[str, Any]]" [arg-type]

) -> None:
ip = client_ips.get((device["user_id"], device["device_id"]), {})
device.update({"last_seen_ts": ip.get("last_seen"), "last_seen_ip": ip.get("ip")})
Expand Down
6 changes: 3 additions & 3 deletions synapse/module_api/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,9 +773,9 @@ async def get_user_ip_and_agents(
# Sanitize some of the data. We don't want to return tokens.
return [
UserIpAndAgent(
ip=str(data["ip"]),
user_agent=str(data["user_agent"]),
last_seen=int(data["last_seen"]),
ip=data["ip"],
user_agent=data["user_agent"],
last_seen=data["last_seen"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: last_seen was int, is now a str?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why this is safe? (Maybe I just need to chase the logic through next week?)

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 thought data["last_seen"] is still an int?

raw_data used to be a Dict[str, Union[str, int]], but is now a TypedDict so these ought to already have the correct type.

)
for data in raw_data
]
Expand Down
Loading