-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Factor out MultiWriter
token from RoomStreamToken
#16427
Conversation
@@ -60,6 +61,8 @@ | |||
from synapse.util.stringutils import parse_and_validate_server_name | |||
|
|||
if TYPE_CHECKING: | |||
from typing_extensions import Self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires typing_extensions >= 4.0
. Not sure how we feel about bumping that? Or just leaving it behind TYPE_CHECKING
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy as long as the packagers are. https://pkgs.org/search/?q=typing-extensions and https://repology.org/project/python:typing-extensions/versions. Debian buster and Ubuntu focal + jammy have 3.x; I can't remember if we've dropped support for those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ISWYM. It'd be nice to be able to just import it, but I don't mind the conditional import and referencing it in "quotes" if that helps the packagers too. (ISTR we have to quote "defer.Deferred[blah]" anyway...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think putting it behind TYPE_CHECKING
is fine.
value_validator=attr.validators.instance_of(int), | ||
mapping_validator=attr.validators.instance_of(immutabledict), | ||
), | ||
kw_only=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG can we use kw_only
everywhere?!?! 😍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit if CI is green.
sytest + workers failing scares me a bit though... |
So we can reuse code when we split up other streams.