-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Turn State into a Mapping
#3036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
||
|
|
||
| class Request(HTTPConnection): | ||
| class Request(HTTPConnection[StateT]): |
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.
We are missing the same in WebSockets.
| strategy: | ||
| matrix: | ||
| python-version: ["3.9", "3.10", "3.11", "3.12", "3.13", "3.14"] | ||
| python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] |
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.
Just so the pipeline can pass, but we shouldn't merge this here.
|
@alex-oleshkevich @abersheeran Can you check this, please? |
All good. |
In the sense that this implementation makes sense, or something different? 🤔 |
We want to have a typed request state and keep backward compatibility. That was achieved, so all is good :) |
|
This was exactly what I had in mind :D thanks @Kludex! |
|
|
||
|
|
||
| class HTTPConnection(Mapping[str, Any]): | ||
| StateT = TypeVar("StateT", bound=Mapping[str, Any] | State, default=State) |
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 not sure if this will pass the TypedDict check in some static type checkers.
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.
Which one you don't think it will not? We test with mypy here, but pyright doesn't seem to be failing (I use it on my environment).
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.
It's great to be able to pass the inspection.
|
Hi there! 👋 |
Uh oh!
There was an error while loading. Please reload this page.