python-hyper / wsproto

Sans-IO WebSocket protocol implementation
https://wsproto.readthedocs.io/
MIT License
261 stars 38 forks source link

`WSConnection.initiate_upgrade_connection` can accept `bytes` as `path` argument #174

Closed vxgmichel closed 2 years ago

vxgmichel commented 2 years ago

In h11, the target argument to h11.Request is typed as Union[bytes, str] (see the latest docs).

Note that it was already the case in version 0.9.0.

IMO, it would make sense to type initiate_upgrade_connection accordingly:

diff --git a/src/wsproto/__init__.py b/src/wsproto/__init__.py
index 307cd2b..92dd195 100644
--- a/src/wsproto/__init__.py
+++ b/src/wsproto/__init__.py
@@ -4,7 +4,7 @@ wsproto

 A WebSocket implementation.
 """
-from typing import Generator, Optional
+from typing import Generator, Optional, Union

 from .connection import Connection, ConnectionState, ConnectionType
 from .events import Event
@@ -40,7 +40,7 @@ class WSConnection:
             return self.handshake.state
         return self.connection.state

-    def initiate_upgrade_connection(self, headers: Headers, path: str) -> None:
+    def initiate_upgrade_connection(self, headers: Headers, path: Union[bytes, str]) -> None:
         self.handshake.initiate_upgrade_connection(headers, path)

     def send(self, event: Event) -> bytes:
diff --git a/src/wsproto/handshake.py b/src/wsproto/handshake.py
index 8a8a93f..d5011f8 100644
--- a/src/wsproto/handshake.py
+++ b/src/wsproto/handshake.py
@@ -69,7 +69,7 @@ class H11Handshake:
         """
         return self._connection

-    def initiate_upgrade_connection(self, headers: Headers, path: str) -> None:
+    def initiate_upgrade_connection(self, headers: Headers, path: Union[bytes, str]) -> None:
         """Initiate an upgrade connection.

         This should be used if the request has already be received and

Would that make sense? I can make a PR if necessary.

pgjones commented 2 years ago

Fixed by 84b4f32b09bda69500e8f0175a8fa0a88736917b