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

requests: Use the Any trick in HTTPError #11207

Merged
merged 7 commits into from
Dec 30, 2023

Conversation

Akuli
Copy link
Collaborator

@Akuli Akuli commented Dec 28, 2023

See #10875 for context.

Usually, when you get a HTTPError, the response will be set. Specifically, it will always be set when the requests library raises the error.

But people also use HTTPError as a generic "just show this generic error message" exception that doesn't include any details about what happened. For example, paasta_tools/mesos_maintenance.py has plenty of code like this (as pointed out by mypy_primer):

    try:
        status = get_maintenance_status(mesos_config_path).json()
        status = status["get_maintenance_status"]["status"]
    except HTTPError:
        raise HTTPError("Error getting maintenance status.")

I think we should get the best of both worlds with the Any trick.

This comment has been minimized.

This comment has been minimized.

@Akuli Akuli marked this pull request as draft December 28, 2023 16:24

This comment has been minimized.

@Akuli
Copy link
Collaborator Author

Akuli commented Dec 28, 2023

The mypy_primer output isn't really comparable to the previous PR, because people have already added redundant casts and None checks:

Also, mypy doesn't warn about unnecessary checks or casts: an if statement with Response | Any or a cast from Response | Any to Response could be useful.

Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

cloud-init (https://github.com/canonical/cloud-init)
- cloudinit/url_helper.py:343: error: Item "None" of "Response | None" has no attribute "status_code"  [union-attr]
- cloudinit/url_helper.py:344: error: Item "None" of "Response | None" has no attribute "headers"  [union-attr]

@Akuli Akuli marked this pull request as ready for review December 28, 2023 16:45
@hauntsaninja hauntsaninja merged commit 3ede056 into python:main Dec 30, 2023
48 checks passed
@Akuli Akuli deleted the requests-http-error-any-trick branch December 30, 2023 10:09
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