Skip to content

Introduce Hypercorn as a test HTTP/2 server - #412

Closed
pgjones wants to merge 2 commits into
encode:masterfrom
pgjones:master
Closed

Introduce Hypercorn as a test HTTP/2 server#412
pgjones wants to merge 2 commits into
encode:masterfrom
pgjones:master

Conversation

@pgjones

Copy link
Copy Markdown

This allows the HTTP/2 tests to be directed at an actual HTTP/2 server
rather than a mock server. This is beneficial as it is closer to the
actual usage by users.

Comment threadtests/dispatch/test_http2.py Outdated
Comment threadtests/dispatch/test_http2.py Outdated
Comment threadtests/dispatch/test_http2.py Outdated
Comment threadtests/dispatch/test_http2.py
@pgjones

Copy link
Copy Markdown
Author

I think it is at a good stage for feedback.

sethmlarson
sethmlarson previously requested changes Sep 29, 2019

@sethmlarsonsethmlarson left a comment

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.

Thanks for opening this @pgjones! Here's some preliminary comments.

Comment threadsetup.cfg
Comment threadtest-requirements.txt
Comment threadtests/dispatch/test_http2.py Outdated

async with AsyncClient(backend=backend) as client:
response = await client.get("http://example.org")
@pytest.mark.skipif(

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.

Can the h2_server raise the skip within the fixture?

@lovelydinosaur

Copy link
Copy Markdown
Member

From a POV of managing encode as a whole this one is a mixed bag.

I'd personally much rather look at adding HTTP/2 support to uvicorn, than switch to hypercorn as the test server. It's also problematic for us to switch to something that has a different set of Python requirements, (3.7+) rather than being able to keep roughly in line across a suite of projects.

I'm way more invested in promoting ecosystem-as-a-whole rather than individual projects, but just wrt. logisitics and management overhead, this change probably comes out as a net negative to me.

@pgjones

Copy link
Copy Markdown
Author

...than switch to hypercorn as the test server.

Hi @tomchristie this adds Hypercorn as a test server rather than replacing uvicorn as the test server, as per #290. In its current form it only uses Hypercorn for the HTTP/2 tests.

@florimondmancaflorimondmanca changed the title Introduce Hypercorn as a test serverIntroduce Hypercorn as a test HTTP/2 serverOct 4, 2019

@florimondmancaflorimondmanca left a comment

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 think this is a great addition.

There's definitely some refactoring required to help make the hypercorn-based HTTP/2 server as self-contained and isolated as possible. For example, move it to conftest.py with a custom H2Server class that exposes the required interface to be able to pass it to serve_in_thread(). I may draft a PR against your fork to demonstrate the idea. :)

@pgjones

Copy link
Copy Markdown
Author

@florimondmanca thanks. I'm happy to tidy this up - I just got it into a workable state to get initial thoughts.

pgjonesand others added 2 commits October 12, 2019 10:00
This allows the HTTP/2 tests to be directed at an actual HTTP/2 server
rather than a mock server. This is beneficial as it is closer to the
actual usage by users.
@pgjones

Copy link
Copy Markdown
Author

Thanks @florimondmanca I've pulled in your changes and rebased against master.

@florimondmancaflorimondmanca added the http/2 Issues and PRs related to HTTP/2 label Oct 19, 2019
Comment threadtests/conftest.py
@florimondmanca

Copy link
Copy Markdown
Contributor

While I appreciate a lot the effort put into getting something (almost!) working for this one, this PR has gone a bit stale because opinions are mixed about eventually integrating it into master. I'll close it, and maybe we can take inspiration from this for a future implementation (maybe when Uvicorn gets HTTP/2?). Thanks @pgjones! ❤️

Kludex referenced this pull request in pydantic/httpx2 May 11, 2026
Bumps [furo](https://github.com/pradyunsg/furo) from 2021.8.31 to 2021.10.9.
- [Release notes](https://github.com/pradyunsg/furo/releases)
- [Changelog](https://github.com/pradyunsg/furo/blob/main/docs/changelog.md)
- [Commits](pradyunsg/furo@2021.08.31...2021.10.09)
---
updated-dependencies:
- dependency-name: furo
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http/2Issues and PRs related to HTTP/2

Development

Successfully merging this pull request may close these issues.

5 participants

@pgjones@lovelydinosaur@florimondmanca@JayH5@sethmlarson