diff --git a/AGENTS.md b/AGENTS.md index cf26c812..c7877d5d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,6 +54,14 @@ on 401/429) for the REST API, and a socket.io *client* for status updates and lo - **Annotator** — thin: it forwards the loop frontend's `handle_user_input` events into `AnnotatorLogic` and keeps a per-frontend history. +`helpers/entrypoint.py` holds what every node's `main.py` repeats: `node_parser` builds a +configargparse parser with `--host`/`--port`, `run_node` starts uvicorn. A setting is a flag +*and* an environment variable from one declaration — `--conf-threshold` reads +`CONF_THRESHOLD`. The exception is `--host`/`--port`, which read `NODE_HOST`/`NODE_PORT`: the +bare `HOST` already means the loop's address, and a node adopting it would hand it to uvicorn +and fail to bind. A node that used to require a prefix passes `legacy_env_prefix`, and the +prefixed names keep working with a warning. + `detector/postprocess.py` and `detector/geometry.py` hold the parts of a detector that do *not* depend on the model: confidence filtering, per-class NMS, box/point clipping, and turning predictions into the loop's dataclasses. A node should import them rather than write its own — diff --git a/learning_loop_node/helpers/entrypoint.py b/learning_loop_node/helpers/entrypoint.py new file mode 100644 index 00000000..830c89ec --- /dev/null +++ b/learning_loop_node/helpers/entrypoint.py @@ -0,0 +1,75 @@ +"""The boilerplate every node's ``main.py`` repeats. + +A node entry point always does the same four things: read a handful of settings, build the +logic object, construct the node, and hand it to uvicorn. Only the middle two are the node's +own, so :func:`node_parser` and :func:`run_node` cover the rest. + +Settings come from a flag *or* an environment variable, because a node is configured on the +command line while developing and through the container environment in deployment. One +declaration gives both: ``--conf-threshold`` reads ``CONF_THRESHOLD``. + +``--host`` and ``--port`` are the exception — they read ``NODE_HOST`` and ``NODE_PORT`` rather +than the names their flags imply, because the bare ``HOST`` already means *the loop's address* +and a node adopting it would hand it to uvicorn and fail to bind. +""" + +import logging +import os +from argparse import Action, Namespace + +import configargparse +import uvicorn + + +def node_parser(*, description: str, legacy_env_prefix: str = '') -> configargparse.ArgumentParser: + """Build the parser for a node, pre-loaded with the settings every node has. + + :param legacy_env_prefix: A prefix an earlier version of this node required, e.g. + ``'DFINE_DETECTOR_'``. Prefixed names are still honoured, with a warning, so a + deployment keeps working until it is updated. Leave empty for a node that has always + read unprefixed names. + """ + parser = _NodeArgumentParser(description=description, legacy_env_prefix=legacy_env_prefix) + parser.add_argument('--host', default='0.0.0.0', env_var='NODE_HOST', + help='Host interface to bind to') + parser.add_argument('--port', type=int, default=80, env_var='NODE_PORT', help='Port to bind to') + return parser + + +def run_node(app: str, args: Namespace) -> None: + """Serve the node. + + :param app: Import string of the node object, conventionally ``'main:node'``. + """ + reload = os.getenv('UVICORN_RELOAD', 'FALSE').lower() in ('true', '1') + logging.info('Uvicorn reload is set to: %s', reload) + uvicorn.run(app, host=args.host, port=args.port, lifespan='on', reload=reload) + + +class _NodeArgumentParser(configargparse.ArgumentParser): + """Parser whose every setting is also an environment variable named after its flag.""" + + def __init__(self, *, description: str, legacy_env_prefix: str) -> None: + super().__init__(description=description) + self.legacy_env_prefix = legacy_env_prefix + + def add_argument(self, *args, **kwargs) -> Action: # type: ignore[override] + action = super().add_argument(*args, **kwargs) + if getattr(action, 'env_var', None) is None and action.dest != 'help': + action.env_var = action.dest.upper() + return action + + def parse_args(self, *args, **kwargs) -> Namespace: # type: ignore[override] + self._adopt_legacy_env_vars() + return super().parse_args(*args, **kwargs) + + def _adopt_legacy_env_vars(self) -> None: + if not self.legacy_env_prefix: + return + for action in self._actions: + name = getattr(action, 'env_var', None) + legacy = self.legacy_env_prefix + name if name else None + if not legacy or name in os.environ or legacy not in os.environ: + continue + os.environ[name] = os.environ[legacy] + logging.warning('%s is deprecated and will stop being read; set %s instead', legacy, name) diff --git a/learning_loop_node/tests/unit/test_entrypoint.py b/learning_loop_node/tests/unit/test_entrypoint.py new file mode 100644 index 00000000..a478563b --- /dev/null +++ b/learning_loop_node/tests/unit/test_entrypoint.py @@ -0,0 +1,71 @@ +import pytest + +from ...helpers.entrypoint import node_parser + +MANAGED = ('WEIGHT_TYPE', 'DFINE_DETECTOR_WEIGHT_TYPE', 'HOST', 'NODE_HOST', 'NODE_PORT', 'PORT') + + +@pytest.fixture(autouse=True) +def clean_env(monkeypatch: pytest.MonkeyPatch): + """Every test starts without the variables it is about to set.""" + for name in MANAGED: + monkeypatch.delenv(name, raising=False) + + +def _parser(**kwargs): + parser = node_parser(description='a node', **kwargs) + parser.add_argument('--weight-type', default='FP16') + return parser + + +def test_every_node_gets_a_host_and_a_port(): + args = _parser().parse_args([]) + assert (args.host, args.port) == ('0.0.0.0', 80) + + +def test_a_flag_beats_everything(): + args = _parser().parse_args(['--weight-type', 'FP32']) + assert args.weight_type == 'FP32' + + +def test_a_setting_is_read_from_the_variable_named_after_its_flag(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('WEIGHT_TYPE', 'FP32') + assert _parser().parse_args([]).weight_type == 'FP32' + + +def test_the_loop_own_host_is_never_mistaken_for_the_bind_address(monkeypatch: pytest.MonkeyPatch): + """HOST is the loop's address. Binding uvicorn to it would leave the node unreachable.""" + monkeypatch.setenv('HOST', 'preview.learning-loop.ai') + assert _parser().parse_args([]).host == '0.0.0.0' + + +def test_the_bind_address_has_a_name_of_its_own(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('NODE_HOST', '127.0.0.1') + monkeypatch.setenv('NODE_PORT', '8080') + args = _parser().parse_args([]) + assert (args.host, args.port) == ('127.0.0.1', 8080) + + +def test_a_node_that_used_a_prefix_still_reads_it(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('DFINE_DETECTOR_WEIGHT_TYPE', 'FP32') + parser = _parser(legacy_env_prefix='DFINE_DETECTOR_') + assert parser.parse_args([]).weight_type == 'FP32' + + +def test_the_prefixed_name_warns_which_one_to_use_instead(monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture): + monkeypatch.setenv('DFINE_DETECTOR_WEIGHT_TYPE', 'FP32') + _parser(legacy_env_prefix='DFINE_DETECTOR_').parse_args([]) + assert 'DFINE_DETECTOR_WEIGHT_TYPE' in caplog.text + assert 'WEIGHT_TYPE' in caplog.text + + +def test_the_current_name_wins_over_the_prefixed_one(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('DFINE_DETECTOR_WEIGHT_TYPE', 'FP32') + monkeypatch.setenv('WEIGHT_TYPE', 'FP16') + assert _parser(legacy_env_prefix='DFINE_DETECTOR_').parse_args([]).weight_type == 'FP16' + + +def test_a_node_without_a_legacy_prefix_ignores_prefixed_names(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('DFINE_DETECTOR_WEIGHT_TYPE', 'FP32') + assert _parser().parse_args([]).weight_type == 'FP16' diff --git a/pyproject.toml b/pyproject.toml index 47409dff..d05073f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,6 +13,7 @@ dependencies = [ "python-socketio>=5.16.2,<6.0.0", "aiofiles>=0.7.0", "python-multipart>=0.0.31", + "configargparse>=1.7.1", "psutil>=5.9.0,<8.0.0", "numpy>=2.0,<3.0", "Pillow>=12.3.0,<13.0.0", diff --git a/uv.lock b/uv.lock index da0c0bf7..1b373e19 100644 --- a/uv.lock +++ b/uv.lock @@ -449,6 +449,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, ] +[[package]] +name = "configargparse" +version = "1.7.5" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/3f/0b/30328302903c55218ffc5199646d0e9d28348ff26c02ba77b2ffc58d294a/configargparse-1.7.5.tar.gz", hash = "sha256:e3f9a7bb6be34d66b2e3c4a2f58e3045f8dfae47b0dc039f87bcfaa0f193fb0f", size = 53548, upload-time = "2026-03-11T02:19:38.144Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fe/19/3ba5e1b0bcc7b91aeab6c258afd70e4907d220fed3972febe38feb40db30/configargparse-1.7.5-py3-none-any.whl", hash = "sha256:1e63fdffedf94da9cd435fc13a1cd24777e76879dd2343912c1f871d4ac8c592", size = 27692, upload-time = "2026-03-11T02:19:36.442Z" }, +] + [[package]] name = "dacite" version = "1.9.2" @@ -781,6 +790,7 @@ source = { virtual = "." } dependencies = [ { name = "aiofiles" }, { name = "aiohttp" }, + { name = "configargparse" }, { name = "dacite" }, { name = "fastapi" }, { name = "httpx" }, @@ -813,6 +823,7 @@ requires-dist = [ { name = "aiofiles", specifier = ">=0.7.0" }, { name = "aiohttp", specifier = ">=3.14.3,<4.0.0" }, { name = "autopep8", marker = "extra == 'dev'", specifier = ">=2.0.2,<3.0.0" }, + { name = "configargparse", specifier = ">=1.7.1" }, { name = "dacite", specifier = ">=1.8.1,<2.0.0" }, { name = "debugpy", marker = "extra == 'dev'", specifier = ">=1.6.7.post1,<2.0.0" }, { name = "fastapi", specifier = ">=0.135,<1.0" },