refactor(freezer): drop Bottle from commit signature
Freezer._freeze only ever used bottle.name, which is always
f"bot-bottle-{agent.slug}". Remove the Bottle parameter from
commit() and _freeze(), derive the container name from agent.slug
directly in each subclass, and delete the _NamedBottle stub that
existed solely to paper over this.
This commit is contained in:
@@ -526,9 +526,9 @@ from .docker import DockerBottleBackend # noqa: E402 # pylint: disable=wrong-i
|
||||
from .macos_container import MacosContainerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position
|
||||
from .smolmachines import SmolmachinesBottleBackend # noqa: E402 # pylint: disable=wrong-import-position
|
||||
|
||||
# Freezer is imported after the backend classes for the same reason —
|
||||
# Freezer.commit_slug constructs ActiveAgent, so the dataclass must be
|
||||
# fully defined first.
|
||||
# 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
|
||||
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from .. import ActiveAgent, Bottle
|
||||
from .. import ActiveAgent
|
||||
from ..freeze import Freezer
|
||||
from .util import commit_container
|
||||
from ...log import info
|
||||
@@ -13,9 +13,10 @@ class DockerFreezer(Freezer):
|
||||
|
||||
backend_name = "docker"
|
||||
|
||||
def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str:
|
||||
def _freeze(self, agent: ActiveAgent) -> str:
|
||||
container = f"bot-bottle-{agent.slug}"
|
||||
image_tag = f"bot-bottle-committed-{agent.slug}:latest"
|
||||
commit_container(bottle.name, image_tag)
|
||||
commit_container(container, image_tag)
|
||||
return image_tag
|
||||
|
||||
def _export_hint(self, slug: str, image_ref: str) -> None:
|
||||
|
||||
@@ -6,8 +6,8 @@ print resume hint) and backend-specific subclasses in their respective
|
||||
backend directories.
|
||||
|
||||
Entry points:
|
||||
Freezer.commit(agent, bottle) — for use within a live launch context
|
||||
Freezer.commit_slug(slug) — for cmd_commit when no live Bottle exists
|
||||
Freezer.commit(agent) — freeze by ActiveAgent
|
||||
Freezer.commit_slug(slug) — convenience wrapper for cmd_commit
|
||||
get_freezer(backend_name) — factory
|
||||
"""
|
||||
|
||||
@@ -15,7 +15,7 @@ from __future__ import annotations
|
||||
|
||||
from abc import ABC, abstractmethod
|
||||
|
||||
from . import ActiveAgent, Bottle, ExecResult
|
||||
from . import ActiveAgent
|
||||
from ..bottle_state import mark_preserved, write_committed_image
|
||||
from ..log import die, info
|
||||
|
||||
@@ -38,8 +38,8 @@ class Freezer(ABC):
|
||||
|
||||
backend_name: str
|
||||
|
||||
def commit(self, agent: ActiveAgent, bottle: Bottle) -> None:
|
||||
"""Freeze `bottle` to a resumable artifact.
|
||||
def commit(self, agent: ActiveAgent) -> None:
|
||||
"""Freeze the bottle for `agent` to a resumable artifact.
|
||||
|
||||
Calls _freeze for the backend-specific snapshot, then writes the
|
||||
committed image reference to per-bottle state and marks the bottle
|
||||
@@ -48,14 +48,14 @@ class Freezer(ABC):
|
||||
Raises CommitCancelled if the user declines an interactive
|
||||
confirmation prompt (e.g. the macos-container stop prompt).
|
||||
"""
|
||||
image_ref = self._freeze(agent, bottle)
|
||||
image_ref = self._freeze(agent)
|
||||
write_committed_image(agent.slug, image_ref)
|
||||
mark_preserved(agent.slug)
|
||||
info(f"to resume from this snapshot: ./cli.py resume {agent.slug}")
|
||||
self._export_hint(agent.slug, image_ref)
|
||||
|
||||
@abstractmethod
|
||||
def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str:
|
||||
def _freeze(self, agent: ActiveAgent) -> str:
|
||||
"""Backend-specific snapshot. Returns the image tag or artifact path
|
||||
stored by write_committed_image. Raises CommitCancelled if the user
|
||||
declines a stop-confirmation prompt."""
|
||||
@@ -65,11 +65,7 @@ class Freezer(ABC):
|
||||
Overridden by backends that provide a meaningful export command."""
|
||||
|
||||
def commit_slug(self, slug: str) -> None:
|
||||
"""Convenience entry for cmd_commit when no live Bottle is available.
|
||||
|
||||
Constructs a minimal ActiveAgent from per-bottle state and a
|
||||
name-only Bottle stub, then delegates to commit(). Freezer
|
||||
subclasses must not call exec / exec_agent / cp_in on the stub."""
|
||||
"""Convenience entry for cmd_commit when only a slug is available."""
|
||||
from ..bottle_state import read_metadata
|
||||
metadata = read_metadata(slug)
|
||||
agent = ActiveAgent(
|
||||
@@ -79,34 +75,7 @@ class Freezer(ABC):
|
||||
started_at=metadata.started_at if metadata else "",
|
||||
services=(),
|
||||
)
|
||||
bottle: Bottle = _NamedBottle(f"bot-bottle-{slug}")
|
||||
self.commit(agent, bottle)
|
||||
|
||||
|
||||
class _NamedBottle(Bottle):
|
||||
"""Name-only Bottle stub for Freezer.commit_slug.
|
||||
|
||||
Only `name` is meaningful. All runtime operations raise
|
||||
NotImplementedError — Freezer._freeze implementations must only
|
||||
access bottle.name."""
|
||||
|
||||
def __init__(self, name: str) -> None:
|
||||
self.name = name
|
||||
|
||||
def agent_argv(self, argv: list[str], *, tty: bool = True) -> list[str]:
|
||||
raise NotImplementedError
|
||||
|
||||
def exec_agent(self, argv: list[str], *, tty: bool = True) -> int:
|
||||
raise NotImplementedError
|
||||
|
||||
def exec(self, script: str, *, user: str = "node") -> ExecResult:
|
||||
raise NotImplementedError
|
||||
|
||||
def cp_in(self, host_path: str, container_path: str) -> None:
|
||||
raise NotImplementedError
|
||||
|
||||
def close(self) -> None:
|
||||
pass
|
||||
self.commit(agent)
|
||||
|
||||
|
||||
def get_freezer(backend_name: str) -> Freezer:
|
||||
|
||||
@@ -8,7 +8,7 @@ from __future__ import annotations
|
||||
|
||||
import sys
|
||||
|
||||
from .. import ActiveAgent, Bottle
|
||||
from .. import ActiveAgent
|
||||
from ..freeze import CommitCancelled, Freezer
|
||||
from .util import commit_container, container_is_running, stop_container
|
||||
from ...log import info
|
||||
@@ -19,9 +19,10 @@ class MacosContainerFreezer(Freezer):
|
||||
|
||||
backend_name = "macos-container"
|
||||
|
||||
def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str:
|
||||
def _freeze(self, agent: ActiveAgent) -> str:
|
||||
container = f"bot-bottle-{agent.slug}"
|
||||
image_tag = f"bot-bottle-committed-{agent.slug}:latest"
|
||||
if container_is_running(bottle.name):
|
||||
if container_is_running(container):
|
||||
sys.stderr.write(
|
||||
f"bot-bottle: bottle {agent.slug!r} is running; "
|
||||
"commit will stop it. Continue? [y/N] "
|
||||
@@ -30,8 +31,8 @@ class MacosContainerFreezer(Freezer):
|
||||
reply = _read_tty_line().strip().lower()
|
||||
if reply not in ("y", "yes"):
|
||||
raise CommitCancelled
|
||||
stop_container(bottle.name)
|
||||
commit_container(bottle.name, image_tag)
|
||||
stop_container(container)
|
||||
commit_container(container, image_tag)
|
||||
return image_tag
|
||||
|
||||
def _export_hint(self, slug: str, image_ref: str) -> None:
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from .. import ActiveAgent, Bottle
|
||||
from .. import ActiveAgent
|
||||
from ..freeze import Freezer
|
||||
from .smolvm import pack_create_from_vm
|
||||
from ...bottle_state import bottle_state_dir
|
||||
@@ -14,10 +14,11 @@ class SmolmachinesFreezer(Freezer):
|
||||
|
||||
backend_name = "smolmachines"
|
||||
|
||||
def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str:
|
||||
def _freeze(self, agent: ActiveAgent) -> str:
|
||||
machine = f"bot-bottle-{agent.slug}"
|
||||
output = bottle_state_dir(agent.slug) / "committed-smolmachine"
|
||||
output.parent.mkdir(parents=True, exist_ok=True)
|
||||
pack_create_from_vm(bottle.name, output)
|
||||
pack_create_from_vm(machine, output)
|
||||
artifact = output.with_name(f"{output.name}.smolmachine")
|
||||
return str(artifact)
|
||||
|
||||
|
||||
@@ -5,14 +5,13 @@ from __future__ import annotations
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, call, patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from bot_bottle import supervise, bottle_state
|
||||
from bot_bottle.backend import ActiveAgent
|
||||
from bot_bottle.backend.freeze import (
|
||||
CommitCancelled,
|
||||
Freezer,
|
||||
_NamedBottle,
|
||||
get_freezer,
|
||||
)
|
||||
from bot_bottle.backend.docker.freezer import DockerFreezer
|
||||
@@ -46,10 +45,6 @@ def _make_agent(slug: str, backend: str = "docker") -> ActiveAgent:
|
||||
)
|
||||
|
||||
|
||||
def _make_bottle(slug: str) -> _NamedBottle:
|
||||
return _NamedBottle(f"bot-bottle-{slug}")
|
||||
|
||||
|
||||
class TestGetFreezer(unittest.TestCase):
|
||||
def test_docker(self):
|
||||
self.assertIsInstance(get_freezer("docker"), DockerFreezer)
|
||||
@@ -85,12 +80,11 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase):
|
||||
started_at="t", backend="docker",
|
||||
))
|
||||
freezer = get_freezer("docker")
|
||||
agent = _make_agent(slug, "docker")
|
||||
bottle = _make_bottle(slug)
|
||||
agent = _make_agent(slug)
|
||||
|
||||
with patch.object(freezer, "_freeze", return_value="bot-bottle-committed-dev-abc12:latest"), \
|
||||
patch("bot_bottle.backend.freeze.info"):
|
||||
freezer.commit(agent, bottle)
|
||||
freezer.commit(agent)
|
||||
|
||||
self.assertEqual(
|
||||
"bot-bottle-committed-dev-abc12:latest",
|
||||
@@ -98,7 +92,7 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase):
|
||||
)
|
||||
self.assertTrue(bottle_state.is_preserved(slug))
|
||||
|
||||
def test_commit_slug_builds_correct_bottle_name(self):
|
||||
def test_commit_slug_passes_correct_slug_to_freeze(self):
|
||||
slug = "dev-abc12"
|
||||
bottle_state.write_metadata(bottle_state.BottleMetadata(
|
||||
identity=slug, agent_name="dev", cwd="", copy_cwd=False,
|
||||
@@ -107,15 +101,15 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase):
|
||||
freezer = get_freezer("docker")
|
||||
captured = {}
|
||||
|
||||
def capture_freeze(agent, bottle):
|
||||
captured["bottle_name"] = bottle.name
|
||||
def capture_freeze(agent):
|
||||
captured["slug"] = agent.slug
|
||||
return "some-ref"
|
||||
|
||||
with patch.object(freezer, "_freeze", side_effect=capture_freeze), \
|
||||
patch("bot_bottle.backend.freeze.info"):
|
||||
freezer.commit_slug(slug)
|
||||
|
||||
self.assertEqual(f"bot-bottle-{slug}", captured["bottle_name"])
|
||||
self.assertEqual(slug, captured["slug"])
|
||||
|
||||
|
||||
class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
@@ -125,20 +119,19 @@ class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
def tearDown(self):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def test_commits_container_and_returns_image_tag(self):
|
||||
def test_commits_container_and_records_image(self):
|
||||
slug = "dev-abc12"
|
||||
freezer = DockerFreezer()
|
||||
agent = _make_agent(slug)
|
||||
bottle = _make_bottle(slug)
|
||||
|
||||
with patch("bot_bottle.backend.docker.freezer.commit_container") as mock_commit, \
|
||||
patch("bot_bottle.backend.freeze.info"), \
|
||||
patch("bot_bottle.backend.docker.freezer.info"):
|
||||
bottle_state.write_metadata(bottle_state.BottleMetadata(
|
||||
identity=slug, agent_name="dev", cwd="", copy_cwd=False,
|
||||
started_at="t", backend="docker",
|
||||
))
|
||||
freezer.commit(agent, bottle)
|
||||
freezer = DockerFreezer()
|
||||
agent = _make_agent(slug)
|
||||
|
||||
with patch("bot_bottle.backend.docker.freezer.commit_container") as mock_commit, \
|
||||
patch("bot_bottle.backend.freeze.info"), \
|
||||
patch("bot_bottle.backend.docker.freezer.info"):
|
||||
freezer.commit(agent)
|
||||
|
||||
mock_commit.assert_called_once_with(
|
||||
f"bot-bottle-{slug}",
|
||||
@@ -169,14 +162,13 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
self._write_meta(slug)
|
||||
freezer = MacosContainerFreezer()
|
||||
agent = _make_agent(slug, "macos-container")
|
||||
bottle = _make_bottle(slug)
|
||||
|
||||
with patch("bot_bottle.backend.macos_container.freezer.container_is_running",
|
||||
return_value=False), \
|
||||
patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit, \
|
||||
patch("bot_bottle.backend.freeze.info"), \
|
||||
patch("bot_bottle.backend.macos_container.freezer.info"):
|
||||
freezer.commit(agent, bottle)
|
||||
freezer.commit(agent)
|
||||
|
||||
mock_commit.assert_called_once_with(
|
||||
f"bot-bottle-{slug}",
|
||||
@@ -188,7 +180,6 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
self._write_meta(slug)
|
||||
freezer = MacosContainerFreezer()
|
||||
agent = _make_agent(slug, "macos-container")
|
||||
bottle = _make_bottle(slug)
|
||||
|
||||
with patch("bot_bottle.backend.macos_container.freezer.container_is_running",
|
||||
return_value=True), \
|
||||
@@ -198,7 +189,7 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit, \
|
||||
patch("bot_bottle.backend.freeze.info"), \
|
||||
patch("bot_bottle.backend.macos_container.freezer.info"):
|
||||
freezer.commit(agent, bottle)
|
||||
freezer.commit(agent)
|
||||
|
||||
mock_stop.assert_called_once_with(f"bot-bottle-{slug}")
|
||||
mock_commit.assert_called_once()
|
||||
@@ -208,7 +199,6 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
self._write_meta(slug)
|
||||
freezer = MacosContainerFreezer()
|
||||
agent = _make_agent(slug, "macos-container")
|
||||
bottle = _make_bottle(slug)
|
||||
|
||||
with patch("bot_bottle.backend.macos_container.freezer.container_is_running",
|
||||
return_value=True), \
|
||||
@@ -217,7 +207,7 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
patch("bot_bottle.backend.macos_container.freezer.stop_container") as mock_stop, \
|
||||
patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit:
|
||||
with self.assertRaises(CommitCancelled):
|
||||
freezer.commit(agent, bottle)
|
||||
freezer.commit(agent)
|
||||
|
||||
mock_stop.assert_not_called()
|
||||
mock_commit.assert_not_called()
|
||||
@@ -238,12 +228,11 @@ class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase):
|
||||
))
|
||||
freezer = SmolmachinesFreezer()
|
||||
agent = _make_agent(slug, "smolmachines")
|
||||
bottle = _make_bottle(slug)
|
||||
|
||||
with patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack, \
|
||||
patch("bot_bottle.backend.freeze.info"), \
|
||||
patch("bot_bottle.backend.smolmachines.freezer.info"):
|
||||
freezer.commit(agent, bottle)
|
||||
freezer.commit(agent)
|
||||
|
||||
expected_output = bottle_state.bottle_state_dir(slug) / "committed-smolmachine"
|
||||
mock_pack.assert_called_once_with(f"bot-bottle-{slug}", expected_output)
|
||||
|
||||
Reference in New Issue
Block a user