Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions conformance/test/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ def pyqwest_client_kwargs(test_request: ClientCompatRequest) -> dict:
kwargs["http_version"] = PyQwestHTTPVersion.HTTP1
case HTTPVersion.HTTP_VERSION_2:
kwargs["http_version"] = PyQwestHTTPVersion.HTTP2
case HTTPVersion.HTTP_VERSION_3:
kwargs["http_version"] = PyQwestHTTPVersion.HTTP3
if test_request.server_tls_cert:
kwargs["tls_ca_cert"] = test_request.server_tls_cert
if test_request.HasField("client_tls_creds"):
Expand Down
1 change: 1 addition & 0 deletions conformance/test/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ features:
versions:
- HTTP_VERSION_1
- HTTP_VERSION_2
- HTTP_VERSION_3
protocols:
- PROTOCOL_CONNECT
- PROTOCOL_GRPC
Expand Down
69 changes: 40 additions & 29 deletions conformance/test/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import argparse
import asyncio
import contextlib
import os
import re
import signal
Expand All @@ -10,13 +11,15 @@
import sys
import time
from contextlib import ExitStack, closing
from pathlib import Path
from tempfile import NamedTemporaryFile
from typing import TYPE_CHECKING, Literal, TypeVar, get_args

# Needs to run before importing from connectrpc
import _cov_embed # noqa: F401
from _util import create_standard_streams
from gen.connectrpc.conformance.v1.config_pb2 import Code as ConformanceCode
from gen.connectrpc.conformance.v1.config_pb2 import HTTPVersion
from gen.connectrpc.conformance.v1.server_compat_pb2 import (
ServerCompatRequest,
ServerCompatResponse,
Expand All @@ -43,6 +46,7 @@
UnaryResponseDefinition,
)
from google.protobuf.any_pb2 import Any
from pyvoy import PyvoyServer

from connectrpc.code import Code
from connectrpc.compression.brotli import BrotliCompression
Expand Down Expand Up @@ -635,40 +639,47 @@ async def serve_pyvoy(
cafile: str | None,
port_future: asyncio.Future[int],
):
args = ["--port=0"]
if certfile:
args.append(f"--tls-cert={certfile}")
if keyfile:
args.append(f"--tls-key={keyfile}")
if cafile:
args.append(f"--tls-ca-cert={cafile}")
tls_cert = Path(certfile) if certfile else None
tls_key = Path(keyfile) if keyfile else None
tls_ca_cert = Path(cafile) if cafile else None
tls_port = 0 if tls_key else None

if mode == "sync":
args.append("--interface=wsgi")
args.append("server:wsgi_app")
interface = "wsgi"
app = "server:wsgi_app"
else:
args.append("server:asgi_app")

proc = await asyncio.create_subprocess_exec(
"pyvoy",
*args,
stderr=asyncio.subprocess.STDOUT,
stdout=asyncio.subprocess.PIPE,
env=_server_env(request),
)
stdout = proc.stdout
assert stdout is not None
stdout = _tee_to_stderr(stdout)
interface = "asgi"
app = "server:asgi_app"

async def start():
# Currently explicit env not accepted but it's safe to set it here.
os.environ["READ_MAX_BYTES"] = str(request.message_receive_limit)
Copy link
Member

Choose a reason for hiding this comment

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

Is this just so we pipe through the environment that was already going to the subprocess?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. While I thought it was a rare use case, I will add extra_env or something to PyvoyServer to simplify this one

async with PyvoyServer(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically switching to programmatic is a workaround for an issue that pyvoy only prints a single port even if there are several (for HTTP/3, when setting port=0, the TLS and QUIC port end up separate due to Envoy's behavior).

But I figured it cleans things up anyways, it's still a subprocess, just not subprocess of subprocess.

Copy link
Member

Choose a reason for hiding this comment

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

is that something you'd want to fix upstream in pyvoy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup intend to, though not to revert this back, I think programmatic works well here

Copy link
Member

Choose a reason for hiding this comment

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

yep, looks reasonable to me.

app,
port=0,
tls_port=tls_port,
interface=interface,
tls_key=tls_key,
tls_cert=tls_cert,
tls_ca_cert=tls_ca_cert,
) as server:
if (
request.http_version == HTTPVersion.HTTP_VERSION_3
and server.listener_port_quic
):
port = server.listener_port_quic
else:
port = server.listener_port_tls or server.listener_port
port_future.set_result(port)
await asyncio.Future() # run until cancelled

task = asyncio.create_task(start())
try:
async for line in stdout:
if b"listening on" in line:
port = int(line.strip().split(b"127.0.0.1:")[1])
port_future.set_result(port)
break
await _consume_log(stdout)
await task
except asyncio.CancelledError:
proc.terminate()
await proc.wait()
task.cancel()
with contextlib.suppress(asyncio.CancelledError):
await task


async def serve_uvicorn(
Expand Down
11 changes: 7 additions & 4 deletions conformance/test/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ def test_server_sync(server: str, cov: Coverage) -> None:
opts = []
match server:
case "gunicorn":
# gunicorn doesn't support HTTP/2
opts = ["--skip", "**/HTTPVersion:2/**"]
# gunicorn doesn't support HTTP/2 or 3
opts = ["--skip", "**/HTTPVersion:2/**", "--skip", "**/HTTPVersion:3/**"]

result = subprocess.run(
[
Expand Down Expand Up @@ -89,6 +89,9 @@ def test_server_async(server: str, cov: Coverage) -> None:
# daphne doesn't support h2c
"--skip",
"**/HTTPVersion:2/**/TLS:false/**",
# daphne doesn't support HTTP/3
"--skip",
"**/HTTPVersion:3/**",
# daphne seems to block on the request body so can't do full duplex even with h2,
# it only works with websockets
"--skip",
Expand All @@ -102,8 +105,8 @@ def test_server_async(server: str, cov: Coverage) -> None:
"gRPC Unexpected Requests/**",
]
case "uvicorn":
# uvicorn doesn't support HTTP/2
opts = ["--skip", "**/HTTPVersion:2/**"]
# uvicorn doesn't support HTTP/2 or 3
opts = ["--skip", "**/HTTPVersion:2/**", "--skip", "**/HTTPVersion:3/**"]
result = subprocess.run(
[
"go",
Expand Down