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

Factorize hypercon.Config() creation #3234

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 18 additions & 4 deletions dummyserver/hypercornserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
import functools
import sys
import threading
from typing import Generator
import typing

import hypercorn
import hypercorn.trio
import hypercorn.typing
import trio
from quart_trio import QuartTrio

from urllib3.util.url import parse_url


# https://github.com/pgjones/hypercorn/blob/19dfb96411575a6a647cdea63fa581b48ebb9180/src/hypercorn/utils.py#L172-L178
async def graceful_shutdown(shutdown_event: threading.Event) -> None:
Expand Down Expand Up @@ -42,8 +44,20 @@ async def _start_server(

@contextlib.contextmanager
def run_hypercorn_in_thread(
config: hypercorn.Config, app: hypercorn.typing.ASGIFramework
) -> Generator[None, None, None]:
host: str, certs: dict[str, typing.Any] | None, app: hypercorn.typing.ASGIFramework
) -> typing.Iterator[int]:
Copy link
Contributor

@graingert graingert Jan 7, 2024

Choose a reason for hiding this comment

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

Iterator is technically the wrong type, because contextmanager incorrectly accepts an object without .throw. This bug might get fixed in typeshed

Suggested change
) -> typing.Iterator[int]:
) -> Generator[int, None, None]

config = hypercorn.Config()
if certs:
config.certfile = certs["certfile"]
config.keyfile = certs["keyfile"]
if "cert_reqs" in certs:
config.verify_mode = certs["cert_reqs"]
if "cacerts" in certs:
config.ca_certs = certs["ca_certs"]
if "alpn_protocols" in certs:
config.alpn_protocols = certs["alpn_protocols"]
config.bind = [f"{host}:0"]

ready_event = threading.Event()
shutdown_event = threading.Event()

Expand All @@ -62,7 +76,7 @@ def run_hypercorn_in_thread(
if not ready_event.is_set():
raise Exception("most likely failed to start server")

yield
yield typing.cast(int, parse_url(config.bind[0]).port)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
yield typing.cast(int, parse_url(config.bind[0]).port)
port = parse_url(config.bind[0]).port
assert port is not None
yield port

Copy link
Member Author

Choose a reason for hiding this comment

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

When can this happen?


shutdown_event.set()
future.result()
Expand Down
61 changes: 13 additions & 48 deletions dummyserver/testcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import threading
import typing

import hypercorn
import pytest
from tornado import httpserver, ioloop, web

Expand All @@ -25,7 +24,6 @@
)
from urllib3.connection import HTTPConnection
from urllib3.util.ssltransport import SSLTransport
from urllib3.util.url import parse_url


def consume_socket(
Expand Down Expand Up @@ -311,17 +309,10 @@ class HypercornDummyServerTestCase:
@classmethod
def setup_class(cls) -> None:
with contextlib.ExitStack() as stack:
config = hypercorn.Config()
if cls.certs:
config.certfile = cls.certs["certfile"]
config.keyfile = cls.certs["keyfile"]
config.verify_mode = cls.certs["cert_reqs"]
config.ca_certs = cls.certs["ca_certs"]
config.alpn_protocols = cls.certs["alpn_protocols"]
config.bind = [f"{cls.host}:0"]
stack.enter_context(run_hypercorn_in_thread(config, hypercorn_app))
cls.port = stack.enter_context(
run_hypercorn_in_thread(cls.host, cls.certs, hypercorn_app)
)
cls._stack = stack.pop_all()
cls.port = typing.cast(int, parse_url(config.bind[0]).port)

@classmethod
def teardown_class(cls) -> None:
Expand Down Expand Up @@ -367,47 +358,21 @@ class HypercornDummyProxyTestCase:
@classmethod
def setup_class(cls) -> None:
with contextlib.ExitStack() as stack:
http_server_config = hypercorn.Config()
http_server_config.bind = [f"{cls.http_host}:0"]
stack.enter_context(
run_hypercorn_in_thread(http_server_config, hypercorn_app)
cls.http_port = stack.enter_context(
run_hypercorn_in_thread(cls.http_host, None, hypercorn_app)
)
cls.http_port = typing.cast(int, parse_url(http_server_config.bind[0]).port)

https_server_config = hypercorn.Config()
https_server_config.certfile = cls.https_certs["certfile"]
https_server_config.keyfile = cls.https_certs["keyfile"]
https_server_config.verify_mode = cls.https_certs["cert_reqs"]
https_server_config.ca_certs = cls.https_certs["ca_certs"]
https_server_config.alpn_protocols = cls.https_certs["alpn_protocols"]
https_server_config.bind = [f"{cls.https_host}:0"]
stack.enter_context(
run_hypercorn_in_thread(https_server_config, hypercorn_app)
cls.https_port = stack.enter_context(
run_hypercorn_in_thread(cls.https_host, cls.https_certs, hypercorn_app)
)
cls.https_port = typing.cast(
int, parse_url(https_server_config.bind[0]).port
cls.proxy_port = stack.enter_context(
run_hypercorn_in_thread(cls.proxy_host, None, ProxyApp())
)

http_proxy_config = hypercorn.Config()
http_proxy_config.bind = [f"{cls.proxy_host}:0"]
stack.enter_context(run_hypercorn_in_thread(http_proxy_config, ProxyApp()))
cls.proxy_port = typing.cast(int, parse_url(http_proxy_config.bind[0]).port)

https_proxy_config = hypercorn.Config()
https_proxy_config.certfile = cls.https_certs["certfile"]
https_proxy_config.keyfile = cls.https_certs["keyfile"]
https_proxy_config.verify_mode = cls.https_certs["cert_reqs"]
https_proxy_config.ca_certs = cls.https_certs["ca_certs"]
https_proxy_config.alpn_protocols = cls.https_certs["alpn_protocols"]
https_proxy_config.bind = [f"{cls.proxy_host}:0"]
upstream_ca_certs = cls.https_certs.get("ca_certs")
stack.enter_context(
run_hypercorn_in_thread(https_proxy_config, ProxyApp(upstream_ca_certs))
)
cls.https_proxy_port = typing.cast(
int, parse_url(https_proxy_config.bind[0]).port
cls.https_proxy_port = stack.enter_context(
run_hypercorn_in_thread(
cls.proxy_host, cls.https_certs, ProxyApp(upstream_ca_certs)
)
)

cls._stack = stack.pop_all()

@classmethod
Expand Down
28 changes: 7 additions & 21 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import typing
from pathlib import Path

import hypercorn
import pytest
import trustme

Expand All @@ -16,7 +15,6 @@
from dummyserver.testcase import HTTPSHypercornDummyServerTestCase
from dummyserver.tornadoserver import HAS_IPV6
from urllib3.util import ssl_
from urllib3.util.url import parse_url

from .tz_stub import stub_timezone_ctx

Expand Down Expand Up @@ -80,12 +78,7 @@ def run_server_in_thread(
ca.cert_pem.write_to_path(ca_cert_path)
server_certs = _write_cert_to_dir(server_cert, tmpdir)

config = hypercorn.Config()
config.certfile = server_certs["certfile"]
config.keyfile = server_certs["keyfile"]
config.bind = [f"{host}:0"]
with run_hypercorn_in_thread(config, hypercorn_app):
port = typing.cast(int, parse_url(config.bind[0]).port)
with run_hypercorn_in_thread(host, server_certs, hypercorn_app) as port:
yield ServerConfig(scheme, host, port, ca_cert_path)


Expand All @@ -105,19 +98,12 @@ def run_server_and_proxy_in_thread(
proxy_certs = _write_cert_to_dir(proxy_cert, tmpdir, "proxy")

with contextlib.ExitStack() as stack:
server_config = hypercorn.Config()
server_config.certfile = server_certs["certfile"]
server_config.keyfile = server_certs["keyfile"]
server_config.bind = ["localhost:0"]
stack.enter_context(run_hypercorn_in_thread(server_config, hypercorn_app))
port = typing.cast(int, parse_url(server_config.bind[0]).port)

proxy_config = hypercorn.Config()
proxy_config.certfile = proxy_certs["certfile"]
proxy_config.keyfile = proxy_certs["keyfile"]
proxy_config.bind = [f"{proxy_host}:0"]
stack.enter_context(run_hypercorn_in_thread(proxy_config, ProxyApp()))
proxy_port = typing.cast(int, parse_url(proxy_config.bind[0]).port)
port = stack.enter_context(
run_hypercorn_in_thread("localhost", server_certs, hypercorn_app)
)
proxy_port = stack.enter_context(
run_hypercorn_in_thread(proxy_host, proxy_certs, ProxyApp())
)

yield (
ServerConfig(proxy_scheme, proxy_host, proxy_port, ca_cert_path),
Expand Down