Skip to content
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

[FEATURE] Incorrect HTTPVersion type-alias #3821

Closed
antos07 opened this issue Jul 30, 2023 · 7 comments · Fixed by #3823
Closed

[FEATURE] Incorrect HTTPVersion type-alias #3821

antos07 opened this issue Jul 30, 2023 · 7 comments · Fixed by #3823
Labels
enhancement type hinting Not bug. Not Docs. But both.

Comments

@antos07
Copy link

antos07 commented Jul 30, 2023

Steps to Reproduce

  1. Use ApplicationBuilder().http_version("2") to enable http2, as it said in the docs

Expected behaviour

Type-checkers accept literal '2'

Actual behaviour

Type-checker complains that Literal["1.1", "2.0"] was expected.
image

Operating System

Debian sid

Version of Python, python-telegram-bot & dependencies

python-telegram-bot 20.4
Bot API 6.7
Python 3.11.4 (main, Jun  7 2023, 10:13:09) [GCC 12.2.0]

Relevant log output

No response

Additional Context

In telegram._utils.types it defines HTTPVersion = Literal["1.1", "2.0"]. The correct one will be HTTPVersion = Literal["1.1", "2"].

@antos07 antos07 changed the title [BUG] Incorrect HTTPVersion type-hint [BUG] Incorrect HTTPVersion type-alias Jul 30, 2023
@antos07
Copy link
Author

antos07 commented Jul 30, 2023

And I would like to fix it

@lemontree210
Copy link
Member

Hey! Thanks for the catch! I would say it would make more sense to fix the docstring of AppBuilder.http_version. I think it's nice to have uniformity of version formats.

@lemontree210
Copy link
Member

OTOH, I do see that HTTP 2.0 is now called HTTP/2, so maybe you're right that the literal too should have the members "1.1" and "2". Let's see what others think :)

@harshil21
Copy link
Member

yeah, we should add "2" to the list, i.e. "1.1", "2.0" and "2"

@antos07
Copy link
Author

antos07 commented Jul 31, 2023

Well, in fact, "2.0" isn't a valid argument and HTTPXRequest throws an exception, when I use it

@Bibo-Joshi
Copy link
Member

That would ofc also have to be adapted :) Would you maybe like to send a PR yourself? If so, please be sure to check out our contribution guide (linked in the PR template and in the sidebar on docs.python-telegram-bot.org).

@Bibo-Joshi Bibo-Joshi changed the title [BUG] Incorrect HTTPVersion type-alias [FEATURE] Incorrect HTTPVersion type-alias Jul 31, 2023
@Bibo-Joshi Bibo-Joshi added enhancement type hinting Not bug. Not Docs. But both. and removed bug 🐛 labels Jul 31, 2023
@Bibo-Joshi
Copy link
Member

Okay, I just went ahead with it 😄 #3823

@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement type hinting Not bug. Not Docs. But both.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants