Skip to content

Fix all typing issues#134

Merged
fantix merged 9 commits into
MagicStack:masterfrom
Kludex:fix-typing
May 25, 2026
Merged

Fix all typing issues#134
fantix merged 9 commits into
MagicStack:masterfrom
Kludex:fix-typing

Conversation

@Kludex
Copy link
Copy Markdown
Contributor

@Kludex Kludex commented Oct 19, 2025

@fantix can you please check this?

Comment on lines -11 to -19
- 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)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not needed. Implementers can follow the protocol.

- on_chunk_header()
- on_chunk_complete()
- on_status(status: bytes)
def __init__(self, protocol: HTTPProtocol | object) -> None:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

protocol (HTTPProtocol): Callback interface for the parser.
"""

def set_dangerous_leniencies(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was missing.

Comment thread httptools/parser/parser.pyi Outdated
...

def feed_data(self, data: Union[bytes, bytearray, memoryview, array]) -> None:
def feed_data(self, data: Union[bytes, bytearray, memoryview, array[int]]) -> None:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

array is a Generic.

Comment on lines +7 to +15
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: ...
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Missing self.

Comment on lines -16 to -25
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
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Irrelevant. The URL is defined above.

Comment on lines +13 to +29
__all__ = (
# protocol
"HTTPProtocol",
# parser
"HttpParser",
"HttpRequestParser",
"HttpResponseParser",
# errors
"HttpParserError",
"HttpParserCallbackError",
"HttpParserInvalidStatusError",
"HttpParserInvalidMethodError",
"HttpParserInvalidURLError",
"HttpParserUpgrade",
# url_parser
"parse_url",
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Proper definition.

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 19, 2025

I've dropped the Unions since they are not needed in pyi files.

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 19, 2025

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... 😅

@fantix
Copy link
Copy Markdown
Member

fantix commented Oct 19, 2025

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.

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 19, 2025

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.

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.

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 20, 2025

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.

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.

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Oct 20, 2025

Btw, this gives less frustration to people when contributing to other projects:

Screenshot 2025-10-20 at 10 27 44

You can find it on:

Screenshot 2025-10-20 at 10 28 20

@fantix fantix merged commit a9bda0e into MagicStack:master May 25, 2026
19 checks passed
@fantix fantix mentioned this pull request May 25, 2026
pull Bot pushed a commit to sysfce2/python-httptools that referenced this pull request May 25, 2026
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants