Fix all typing issues - #134
Conversation
Sorry, something went wrong.
| - on_message_begin() | ||
| - on_url(url: bytes) | ||
| - on_header(name: bytes, value: bytes) | ||
| - on_headers_complete() | ||
| - on_body(body: bytes) | ||
| - on_message_complete() | ||
| - on_chunk_header() | ||
| - on_chunk_complete() | ||
| - on_status(status: bytes) |
There was a problem hiding this comment.
This is not needed. Implementers can follow the protocol.
Sorry, something went wrong.
| - on_chunk_header() | ||
| - on_chunk_complete() | ||
| - on_status(status: bytes) | ||
| def __init__(self, protocol: HTTPProtocol | object) -> None: |
There was a problem hiding this comment.
object is less permissive than Any.
Python still doesn't support "not required class method protocol": https://discuss.python.org/t/discussion-optional-class-and-protocol-fields-and-methods
Sorry, something went wrong.
| protocol (HTTPProtocol): Callback interface for the parser. | ||
| """ | ||
|
|
||
| def set_dangerous_leniencies( |
There was a problem hiding this comment.
This was missing.
Sorry, something went wrong.
| ... | ||
|
|
||
| def feed_data(self, data: Union[bytes, bytearray, memoryview, array]) -> None: | ||
| def feed_data(self, data: Union[bytes, bytearray, memoryview, array[int]]) -> None: |
There was a problem hiding this comment.
array is a Generic.
Sorry, something went wrong.
| def on_message_begin(self) -> None: ... | ||
| def on_url(self, url: bytes) -> None: ... | ||
| def on_header(self, name: bytes, value: bytes) -> None: ... | ||
| def on_headers_complete(self) -> None: ... | ||
| def on_body(self, body: bytes) -> None: ... | ||
| def on_message_complete(self) -> None: ... | ||
| def on_chunk_header(self) -> None: ... | ||
| def on_chunk_complete(self) -> None: ... | ||
| def on_status(self, status: bytes) -> None: ... |
There was a problem hiding this comment.
Missing self.
Sorry, something went wrong.
| Returns an instance of ``httptools.URL`` class with the | ||
| following attributes: | ||
|
|
||
| - schema: bytes | ||
| - host: bytes | ||
| - port: int | ||
| - path: bytes | ||
| - query: bytes | ||
| - fragment: bytes | ||
| - userinfo: bytes |
There was a problem hiding this comment.
Irrelevant. The URL is defined above.
Sorry, something went wrong.
| __all__ = ( | ||
| # protocol | ||
| "HTTPProtocol", | ||
| # parser | ||
| "HttpParser", | ||
| "HttpRequestParser", | ||
| "HttpResponseParser", | ||
| # errors | ||
| "HttpParserError", | ||
| "HttpParserCallbackError", | ||
| "HttpParserInvalidStatusError", | ||
| "HttpParserInvalidMethodError", | ||
| "HttpParserInvalidURLError", | ||
| "HttpParserUpgrade", | ||
| # url_parser | ||
| "parse_url", | ||
| ) |
There was a problem hiding this comment.
Proper definition.
Sorry, something went wrong.
|
I've dropped the Unions since they are not needed in pyi files. |
Sorry, something went wrong.
|
I've tested this on uvicorn, it works like a charm. It would be nice to add a type checker in this repository. EDIT: I meant the typing... 😅 |
Sorry, something went wrong.
|
Thank you! Looks there's an error: ImportError: cannot import name 'HttpParser' from 'httptools.parser.parser' Let me add a CI workflow to check typing. |
Sorry, something went wrong.
Then it seems the issue is that the parser.pyi exists? Should be __init__.pyi? 🤔 I'm on a plane. I'll fix this tomorrow. |
Sorry, something went wrong.
It seems HttpParser is marked as internal. I've fixed it. I've added the type checker to the pipeline, but feel free to tweak whatever you want. |
Sorry, something went wrong.
|
Btw, this gives less frustration to people when contributing to other projects: 
You can find it on: 
|
Sorry, something went wrong.
Changes ======= * Add http-parser and llhttp licenses into the wheels (MagicStack#135) (by @justeph in c398a15) * Mark cython module as free-threading compatible (MagicStack#139) (by @kumaraditya303 in 28d1db1) * Fix all typing issues (MagicStack#134) (by @Kludex in a9bda0e) * Bump llhttp to 9.4.1 (MagicStack#145) (by @fantix in e3e8d71) * Security: fix URL truncation issue (MagicStack#144) (by @fantix in a0283f0 for MagicStack#142) * Allow building with latest setuptools (MagicStack#138) (by @OldManYellsAtCloud in c403ad1)
@fantix can you please check this?