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

Upgrade httpx #1935

Merged
merged 3 commits into from
Sep 27, 2020
Merged

Upgrade httpx #1935

merged 3 commits into from
Sep 27, 2020

Conversation

ahopkins
Copy link
Member

I had thought pretty long about whether to get the PR for removing httpx requirement (#1850), but that didn't seem quite appropriate just yet.

Because httpx is a fairly fast moving project, there were a number of changes needed to be made to some tests as well as the test client. Hopefully, we will get sanic-testing completed and then we will not need to deal with these changes here.

Copy link
Member

@ashleysommer ashleysommer 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 to me

ashleysommer
ashleysommer previously approved these changes Sep 27, 2020
@ashleysommer
Copy link
Member

ReuseableSanicTestClient and DelayableHTTPConnection are some pretty hairy hacks I put together a couple of years ago in order to test the HTTP Keep-Alive mechanism, and Request Timeouts respectively.
They're a pain to maintain, but I don't know of any other way to test these features (unless httpx gets these features natively).
It might be worth considering pulling these out of the sanic test code and into the Sanic-Testing project, to keep all of that stuff together.

@ashleysommer
Copy link
Member

Black test failing:

would reformat /home/travis/build/huge-success/sanic/sanic/testing.py
would reformat /home/travis/build/huge-success/sanic/sanic/response.py

@ahopkins
Copy link
Member Author

I agree. I have written and rewritten those several times because of the fast pace at which underlying libraries reorganize themselves. I do have a thought on how to handle, and this is actually THE reason I think we need to hold off on sanic-testing for now.

While I understand the desire to move them out to keep them closer, I think the test themselves need to stay here because it is Sanic functionality we need to worry about.

Therefore, I want to add the functionality into the core of sanic-testing so the tests can stay, but the hack clients can go.

@ahopkins
Copy link
Member Author

Had to upgrade black and isort with the particular env I had running. Should be ready when done building.

@ashleysommer ashleysommer merged commit ed777e9 into master Sep 27, 2020
@ahopkins ahopkins deleted the httpx-upgrade branch September 28, 2020 20:19
@ahopkins ahopkins mentioned this pull request Sep 30, 2020
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