Skip to content
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/19756.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Force keyword-only args for `Duration` (prevent footgun) so people have to specify which time unit they want to us.
27 changes: 27 additions & 0 deletions synapse/util/duration.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,33 @@ class Duration(timedelta):
```
"""

# Using `__new__` (instead of `__init__`) because that's what `timedelta` uses
def __new__(
cls,
# The whole goal of overriding `__new__` is to require keyword-only arguments.
# Without this, `Duration(5)` would create a duration represnting 5 *days*
# (timedelta's default), but callers almost certainly want to specify which unit
# like seconds or hours.
*,
days: float = 0,
seconds: float = 0,
microseconds: float = 0,
milliseconds: float = 0,
minutes: float = 0,
hours: float = 0,
weeks: float = 0,
) -> "Duration":
return super().__new__(
cls,
days=days,
seconds=seconds,
microseconds=microseconds,
milliseconds=milliseconds,
minutes=minutes,
hours=hours,
weeks=weeks,
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When you forget the keyword arg now, the linter complains like this:

Duration(5)

synapse/handlers/worker_lock.py:264: error: Too many positional arguments for "Duration"  [misc]

def as_millis(self) -> int:
"""Returns the duration in milliseconds."""
return int(self / _ONE_MILLISECOND)
Expand Down
Loading