Compare commits

...

3 Commits

Author SHA1 Message Date
didericis-claude b05775b581 fix(backend): fix pyright errors in lazy-load implementation
lint / lint (push) Successful in 2m13s
test / unit (pull_request) Successful in 1m6s
test / integration (pull_request) Successful in 24s
test / coverage (pull_request) Successful in 1m18s
- Rename _BACKENDS → _backends: pyright treats uppercase module-level
  names as constants and flags the reassignment in _get_backends() as
  reportConstantRedefinition; lowercase avoids this.
- Add TYPE_CHECKING guard importing CommitCancelled/Freezer/get_freezer
  from .freeze: pyright cannot see module-level __getattr__ bindings, so
  reportUnsupportedDunderAll fired for those three __all__ entries; the
  guard makes them visible to the type checker without running at import
  time.
- Update test_backend_selection.py to patch _backends (lowercase).
2026-07-14 09:07:21 +00:00
didericis-claude 2a2669b69f fix(backend): silence pylint false positives from lazy-load pattern
lint / lint (push) Failing after 2m17s
test / unit (pull_request) Successful in 1m7s
test / integration (pull_request) Successful in 24s
test / coverage (pull_request) Successful in 1m18s
`undefined-all-variable` fires on CommitCancelled / Freezer / get_freezer
in __all__ because pylint can't see module-level __getattr__ bindings;
`global-statement` fires on the _BACKENDS singleton setter. Both are
intentional patterns — add inline disables rather than suppress globally.
2026-07-14 08:21:42 +00:00
didericis-claude 8ae6561b33 perf: lazy-load backend modules and consolidate docker subprocess helpers
lint / lint (push) Failing after 2m7s
Importing backend.docker.util previously triggered eager loading of all
three backend packages (~76 modules) because backend/__init__.py imported
DockerBottleBackend, FirecrackerBottleBackend, and MacosContainerBottleBackend
at module scope. This made the module prohibitively expensive to import
from the orchestrator layer and elsewhere.

The three backend imports are now deferred into _get_backends(), which
loads all three on first call and caches the result in the module-level
_BACKENDS variable (initially None). Module-level __getattr__ exposes
backend classes and freeze symbols lazily for existing import/patch sites.

backend/docker/util.py raw subprocess.run(["docker", ...]) calls are
replaced with the shared run_docker primitive from docker_cmd, eliminating
the duplication between the backend and orchestrator implementations.
_silent_run() is removed; image_exists() is inlined directly onto
run_docker. The commit_container test is updated to patch run_docker
instead of subprocess.run.
2026-07-14 08:15:19 +00:00
4 changed files with 92 additions and 76 deletions
+73 -31
View File
@@ -40,7 +40,7 @@ from abc import ABC, abstractmethod
from contextlib import AbstractContextManager
from dataclasses import dataclass
from pathlib import Path
from typing import Any, Generic, Sequence, TypeVar
from typing import TYPE_CHECKING, Any, Generic, Sequence, TypeVar
from ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provision_plan
from ..egress import EgressPlan
@@ -54,6 +54,9 @@ from ..workspace import WorkspacePlan, workspace_plan
from .print_util import print_multi, visible_agent_env_names
from .util import host_skill_dir
if TYPE_CHECKING:
from .freeze import CommitCancelled, Freezer, get_freezer
@dataclass(frozen=True)
class BottleSpec:
@@ -572,28 +575,63 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
Not called by the launch path or the test suite."""
# Import concrete backend classes AFTER the base types are defined, so
# each backend module can pull BottleSpec / BottlePlan / BottleBackend
# via `from . import ...` without hitting a partially-initialized module.
from .docker import DockerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position
from .firecracker import FirecrackerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position
from .macos_container import MacosContainerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position
# Freezer is imported after the backend classes for the same reason:
# Freezer.commit_slug constructs ActiveAgent, which must be fully
# defined first.
from .freeze import CommitCancelled, Freezer, get_freezer # noqa: E402 # pylint: disable=wrong-import-position
# _backends is None until the first call to _get_backends(), at which
# point all three concrete backend classes are imported and instantiated.
# Keeping the imports out of module scope means that importing any
# backend sub-module (e.g. `backend.docker.util`) no longer drags the
# firecracker and macos-container implementations into memory.
#
# Tests may replace _backends with a {name: fake} dict via patch.object;
# _get_backends() returns the current module-level value as-is when it
# is not None, so test fakes take effect without triggering real imports.
_backends: dict[str, BottleBackend[Any, Any]] | None = None
# The dict is heterogeneous: each value is a BottleBackend specialized
# over its own plan type. Concrete plan types are erased here because
# the registry is selected at runtime and the CLI only needs the
# unparameterized methods (prepare → plan → launch(plan), cleanup, etc.).
_BACKENDS: dict[str, BottleBackend[Any, Any]] = {
"docker": DockerBottleBackend(),
"firecracker": FirecrackerBottleBackend(),
"macos-container": MacosContainerBottleBackend(),
}
def _get_backends() -> dict[str, BottleBackend[Any, Any]]:
"""Return the registry of all backend instances, loading lazily on first call."""
global _backends # pylint: disable=global-statement
if _backends is None:
from .docker import DockerBottleBackend
from .firecracker import FirecrackerBottleBackend
from .macos_container import MacosContainerBottleBackend
_backends = {
"docker": DockerBottleBackend(),
"firecracker": FirecrackerBottleBackend(),
"macos-container": MacosContainerBottleBackend(),
}
return _backends
def __getattr__(name: str) -> Any:
"""Lazily surface concrete backend classes and freeze symbols at the
package level so existing `from bot_bottle.backend import X` and
`patch.object(backend_mod, X, ...)` call-sites keep working without
forcing an import of every backend at module-init time."""
if name == "DockerBottleBackend":
from .docker import DockerBottleBackend
globals()[name] = DockerBottleBackend
return DockerBottleBackend
if name == "FirecrackerBottleBackend":
from .firecracker import FirecrackerBottleBackend
globals()[name] = FirecrackerBottleBackend
return FirecrackerBottleBackend
if name == "MacosContainerBottleBackend":
from .macos_container import MacosContainerBottleBackend
globals()[name] = MacosContainerBottleBackend
return MacosContainerBottleBackend
if name == "CommitCancelled":
from .freeze import CommitCancelled
globals()[name] = CommitCancelled
return CommitCancelled
if name == "Freezer":
from .freeze import Freezer
globals()[name] = Freezer
return Freezer
if name == "get_freezer":
from .freeze import get_freezer
globals()[name] = get_freezer
return get_freezer
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
def get_bottle_backend(
@@ -611,10 +649,11 @@ def get_bottle_backend(
Dies with a pointer at the known backends if the chosen name
isn't implemented."""
resolved = name or os.environ.get("BOT_BOTTLE_BACKEND") or _default_backend_name()
if resolved not in _BACKENDS:
known = ", ".join(sorted(_BACKENDS))
backends = _get_backends()
if resolved not in backends:
known = ", ".join(sorted(backends))
die(f"unknown backend {resolved!r}; known backends: {known}")
return _BACKENDS[resolved]
return backends[resolved]
def _default_backend_name() -> str:
@@ -624,16 +663,17 @@ def _default_backend_name() -> str:
# `firecracker` binary isn't installed yet: selecting it here routes
# start through firecracker's preflight, which prints an install
# pointer, instead of silently falling back to docker.
from .firecracker import FirecrackerBottleBackend
if FirecrackerBottleBackend.is_host_capable():
return "firecracker"
return "docker"
def known_backend_names() -> tuple[str, ...]:
"""Sorted tuple of all backend keys in `_BACKENDS`. Used by
"""Sorted tuple of all backend keys in `_get_backends()`. Used by
argparse (`--backend` choices) and the dashboard's backend
picker."""
return tuple(sorted(_BACKENDS))
return tuple(sorted(_get_backends()))
def has_backend(name: str) -> bool:
@@ -645,9 +685,10 @@ def has_backend(name: str) -> bool:
Returns False for unknown names so callers can pass
arbitrary input without separate validation."""
if name not in _BACKENDS:
backends = _get_backends()
if name not in backends:
return False
return _BACKENDS[name].is_available()
return backends[name].is_available()
def enumerate_active_agents() -> list[ActiveAgent]:
@@ -663,10 +704,11 @@ def enumerate_active_agents() -> list[ActiveAgent]:
deterministic tiebreaker. Agents with missing metadata
(`started_at == ""`) sort first."""
out: list[ActiveAgent] = []
for name in known_backend_names():
if not has_backend(name):
backends = _get_backends()
for name in sorted(backends):
if not backends[name].is_available():
continue
out.extend(_BACKENDS[name].enumerate_active())
out.extend(backends[name].enumerate_active())
out.sort(key=lambda a: (a.started_at, a.slug))
return out
+8 -34
View File
@@ -7,8 +7,9 @@ from __future__ import annotations
import re
import shutil
import subprocess
from typing import Iterable, Iterator
from typing import Iterator
from ...docker_cmd import run_docker
from ...log import die, info
# from ...workspace import WorkspacePlan
@@ -30,12 +31,7 @@ def container_name_candidates(base: str) -> Iterator[str]:
def runsc_available() -> bool:
"""Return True if the Docker daemon has the gVisor (`runsc`) runtime
registered. Called once per prepare; the result lives on the plan."""
r = subprocess.run(
["docker", "info", "--format", "{{json .Runtimes}}"],
capture_output=True,
text=True,
check=False,
)
r = run_docker(["docker", "info", "--format", "{{json .Runtimes}}"])
return r.returncode == 0 and "runsc" in r.stdout
@@ -49,20 +45,15 @@ def require_docker() -> None:
def image_exists(ref: str) -> bool:
return _silent_run(["docker", "image", "inspect", ref]) == 0
return run_docker(["docker", "image", "inspect", ref]).returncode == 0
def container_exists(name: str) -> bool:
"""Returns True if a container (running or stopped) with the given
name exists. Uses `docker ps -a -q -f name=^<name>$` so substring
matches don't false-positive."""
result = subprocess.run(
["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"],
capture_output=True,
text=True,
check=True,
)
return bool(result.stdout.strip())
result = run_docker(["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"])
return result.returncode == 0 and bool(result.stdout.strip())
def force_remove_container(name: str) -> None:
@@ -70,12 +61,7 @@ def force_remove_container(name: str) -> None:
doesn't — and the rm itself is best-effort (errors swallowed) so
this is safe to register as a teardown callback."""
if container_exists(name):
subprocess.run(
["docker", "rm", "-f", name],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
check=False,
)
run_docker(["docker", "rm", "-f", name])
def docker_exec_root(container: str, argv: list[str]) -> None:
@@ -155,22 +141,10 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
def commit_container(container_name: str, image_tag: str) -> None:
"""Run `docker commit <container_name> <image_tag>` to snapshot the
running container's filesystem state as a local Docker image."""
result = subprocess.run(
["docker", "commit", container_name, image_tag],
capture_output=True, text=True, check=False,
)
result = run_docker(["docker", "commit", container_name, image_tag])
if result.returncode != 0:
die(
f"docker commit {container_name!r}{image_tag!r} failed: "
f"{(result.stderr or '').strip() or '<no stderr>'}"
)
info(f"committed {container_name!r}{image_tag!r}")
def _silent_run(cmd: Iterable[str]) -> int:
return subprocess.run(
list(cmd),
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
check=False,
).returncode
+8 -8
View File
@@ -40,7 +40,7 @@ class TestGetBottleBackend(unittest.TestCase):
return True
with patch.dict(os.environ, {}, clear=True), \
patch.object(backend_mod, "_BACKENDS", {
patch.object(backend_mod, "_backends", {
"macos-container": _FakeBackend(),
"docker": _FakeBackend(),
}):
@@ -61,7 +61,7 @@ class TestGetBottleBackend(unittest.TestCase):
with patch.dict(os.environ, {}, clear=True), \
patch.object(backend_mod.FirecrackerBottleBackend,
"is_host_capable", classmethod(lambda cls: False)), \
patch.object(backend_mod, "_BACKENDS", {
patch.object(backend_mod, "_backends", {
"macos-container": _FakeBackend("macos-container", False),
"docker": _FakeBackend("docker", True),
}):
@@ -83,7 +83,7 @@ class TestGetBottleBackend(unittest.TestCase):
with patch.dict(os.environ, {}, clear=True), \
patch.object(backend_mod.FirecrackerBottleBackend,
"is_host_capable", classmethod(lambda cls: True)), \
patch.object(backend_mod, "_BACKENDS", {
patch.object(backend_mod, "_backends", {
"macos-container": _FakeBackend("macos-container", False),
"firecracker": _FakeBackend("firecracker", False),
"docker": _FakeBackend("docker", True),
@@ -133,7 +133,7 @@ class TestEnumerateActiveAgents(unittest.TestCase):
return self._items
with patch.object(
backend_mod, "_BACKENDS",
backend_mod, "_backends",
{"docker": _FakeBackend([a]), "firecracker": _FakeBackend([b])},
):
self.assertEqual([a, b], enumerate_active_agents())
@@ -167,7 +167,7 @@ class TestEnumerateActiveAgents(unittest.TestCase):
return self._items
with patch.object(
backend_mod, "_BACKENDS",
backend_mod, "_backends",
{
"docker": _FakeBackend([newer, tie_b]),
"firecracker": _FakeBackend([missing_metadata, tie_a]),
@@ -187,7 +187,7 @@ class TestEnumerateActiveAgents(unittest.TestCase):
return []
with patch.object(
backend_mod, "_BACKENDS",
backend_mod, "_backends",
{"docker": _FakeBackend(), "firecracker": _FakeBackend()},
):
self.assertEqual([], enumerate_active_agents())
@@ -218,7 +218,7 @@ class TestEnumerateActiveAgents(unittest.TestCase):
return self._items
with patch.object(
backend_mod, "_BACKENDS",
backend_mod, "_backends",
{
"docker": _FakeBackend([present], available=True),
"firecracker": _FakeBackend([hidden], available=False),
@@ -234,7 +234,7 @@ class TestHasBackend(unittest.TestCase):
return False
with patch.object(
backend_mod, "_BACKENDS", {"docker": _FakeBackend()},
backend_mod, "_backends", {"docker": _FakeBackend()},
):
from bot_bottle.backend import has_backend
self.assertFalse(has_backend("docker"))
+3 -3
View File
@@ -29,7 +29,7 @@ def _fail(stderr: str = "boom") -> subprocess.CompletedProcess: # type: ignore
class TestCommitContainer(unittest.TestCase):
def test_runs_docker_commit(self):
with patch.object(
docker_mod.subprocess, "run", return_value=_ok(),
docker_mod, "run_docker", return_value=_ok(),
) as run, patch.object(docker_mod, "info"):
docker_mod.commit_container(
"bot-bottle-dev-abc12",
@@ -47,7 +47,7 @@ class TestCommitContainer(unittest.TestCase):
def test_dies_on_docker_commit_failure(self):
with patch.object(
docker_mod.subprocess, "run", return_value=_fail("No such container"),
docker_mod, "run_docker", return_value=_fail("No such container"),
), patch.object(
docker_mod, "die", side_effect=SystemExit("die"),
) as die:
@@ -58,7 +58,7 @@ class TestCommitContainer(unittest.TestCase):
def test_die_message_includes_image_tag(self):
with patch.object(
docker_mod.subprocess, "run", return_value=_fail("boom"),
docker_mod, "run_docker", return_value=_fail("boom"),
), patch.object(
docker_mod, "die", side_effect=SystemExit("die"),
) as die: