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

Improve performance of getting unread counts in rooms #13119

Merged
merged 6 commits into from
Jun 29, 2022

Conversation

erikjohnston
Copy link
Member

No description provided.

@erikjohnston erikjohnston marked this pull request as ready for review June 27, 2022 13:10
@erikjohnston erikjohnston requested a review from a team as a code owner June 27, 2022 13:10
@erikjohnston erikjohnston force-pushed the erikj/push_actions_perf branch from 7603dcd to 8845dfd Compare June 27, 2022 14:24
Copy link
Contributor

@squahtx squahtx left a comment

Choose a reason for hiding this comment

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

lgtm, two minor comments

synapse/storage/databases/main/event_push_actions.py Outdated Show resolved Hide resolved
Comment on lines 800 to 815
@overload
def get_stream_id_for_event_txn(
self,
txn: LoggingTransaction,
event_id: str,
allow_none: Literal[True],
) -> int:
...

@overload
def get_stream_id_for_event_txn(
self,
txn: LoggingTransaction,
event_id: str,
) -> int:
...
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get away with one fewer overload if we include the default value for allow_none?

Suggested change
@overload
def get_stream_id_for_event_txn(
self,
txn: LoggingTransaction,
event_id: str,
allow_none: Literal[True],
) -> int:
...
@overload
def get_stream_id_for_event_txn(
self,
txn: LoggingTransaction,
event_id: str,
) -> int:
...
@overload
def get_stream_id_for_event_txn(
self,
txn: LoggingTransaction,
event_id: str,
allow_none: Literal[True] = True,
) -> int:
...

Copy link
Member Author

Choose a reason for hiding this comment

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

Err, yes. I got confused and these type hints are actually wrong (allow_none=True should return an Optional)

@erikjohnston erikjohnston enabled auto-merge (squash) June 29, 2022 10:03
@erikjohnston erikjohnston merged commit 92a0c18 into develop Jun 29, 2022
@erikjohnston erikjohnston deleted the erikj/push_actions_perf branch June 29, 2022 10:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants