Enable mypy for tools/quic/ #28916
Enable mypy for tools/quic/ #28916
Conversation
|
@yutakahirano can you review this, given that you added this code in #22844? |
This code was already mostly annotated, but needs updating to pass our mypy.ini rules. A few changes are noteworthy: - QuicTransportProtocol.streams was removed rather than annotated, because it's actually unused. - An assert for self.handler is added after process_client_indication() to it clear that that it can't be None in the following loop. Part of #28833.
tools/quic/quic_transport_server.py
Outdated
| self.connection = connection | ||
| self.global_dict = global_dict | ||
|
|
||
| def handle_client_indication( | ||
| self, | ||
| origin: str, | ||
| origin: Optional[str], |
There was a problem hiding this comment.
Can you keep this non-optional? Given we excluded the case where origin is None in process_client_indication, origin_string must not be none there, and hence origin must not be None here.
There was a problem hiding this comment.
There's an error 'Argument 1 to "handle_client_indication" of "EventHandler" has incompatible type "Optional[str]"; expected "str"' at self.handler.handle_client_indication(origin_string, query) if this is made non-optional, and at a glance it looks like origin_string could be none there, but actually it can't because of the if origin is None check. I'll see how I can convince mypy that origin_string can't be None there.
There was a problem hiding this comment.
OK, I've added some asserts which should have the right effect. I'm not sure if there's a prettier way to how that two variables are linked in this way, but asserting works.
This code was already mostly annotated, but needs updating to pass our
mypy.ini rules. A few changes are noteworthy:
because it's actually unused.
to it clear that that it can't be None in the following loop.
Part of #28833.