Skip to content

Commit

Permalink
Fix critical regression in tiled serve directory (bluesky#701)
Browse files Browse the repository at this point in the history
* Missed passing log_config to one conditional branch.

* Fix critical regression in 'tiled serve directory'

* Address pydantic change making noise in server logs.

* Support running on a random port.

* Test server CLI entrypoints.

* Clean up processes.

* Remove extra process.terminate().
  • Loading branch information
danielballan authored Mar 28, 2024
1 parent 33286ae commit 487e30f
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 30 deletions.
78 changes: 78 additions & 0 deletions tiled/_tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import contextlib
import re
import subprocess
import sys
import threading
from queue import Queue

import httpx
import pytest


@contextlib.contextmanager
def run_cli(command):
"Run '/path/to/this/python -m ...'"
process = subprocess.Popen(
[sys.executable, "-m"] + command.split(),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
yield process
process.terminate()


def scrape_server_url_from_logs(process):
"Scrape from server logs 'Uvicorn running on https://...'"

def target(queue):
pattern = re.compile(r"Uvicorn running on (\S*)")
while not process.poll():
line = process.stderr.readline()
if match := pattern.search(line.decode()):
break
url = match.group(1)
queue.put(url)

queue = Queue()
thread = threading.Thread(target=target, args=(queue,))
thread.start()
url = queue.get(timeout=10)
# If the server has an error starting up, the target() will
# never find a match, and a TimeoutError will be raised above.
# The thread will leak. This is the best reasonably simple,
# portable approach available.
thread.join()
return url


def check_server_readiness(process):
"Given a server process, check that it responds successfully to HTTP."
url = scrape_server_url_from_logs(process)
httpx.get(url).raise_for_status()


@pytest.mark.parametrize(
"args",
[
"",
"--verbose",
"--api-key secret",
],
)
def test_serve_directory(args, tmpdir):
"Test 'tiled serve directory ... with a variety of arguments."
with run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) as process:
check_server_readiness(process)


@pytest.mark.parametrize(
"args",
[
"",
"--api-key secret",
],
)
def test_serve_catalog_temp(args, tmpdir):
"Test 'tiled serve catalog --temp ... with a variety of arguments."
with run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) as process:
check_server_readiness(process)
89 changes: 65 additions & 24 deletions tiled/commandline/_serve.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import re
from pathlib import Path
from typing import List, Optional
Expand Down Expand Up @@ -140,7 +141,7 @@ def serve_directory(
asyncio.run(initialize_database(engine))
stamp_head(ALEMBIC_INI_TEMPLATE_PATH, ALEMBIC_DIR, database)

from ..catalog import from_uri
from ..catalog import from_uri as catalog_from_uri
from ..server.app import build_app, print_admin_api_key_if_generated

server_settings = {}
Expand Down Expand Up @@ -182,15 +183,25 @@ def serve_directory(
)
mimetype, obj_ref = match.groups()
adapters_by_mimetype[mimetype] = obj_ref
catalog_adapter = from_uri(
catalog_adapter = catalog_from_uri(
database,
readable_storage=[directory],
adapters_by_mimetype=adapters_by_mimetype,
)
typer.echo(f"Indexing '{directory}' ...")
if verbose:
register_logger.addHandler(StreamHandler())
register_logger.setLevel("INFO")
# Set the API key manually here, rather than letting the server do it,
# so that we can pass it to the client.
generated = False
if api_key is None:
api_key = os.getenv("TILED_SINGLE_USER_API_KEY")
if api_key is None:
import secrets

api_key = secrets.token_hex(32)
generated = True

web_app = build_app(
catalog_adapter,
{
Expand All @@ -199,15 +210,47 @@ def serve_directory(
},
server_settings,
)
import functools

import anyio
import uvicorn

from ..client import from_uri as client_from_uri

print_admin_api_key_if_generated(web_app, host=host, port=port, force=generated)
config = uvicorn.Config(web_app, host=host, port=port)
server = uvicorn.Server(config)

async def run_server():
await server.serve()

async def wait_for_server():
"Wait for server to start up, or raise TimeoutError."
for _ in range(100):
await asyncio.sleep(0.1)
if server.started:
break
else:
raise TimeoutError("Server did not start in 10 seconds.")
host, port = server.servers[0].sockets[0].getsockname()
api_url = f"http://{host}:{port}/api/v1/"
return api_url

if watch:

async def walk_and_serve():
import anyio
async def serve_and_walk():
server_task = asyncio.create_task(run_server())
api_url = await wait_for_server()
# When we add an AsyncClient for Tiled, use that here.
client = await anyio.to_thread.run_sync(
functools.partial(client_from_uri, api_url, api_key=api_key)
)

typer.echo(f"Server is up. Indexing files in {directory}...")
event = anyio.Event()
asyncio.create_task(
watch_(
catalog_adapter,
client,
directory,
initial_walk_complete_event=event,
mimetype_detection_hook=mimetype_detection_hook,
Expand All @@ -218,35 +261,33 @@ async def walk_and_serve():
)
)
await event.wait()
typer.echo("Initial indexing complete. Starting server...")
print_admin_api_key_if_generated(web_app, host=host, port=port)
typer.echo("Initial indexing complete. Watching for changes...")
await server_task

import uvicorn
else:

config = uvicorn.Config(web_app, host=host, port=port)
server = uvicorn.Server(config)
await server.serve()
async def serve_and_walk():
server_task = asyncio.create_task(run_server())
api_url = await wait_for_server()
# When we add an AsyncClient for Tiled, use that here.
client = await anyio.to_thread.run_sync(
functools.partial(client_from_uri, api_url, api_key=api_key)
)

asyncio.run(walk_and_serve())
else:
asyncio.run(
register(
catalog_adapter,
typer.echo(f"Server is up. Indexing files in {directory}...")
await register(
client,
directory,
mimetype_detection_hook=mimetype_detection_hook,
mimetypes_by_file_ext=mimetypes_by_file_ext,
adapters_by_mimetype=adapters_by_mimetype,
walkers=walkers,
key_from_filename=key_from_filename,
)
)

typer.echo("Indexing complete. Starting server...")
print_admin_api_key_if_generated(web_app, host=host, port=port)

import uvicorn
typer.echo("Indexing complete.")
await server_task

uvicorn.run(web_app, host=host, port=port, log_config=log_config)
asyncio.run(serve_and_walk())


def serve_catalog(
Expand Down
7 changes: 5 additions & 2 deletions tiled/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,10 @@ def __getattr__(name):
raise AttributeError(name)


def print_admin_api_key_if_generated(web_app, host, port):
def print_admin_api_key_if_generated(
web_app: FastAPI, host: str, port: int, force: bool = False
):
"Print message to stderr with API key if server-generated (or force=True)."
host = host or "127.0.0.1"
port = port or 8000
settings = web_app.dependency_overrides.get(get_settings, get_settings)()
Expand All @@ -972,7 +975,7 @@ def print_admin_api_key_if_generated(web_app, host, port):
""",
file=sys.stderr,
)
if (not authenticators) and settings.single_user_api_key_generated:
if (not authenticators) and (force or settings.single_user_api_key_generated):
print(
f"""
Navigate a web browser or connect a Tiled client to:
Expand Down
6 changes: 2 additions & 4 deletions tiled/server/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Error(pydantic.BaseModel):
message: str


class Response(pydantic.generics.GenericModel, Generic[DataT, LinksT, MetaT]):
class Response(pydantic.BaseModel, Generic[DataT, LinksT, MetaT]):
data: Optional[DataT]
error: Optional[Error] = None
links: Optional[LinksT] = None
Expand Down Expand Up @@ -243,9 +243,7 @@ class ContainerMeta(pydantic.BaseModel):
count: int


class Resource(
pydantic.generics.GenericModel, Generic[AttributesT, ResourceLinksT, ResourceMetaT]
):
class Resource(pydantic.BaseModel, Generic[AttributesT, ResourceLinksT, ResourceMetaT]):
"A JSON API Resource"
id: Union[str, uuid.UUID]
attributes: AttributesT
Expand Down

0 comments on commit 487e30f

Please sign in to comment.