From 0a38ce827a9416316dfbc8fb9a3fcb02297bdf9c Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Tue, 19 Mar 2024 13:52:11 -0400 Subject: [PATCH 1/7] Missed passing log_config to one conditional branch. --- tiled/commandline/_serve.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tiled/commandline/_serve.py b/tiled/commandline/_serve.py index b8a19c29c..f131360fd 100644 --- a/tiled/commandline/_serve.py +++ b/tiled/commandline/_serve.py @@ -223,7 +223,9 @@ async def walk_and_serve(): import uvicorn - config = uvicorn.Config(web_app, host=host, port=port) + config = uvicorn.Config( + web_app, host=host, port=port, log_config=log_config + ) server = uvicorn.Server(config) await server.serve() From a4af59d0ec18fa2c6f556a8881f9f5db3e11ee6b Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Wed, 20 Mar 2024 13:54:57 -0400 Subject: [PATCH 2/7] Fix critical regression in 'tiled serve directory' --- tiled/commandline/_serve.py | 83 ++++++++++++++++++++++++++----------- tiled/server/app.py | 7 +++- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/tiled/commandline/_serve.py b/tiled/commandline/_serve.py index f131360fd..e4a059c8f 100644 --- a/tiled/commandline/_serve.py +++ b/tiled/commandline/_serve.py @@ -1,3 +1,4 @@ +import os import re from pathlib import Path from typing import List, Optional @@ -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 = {} @@ -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, { @@ -199,15 +210,39 @@ def serve_directory( }, server_settings, ) + import functools + + import anyio + import httpcore + 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) + api_url = f"http://{host}:{port}/api/v1/" + + async def run_server(): + config = uvicorn.Config(web_app, host=host, port=port) + server = uvicorn.Server(config) + await server.serve() + if watch: - async def walk_and_serve(): - import anyio + async def serve_and_walk(): + server_task = asyncio.create_task(run_server()) + # Wait for server to start up, with a retry loop. + async with httpcore.AsyncConnectionPool(retries=5) as http: + await http.request("GET", api_url) + # 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, @@ -218,22 +253,24 @@ 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, log_config=log_config + async def serve_and_walk(): + server_task = asyncio.create_task(run_server()) + # Wait for server to start up, with a retry loop. + async with httpcore.AsyncConnectionPool(retries=5) as http: + await http.request("GET", api_url) + # 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) ) - server = uvicorn.Server(config) - await server.serve() - 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, @@ -241,14 +278,10 @@ async def walk_and_serve(): 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( diff --git a/tiled/server/app.py b/tiled/server/app.py index cf6be65d3..d3c8c6329 100644 --- a/tiled/server/app.py +++ b/tiled/server/app.py @@ -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)() @@ -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: From a09bc536fcebae782731a84a0e0d9279c582714b Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Tue, 26 Mar 2024 16:44:13 -0400 Subject: [PATCH 3/7] Address pydantic change making noise in server logs. --- tiled/server/schemas.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tiled/server/schemas.py b/tiled/server/schemas.py index 270078ee2..e3df83752 100644 --- a/tiled/server/schemas.py +++ b/tiled/server/schemas.py @@ -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 @@ -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 From 00614ec2902768dcbb016a9ebff79f770ff32afa Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Thu, 28 Mar 2024 13:37:30 -0400 Subject: [PATCH 4/7] Support running on a random port. --- tiled/commandline/_serve.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tiled/commandline/_serve.py b/tiled/commandline/_serve.py index e4a059c8f..1d52454e2 100644 --- a/tiled/commandline/_serve.py +++ b/tiled/commandline/_serve.py @@ -213,26 +213,34 @@ def serve_directory( import functools import anyio - import httpcore 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) - api_url = f"http://{host}:{port}/api/v1/" + config = uvicorn.Config(web_app, host=host, port=port) + server = uvicorn.Server(config) async def run_server(): - config = uvicorn.Config(web_app, host=host, port=port) - server = uvicorn.Server(config) 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 serve_and_walk(): server_task = asyncio.create_task(run_server()) - # Wait for server to start up, with a retry loop. - async with httpcore.AsyncConnectionPool(retries=5) as http: - await http.request("GET", api_url) + 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) @@ -260,9 +268,7 @@ async def serve_and_walk(): async def serve_and_walk(): server_task = asyncio.create_task(run_server()) - # Wait for server to start up, with a retry loop. - async with httpcore.AsyncConnectionPool(retries=5) as http: - await http.request("GET", api_url) + 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) From ce725530e69f60223bd80af915079e40e4e1d345 Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Thu, 28 Mar 2024 13:37:54 -0400 Subject: [PATCH 5/7] Test server CLI entrypoints. --- tiled/_tests/test_cli.py | 75 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tiled/_tests/test_cli.py diff --git a/tiled/_tests/test_cli.py b/tiled/_tests/test_cli.py new file mode 100644 index 000000000..d818bd678 --- /dev/null +++ b/tiled/_tests/test_cli.py @@ -0,0 +1,75 @@ +import re +import subprocess +import sys +import threading +from queue import Queue + +import httpx +import pytest + + +def run_cli(command): + "Run '/path/to/this/python -m ...'" + return subprocess.Popen( + [sys.executable, "-m"] + command.split(), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + +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() + process.terminate() + + +@pytest.mark.parametrize( + "args", + [ + "", + "--verbose", + "--api-key secret", + ], +) +def test_serve_directory(args, tmpdir): + "Test 'tiled serve directory ... with a variety of arguments." + process = run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) + 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." + process = run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) + check_server_readiness(process) From 30347d7f4f74ad3c614b2ec536a945b910d63b92 Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Thu, 28 Mar 2024 14:01:11 -0400 Subject: [PATCH 6/7] Clean up processes. --- tiled/_tests/test_cli.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tiled/_tests/test_cli.py b/tiled/_tests/test_cli.py index d818bd678..fc173d9f2 100644 --- a/tiled/_tests/test_cli.py +++ b/tiled/_tests/test_cli.py @@ -1,3 +1,4 @@ +import contextlib import re import subprocess import sys @@ -8,13 +9,16 @@ import pytest +@contextlib.contextmanager def run_cli(command): "Run '/path/to/this/python -m ...'" - return subprocess.Popen( + 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): @@ -58,8 +62,8 @@ def check_server_readiness(process): ) def test_serve_directory(args, tmpdir): "Test 'tiled serve directory ... with a variety of arguments." - process = run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) - check_server_readiness(process) + with run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) as process: + check_server_readiness(process) @pytest.mark.parametrize( @@ -71,5 +75,5 @@ def test_serve_directory(args, tmpdir): ) def test_serve_catalog_temp(args, tmpdir): "Test 'tiled serve catalog --temp ... with a variety of arguments." - process = run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) - check_server_readiness(process) + with run_cli(f"tiled serve directory {tmpdir!s} --port 0 " + args) as process: + check_server_readiness(process) From ab0037b9b4bf03a0c6b81d9def7e690b07704b44 Mon Sep 17 00:00:00 2001 From: Dan Allan Date: Thu, 28 Mar 2024 14:10:15 -0400 Subject: [PATCH 7/7] Remove extra process.terminate(). --- tiled/_tests/test_cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tiled/_tests/test_cli.py b/tiled/_tests/test_cli.py index fc173d9f2..f2f764a1c 100644 --- a/tiled/_tests/test_cli.py +++ b/tiled/_tests/test_cli.py @@ -49,7 +49,6 @@ 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() - process.terminate() @pytest.mark.parametrize(