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

Python 3.11, support new http.HTTPMethod enum in action and view #8995

Closed
sshishov opened this issue May 27, 2023 · 8 comments
Closed

Python 3.11, support new http.HTTPMethod enum in action and view #8995

sshishov opened this issue May 27, 2023 · 8 comments
Milestone

Comments

@sshishov
Copy link
Contributor

Bug report

When using python 3.11, instead of hardcoding HTTP verbs, now we can use official enum: HTTPMethod as well as HTTPStatus for statuses

What's wrong

We would like to use it in decorators.action(methods=[HTTPMethod.POST]) for instance.
Currently it does not work and produce the following error (with djangorestframework-stubs package):

error: List item 0 has incompatible type "Literal[HTTPMethod.POST]"; expected "Literal['GET', 'POST', 'DELETE', 'PUT', 'PATCH', 'TRACE', 'HEAD', 'OPTIONS', 'get', 'post', 'delete', 'put', 'patch', 'trace', 'head', 'options']"  [list-item]

How is that should be

No mypy error should be reported

System information

  • OS: MacOS / Debian (docker)
  • python version: 3.11
  • django version: 3.2.18
  • mypy version: 1.2.0
  • django-stubs version: 1.16.0
  • djangorestframework-stubs version: 1.10.0

Original issue:

The issue was originally reported against the stub project, but recommended to move the issue/discussion over here: typeddjango/djangorestframework-stubs#396

@kevin-brown
Copy link
Member

If you're able to add tests which confirm that DRF doesn't experience unexpected behavior when the enums are used instead of the strings (so new tests, don't just change existing tests) then I would be willing to review those. At that point we should be able to update the type stubs to reflect that as supported behavior.

@pavelkraleu
Copy link
Contributor

@kevin-brown Do you think that test I have drafted here is enough?
lower() is called on each method in @action which I think makes it safe to use HTTPMethod.

Screenshot 2023-07-27 at 16 50 14

@auvipy
Copy link
Member

auvipy commented Jul 28, 2023

that would be a good start

@pavelkraleu
Copy link
Contributor

@auvipy Do you think some other changes are needed?

Code below passes which I think proofs that everything below line 146 on image above behaves same for methods as str or HTTPMethod.

from http import HTTPMethod

assert isinstance(HTTPMethod.POST.lower(), str)

@sshishov
Copy link
Contributor Author

sshishov commented Aug 6, 2023

Just a heads-up, we are using this HTTPMethod.POST approach in production already for a long time, and there were no issues so far.

Side note: for requests library we are using HTTPStatus.OK and others to identify the statuses and HTTPMethod.GET and others to identify HTTP method used.

Thanks for looking into it @pavelkraleu

@pavelkraleu
Copy link
Contributor

Thank you for the confirmation @sshishov, I will make this change into final PR 💪

@pavelkraleu
Copy link
Contributor

@auvipy Could you look into it please? 👆

@auvipy auvipy added this to the 3.15 milestone Aug 15, 2023
@auvipy
Copy link
Member

auvipy commented Aug 15, 2023

thanks for the test and documentation update.

@auvipy auvipy closed this as completed Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants