Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve static type checking #225

Merged
merged 4 commits into from
May 25, 2021
Merged

Improve static type checking #225

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96ae78b
Improve static type checking
kibablu May 24, 2021
ccf60f0
Add a changelog on #225
kibablu May 24, 2021
8ae8f52
update on is_too_long as part of #201
kibablu May 25, 2021
36fe1e1
update on is_too_long(remove unused Dict) as part of #201
kibablu May 25, 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/225.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve static type checking. Contributed by Omar Mohamed.
4 changes: 2 additions & 2 deletions sygnal/apnstruncate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# Copied and adapted from
# https://raw.githubusercontent.com/matrix-org/pushbaby/master/pushbaby/truncate.py
import json
from typing import Any, List, Tuple, Union
from typing import Any, Dict, List, Tuple, Union


def json_encode(payload: Any) -> bytes:
Expand All @@ -26,7 +26,7 @@ class BodyTooLongException(Exception):
pass


def is_too_long(payload, max_length=2048):
def is_too_long(payload: Dict[Any, Any], max_length: int = 2048) -> bool:
Copy link
Contributor

@babolivier babolivier May 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like the type annotation you want is dict here (instead of Dict[Any, 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.

on it, Thank you :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that you've changed that with 8ae8f52 you'll also want to remove the import to typing.Dict on line 18 since it's not used anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I have just removed it after seeing my test fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@babolivier is there a difference between typing.Dict and dict on python versions?
while running mypy --strict sygnal/ tests/ setup.py I'm getting an error 👇
dict
I'm running python 3.8

sygnal/sygnal/apnstruncate.py

Line 29 in 656c0ea

def is_too_long(payload: dict, max_length: int = 2048) -> bool:

"""
Returns True if the given payload dictionary is too long for a push.
Note that the maximum is now 2kB "In iOS 8 and later" although in
Expand Down