Skip to content

Silence PIE786 in some cases#1283

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.

Sign up for GitHub

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

Closed
cdeler wants to merge 4 commits into from
Closed

Silence PIE786 in some cases #1283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
225cc1a
Silent PIE786 in project config
cdeler Sep 11, 2020
fb29354
Replaced BaseException by Exception in _client.py
cdeler Sep 11, 2020
f8f2dc4
Disabled PIE786 per line
cdeler Sep 12, 2020
3dcf93f
Merge branch 'master' into fixed-problem-with-pipeline
cdeler Sep 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions httpx/_client.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -763,7 +763,7 @@ def _send_handling_auth(
next_request = auth_flow.send(response)
except StopIteration:
return response
except BaseException as exc:
except Exception as exc: # noqa: PIE786

@florimondmancaflorimondmancaSep 12, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we stick to catching BaseException here and in the other change in this file?

@cdelercdelerSep 12, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was thinking about that.

What the benefits of BaseException?

Catching BaseException there, we also handleKeyboardInterrupt, GeneratorExit and SystemExit and close the response in these cases. I don't think it's a proper behaviour (but may be I miss something)

@florimondmancaflorimondmancaSep 12, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm okay with either, but since it would be a behavioral let's try and submit that as a separate PR, so that this only focuses on silencing any PIE786 errors?

@cdelercdelerSep 12, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure, I'll do that as soon as I reach my laptop

response.close()
raise exc from None
else:
Expand DownExpand Up@@ -1386,7 +1386,7 @@ async def _send_handling_auth(
next_request = await auth_flow.asend(response)
except StopAsyncIteration:
return response
except BaseException as exc:
except Exception as exc: # noqa: PIE786
await response.aclose()
raise exc from None
else:
Expand Down
2 changes: 1 addition & 1 deletion httpx/_exceptions.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -340,7 +340,7 @@ def map_exceptions(
) -> typing.Iterator[None]:
try:
yield
except Exception as exc:
except Exception as exc: # noqa: PIE786
mapped_exc = None

for from_exc, to_exc in mapping.items():
Expand Down
2 changes: 1 addition & 1 deletion httpx/_transports/asgi.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ async def send(message: dict) -> None:

try:
await self.app(scope, receive, send)
except Exception:
except Exception: # noqa: PIE786
if self.raise_app_exceptions or not response_complete.is_set():
raise

Expand Down