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.

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

Jump to bottom

Refactor middleware #325

Merged
merged 3 commits into from
Sep 7, 2019
Merged

Refactor middleware #325

merged 3 commits into from
Sep 7, 2019

Conversation

florimondmanca
Copy link
Member

@florimondmanca florimondmanca commented Sep 7, 2019 •
edited
Loading

Prompted by #305 (comment)

Moves the existing middleware in middleware.py into their own module in a middleware package.

Next up, a basic auth refactor including:

  • Add and use to_bytes()
  • Extract a reusable build_basic_auth_header() helper.

@florimondmanca florimondmanca force-pushed the refactor/middleware-package branch from 6b94d00 to 4545646 Compare September 7, 2019 20:52
@sethmlarson
Copy link
Contributor

Could we do those extras in this PR too under the cover of "refactor middlewares"?

@florimondmanca
Copy link
Member Author

florimondmanca commented Sep 7, 2019 •
edited
Loading

Could we do those extras in this PR too under the cover of "refactor middlewares"?

Uh, I guess I'm OK with that. I have another idea for refactoring the RedirectMiddleware to use contextvars, but that will definitely be in another PR. 😉 I'll push the extras.

@florimondmanca florimondmanca changed the title Split middleware into a subpackage Refactor middleware Sep 7, 2019
@florimondmanca florimondmanca force-pushed the refactor/middleware-package branch from 64e2b1f to 910af27 Compare September 7, 2019 21:13
@sethmlarson
Copy link
Contributor

Totally agree. You might have some minor conflicts over the PR I just merged.

@florimondmanca
Copy link
Member Author

Already force-pushed a rebase. 👍

sethmlarson
sethmlarson reviewed Sep 7, 2019
View reviewed changes
httpx/utils.py Outdated Show resolved Hide resolved
sethmlarson
sethmlarson approved these changes Sep 7, 2019
View reviewed changes
Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

If you fix the one comment below you're good to merge this. 👍

@florimondmanca florimondmanca merged commit 71648ec into master Sep 7, 2019
@florimondmanca florimondmanca deleted the refactor/middleware-package branch September 7, 2019 21:22
This was referenced Sep 7, 2019
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sethmlarson sethmlarson sethmlarson approved these changes

@yeraydiazdiaz yeraydiazdiaz Awaiting requested review from yeraydiazdiaz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@florimondmanca @sethmlarson