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

Refactor ASGITransport.request() #1021

Merged
merged 2 commits into from
Jun 13, 2020
Merged

Refactor ASGITransport.request() #1021

merged 2 commits into from
Jun 13, 2020

Conversation

florimondmanca
Copy link
Member

@florimondmanca florimondmanca commented Jun 13, 2020

Prompted by #998 (comment), small bit of refactoring for ASGITransport to help reduce the diff there. I think these are valid readability improvements any case.

  • Unify typing import style.
  • Declare related variables in logical groups.
  • Cleanup any unnecessary nonlocal's
  • Add some comments on how everything is split.

@florimondmanca florimondmanca added the refactor Issues and PRs related to code refactoring label Jun 13, 2020
@@ -144,7 +148,7 @@ async def send(message: dict) -> None:
try:
await self.app(scope, receive, send)
except Exception:
if self.raise_app_exceptions or not response_complete:
if self.raise_app_exceptions or not response_complete.is_set():
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this was a typo before.

Copy link
Member

Choose a reason for hiding this comment

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

Think that was my mistake. Missed this when changing response_complete from a bool to an event. Also didn't realise the nonlocal could be removed 😬

@florimondmanca florimondmanca requested a review from a team June 13, 2020 13:42
Copy link
Member

@JayH5 JayH5 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@@ -144,7 +148,7 @@ async def send(message: dict) -> None:
try:
await self.app(scope, receive, send)
except Exception:
if self.raise_app_exceptions or not response_complete:
if self.raise_app_exceptions or not response_complete.is_set():
Copy link
Member

Choose a reason for hiding this comment

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

Think that was my mistake. Missed this when changing response_complete from a bool to an event. Also didn't realise the nonlocal could be removed 😬

@florimondmanca florimondmanca merged commit b481166 into master Jun 13, 2020
@florimondmanca florimondmanca deleted the refactor-asgi branch June 13, 2020 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues and PRs related to code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants