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

Allow for an HttpError without a response #1023

Merged
merged 5 commits into from
Oct 22, 2023
Merged

Conversation

bhrutledge
Copy link
Contributor

Hopefully this will fix the CI failures that started last week, probably as a result of types-requests 2.31.09.

@@ -32,6 +32,9 @@ def main() -> Any:
try:
error = cli.dispatch(sys.argv[1:])
except requests.HTTPError as exc:
if not exc.response: # pragma: no cover
raise
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-raising because that follows the pattern of other unexpected exceptions. IE, I don't know what to do with an HTTPError that doesn't include a response.

Copy link
Member

Choose a reason for hiding this comment

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

I can say with authority that types-requests is wrong and you should ignore this from mypy or typing.cast the response attribute.

Integration test is failing is a good sign this is the wrong change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The source for RequestException (the base class of HttpError) seems to suggest that it is possible for exc.response to be None.

I thought the integration test failure was a pre-existing condition, but I see that's not the case. I don't understand why this changed caused it fail.

I switched to cast() in 0b73a90.

@bhrutledge
Copy link
Contributor Author

Not sure why the integration tests are failing, but we haven't let that be a blocker in the past.

@sigmavirus24 sigmavirus24 merged commit 88245fc into main Oct 22, 2023
20 checks passed
@sigmavirus24 sigmavirus24 deleted the fix-response-error branch October 22, 2023 01:36
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.

None yet

2 participants