The shipped config could not load on a machine without a .env
config/default.yaml reads its camera's host, username and password from
${ENV}. Unset placeholders parse as YAML null, and CameraConfig had no
_normalize_blanks - the guard ApiSection and EmailSection have had all
along - so loading it raised three pydantic errors and the engine would
not start at all. On a developer's checkout .env is right there, which is
why this survived: the failing machine is every machine the product is
actually installed on, and installer/build.ps1 runs this suite, so the
Windows build would have failed on a fresh clone.
The normalisation is field-by-field, never a blanket None -> "": `webcam`
is an Optional[int] whose None means "this is not a webcam", and `tuning`
is a nested model. Sweeping either trades one validation error for
another - which it did, on the first attempt.
Second bug behind the same line: that camera entry would then have been
SEEDED into a fresh install, giving a shop a camera called cam1 that
nobody added, retrying a connection to "" forever, with the first task on
a new PC being to work out what it was. CameraConfig.addressed() says
what a camera entry needs to be one, and seed() drops the rest.
Found by cloning the repository into a temp directory and running the
tests there. Nothing in a working tree can find this class of bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HViLj9gYNRtSr7YVZmW5sn
This commit is contained in:
@@ -182,13 +182,22 @@ class CameraStore:
|
|||||||
|
|
||||||
After that the store is authoritative — otherwise a camera the user
|
After that the store is authoritative — otherwise a camera the user
|
||||||
deleted in the UI would reappear on every restart.
|
deleted in the UI would reappear on every restart.
|
||||||
|
|
||||||
|
Entries with no address are dropped rather than imported. The bundled
|
||||||
|
config declares one whose host comes from the environment, which is
|
||||||
|
right for a checkout with a .env and resolves to nothing on every
|
||||||
|
machine the product is actually installed on. Seeding that would put a
|
||||||
|
camera the shop never added into a brand new install, permanently
|
||||||
|
failing to connect to "" — and the first thing they would have to do is
|
||||||
|
work out what it was and delete it.
|
||||||
"""
|
"""
|
||||||
with self._lock:
|
with self._lock:
|
||||||
if self.path.exists() or not cameras:
|
usable = [c for c in cameras if c.addressed()]
|
||||||
|
if self.path.exists() or not usable:
|
||||||
return False
|
return False
|
||||||
for cam in cameras:
|
for cam in usable:
|
||||||
self._cameras[cam.id] = cam
|
self._cameras[cam.id] = cam
|
||||||
self._save()
|
self._save()
|
||||||
log.info("seeded %d camera(s) from YAML into %s",
|
log.info("seeded %d camera(s) from YAML into %s",
|
||||||
len(cameras), self.path)
|
len(usable), self.path)
|
||||||
return True
|
return True
|
||||||
|
|||||||
@@ -55,6 +55,43 @@ class CameraConfig(BaseModel):
|
|||||||
max_width: int = 1280 # frames wider than this are downscaled at ingest
|
max_width: int = 1280 # frames wider than this are downscaled at ingest
|
||||||
tuning: CameraTuning = CameraTuning()
|
tuning: CameraTuning = CameraTuning()
|
||||||
|
|
||||||
|
@model_validator(mode="before")
|
||||||
|
@classmethod
|
||||||
|
def _normalize_blanks(cls, values):
|
||||||
|
# Unset ${ENV} placeholders parse as YAML null, not "" - the same trap
|
||||||
|
# ApiSection and EmailSection already guard against, and this section
|
||||||
|
# did not. The bundled config/default.yaml reads its camera's host,
|
||||||
|
# username and password from the environment, so on any machine
|
||||||
|
# WITHOUT a .env - which is every machine the product is installed on -
|
||||||
|
# loading it raised three pydantic errors and the engine would not
|
||||||
|
# start at all. Found by cloning the repository and running the tests.
|
||||||
|
if not isinstance(values, dict):
|
||||||
|
return values
|
||||||
|
# Named fields only, never a blanket None -> "". `webcam` is an
|
||||||
|
# Optional[int] whose None is meaningful ("this is not a webcam"), and
|
||||||
|
# `tuning` is a nested model; sweeping either into "" trades one
|
||||||
|
# validation error for another.
|
||||||
|
values = dict(values)
|
||||||
|
for key in ("url", "host", "path", "username", "password"):
|
||||||
|
if values.get(key) is None:
|
||||||
|
values[key] = ""
|
||||||
|
for key, default in (("port", 554), ("max_width", 1280), ("path", "/")):
|
||||||
|
if values.get(key) in (None, ""):
|
||||||
|
values[key] = default
|
||||||
|
return values
|
||||||
|
|
||||||
|
def addressed(self) -> bool:
|
||||||
|
"""True when this entry actually names something to connect to.
|
||||||
|
|
||||||
|
The bundled config declares a camera whose address comes from the
|
||||||
|
environment, for development. On an installed PC there is no such
|
||||||
|
environment, so that entry resolves to a camera with no address - and
|
||||||
|
seeding it would put a permanently-failing camera nobody added into
|
||||||
|
every fresh install, retrying a connection to "" forever. An entry with
|
||||||
|
no url, no host and no webcam is not a camera.
|
||||||
|
"""
|
||||||
|
return bool(self.url or self.host) or self.webcam is not None
|
||||||
|
|
||||||
def source(self) -> "str | int":
|
def source(self) -> "str | int":
|
||||||
"""Resolved capture source: webcam index, explicit URL, or a URL
|
"""Resolved capture source: webcam index, explicit URL, or a URL
|
||||||
built from parts with percent-encoded credentials."""
|
built from parts with percent-encoded credentials."""
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
"""The shipped YAML carries thresholds measured on the deployment site; the
|
"""The shipped YAML carries thresholds measured on the deployment site; the
|
||||||
pydantic defaults must not drift away from them (a trimmed config would then
|
pydantic defaults must not drift away from them (a trimmed config would then
|
||||||
silently re-admit the false positives those values were tuned to reject)."""
|
silently re-admit the false positives those values were tuned to reject)."""
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from behavision.config import (ApiSection, Config, DetectionSection,
|
from behavision.config import (ApiSection, Config, DetectionSection,
|
||||||
@@ -123,3 +125,62 @@ def test_no_usable_model_fails_loudly(tmp_path):
|
|||||||
(tmp_path / "w600k_mbf.onnx").write_bytes(b"garbage")
|
(tmp_path / "w600k_mbf.onnx").write_bytes(b"garbage")
|
||||||
with pytest.raises(FileNotFoundError, match="no usable recognition model"):
|
with pytest.raises(FileNotFoundError, match="no usable recognition model"):
|
||||||
ArcFaceEncoder(tmp_path)
|
ArcFaceEncoder(tmp_path)
|
||||||
|
|
||||||
|
|
||||||
|
def _no_env(monkeypatch, tmp_path):
|
||||||
|
"""Make the process look like an installed PC rather than a checkout.
|
||||||
|
|
||||||
|
`load_config` calls `load_dotenv` first, so simply unsetting the variables
|
||||||
|
is not enough in a working tree - the .env sitting next to the code puts
|
||||||
|
them straight back. Removing the .env from the picture is the condition
|
||||||
|
these tests are actually about.
|
||||||
|
"""
|
||||||
|
import behavision.paths as paths
|
||||||
|
monkeypatch.setattr(paths, "env_file", lambda: None)
|
||||||
|
for var in ("BEHAVISION_CAM1_HOST", "BEHAVISION_CAM1_USERNAME",
|
||||||
|
"BEHAVISION_CAM1_PASSWORD", "BEHAVISION_API_USER",
|
||||||
|
"BEHAVISION_API_PASSWORD", "BEHAVISION_WEBHOOK_URL"):
|
||||||
|
monkeypatch.delenv(var, raising=False)
|
||||||
|
monkeypatch.setenv("BEHAVISION_DATA_DIR", str(tmp_path))
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_bundled_config_loads_with_no_environment_at_all(tmp_path, monkeypatch):
|
||||||
|
"""What a freshly installed PC looks like: config/default.yaml, and nothing
|
||||||
|
else.
|
||||||
|
|
||||||
|
Every `${VAR}` in that file then resolves to YAML null. The engine's own
|
||||||
|
sections already guarded against that; the camera section did not, so the
|
||||||
|
shipped config raised three pydantic errors on host, username and password
|
||||||
|
and the engine would not start at all on any machine without a developer's
|
||||||
|
.env - which is every machine the product is installed on. It was found by
|
||||||
|
cloning the repository and running this suite, never by working in a
|
||||||
|
checkout where .env is right there.
|
||||||
|
"""
|
||||||
|
_no_env(monkeypatch, tmp_path)
|
||||||
|
|
||||||
|
root = Path(__file__).resolve().parent.parent
|
||||||
|
cfg = load_config(root / "config" / "default.yaml")
|
||||||
|
|
||||||
|
assert cfg.api.username == ""
|
||||||
|
assert cfg.api.port == 8010
|
||||||
|
for cam in cfg.cameras:
|
||||||
|
assert cam.host == "" and cam.username == "" and cam.password == ""
|
||||||
|
assert cam.port == 554
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_camera_with_no_address_is_not_seeded(tmp_path, monkeypatch):
|
||||||
|
"""The bundled config declares a camera whose address comes from the
|
||||||
|
environment. On an installed PC there is none, so seeding it would put a
|
||||||
|
camera the shop never added into a brand new install, permanently failing
|
||||||
|
to connect to "" - and their first task would be working out what it was."""
|
||||||
|
from behavision.cameras import CameraStore
|
||||||
|
|
||||||
|
_no_env(monkeypatch, tmp_path)
|
||||||
|
|
||||||
|
root = Path(__file__).resolve().parent.parent
|
||||||
|
cfg = load_config(root / "config" / "default.yaml")
|
||||||
|
|
||||||
|
store = CameraStore(tmp_path / "cameras.json")
|
||||||
|
assert store.seed(cfg.cameras) is False
|
||||||
|
assert store.list() == []
|
||||||
|
assert not (tmp_path / "cameras.json").exists()
|
||||||
|
|||||||
Reference in New Issue
Block a user