perf: lazy-load backend modules and consolidate docker subprocess helpers
lint / lint (push) Failing after 2m7s
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.
This commit is contained in:
@@ -572,28 +572,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
|
||||
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 +646,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 +660,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 +682,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 +701,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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user