-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: container management and bootstrap logic #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,63 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import click | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _log(msg: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Bootstrap logging — stderr only, never stdout (stdio MCP uses stdout).""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"[context8] {msg}", file=sys.stderr, flush=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _bootstrap() -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Idempotent bootstrap: container up, collection ready, models cached. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Safe to run on every `serve` invocation — each step is a no-op when already | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| satisfied. All output goes to stderr so the MCP stdio protocol stays clean. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ...docker import ensure_running, is_container_running | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not is_container_running(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _log("starting DB container...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ok, msg = ensure_running(timeout_secs=30) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not ok: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _log(f"FATAL: container failed to start: {msg}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise SystemExit(1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _log(f"container ready ({msg})") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+29
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not is_container_running(): | |
| _log("starting DB container...") | |
| ok, msg = ensure_running(timeout_secs=30) | |
| if not ok: | |
| _log(f"FATAL: container failed to start: {msg}") | |
| raise SystemExit(1) | |
| _log(f"container ready ({msg})") | |
| was_running = is_container_running() | |
| if was_running: | |
| _log("verifying DB container readiness...") | |
| else: | |
| _log("starting DB container...") | |
| ok, msg = ensure_running(timeout_secs=30) | |
| if not ok: | |
| _log(f"FATAL: container failed to start: {msg}") | |
| raise SystemExit(1) | |
| _log(f"container ready ({msg})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider handling storage initialization failures to produce a clearer fatal message.
If StorageService() or storage.initialize() fails (e.g. DB unreachable, bad config), the exception will currently bubble up and crash without the clear stderr message you use for the container bootstrap. Consider wrapping storage initialization in a try/except, logging a FATAL via _log, and exiting with SystemExit(1) so failures are reported consistently and predictably.
| from ...storage import StorageService | |
| storage = StorageService() | |
| created = storage.initialize() | |
| if created: | |
| _log("collection created") | |
| storage.close() | |
| from ...storage import StorageService | |
| storage = None | |
| try: | |
| storage = StorageService() | |
| created = storage.initialize() | |
| if created: | |
| _log("collection created") | |
| except Exception as e: | |
| _log(f"FATAL: storage initialization failed: {e}") | |
| raise SystemExit(1) | |
| finally: | |
| if storage is not None: | |
| try: | |
| storage.close() | |
| except Exception as e: | |
| _log(f"warning: failed to close storage cleanly ({e})") |
Copilot
AI
Apr 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
storage.close() won’t run if storage.initialize() raises (e.g., DB connection issue). Wrap the StorageService lifecycle in a try/finally (or context manager) so the client connection is always closed during bootstrap failures.
| created = storage.initialize() | |
| if created: | |
| _log("collection created") | |
| storage.close() | |
| try: | |
| created = storage.initialize() | |
| if created: | |
| _log("collection created") | |
| finally: | |
| storage.close() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| """Docker management for Context8 — start/stop/check the Actian VectorAI DB container. | ||
| """Container runtime management for Context8 — start/stop/check the Actian | ||
| VectorAI DB container. | ||
|
|
||
| Generates docker-compose.yml on demand into ~/.context8/ so it works | ||
| whether installed via pip, uv, or from source. Never requires the user | ||
| to have a compose file in their project. | ||
| Generates a compose file on demand into ~/.context8/ so it works whether | ||
| installed via pip, uv, or from source. Supports both Docker and Podman — | ||
| the runtime is detected automatically. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
@@ -19,7 +20,6 @@ | |
| CONTEXT8_DIR = _home() / ".context8" | ||
|
|
||
| COMPOSE_TEMPLATE = """\ | ||
| version: "3.8" | ||
| services: | ||
| vectoraidb: | ||
| image: docker.io/williamimoh/actian-vectorai-db:latest | ||
|
|
@@ -34,6 +34,9 @@ | |
|
|
||
| CONTAINER_NAME = "context8_db" | ||
|
|
||
| _runtime_cache: str | None = None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (complexity): Consider simplifying runtime and compose detection by replacing the multiple helper functions and global caches with a single linear resolver that returns both the compose command and runtime. You can drop most of the indirection by collapsing runtime/compose detection into a single linear resolver and removing the global caches/sentinels. That keeps Docker/Podman support but makes the control flow much easier to follow. For example, you can replace def _iter_compose_candidates() -> list[tuple[list[str], str]]:
# command, inferred runtime
return [
(["docker", "compose"], "docker"),
(["docker-compose"], "docker"),
(["podman", "compose"], "podman"),
(["podman-compose"], "podman"),
]
def _resolve_compose() -> tuple[list[str], str] | None:
"""Return (compose_cmd, runtime) or None if nothing is usable."""
for cmd, runtime in _iter_compose_candidates():
try:
subprocess.run(
cmd + ["version"],
capture_output=True,
check=True,
timeout=5,
)
return cmd, runtime
except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired):
continue
return NoneThen def run_compose(args: list[str]) -> subprocess.CompletedProcess:
"""Run a compose command against the Context8 compose file."""
_ensure_compose_file()
resolved = _resolve_compose()
if resolved is None:
return subprocess.CompletedProcess(
args=args,
returncode=1,
stdout="",
stderr=(
"no compose tool found — install one of: "
"`docker compose`, `podman compose`, `podman-compose`, `docker-compose`"
),
)
cmd, _runtime = resolved
return subprocess.run(
cmd + args,
cwd=str(_compose_dir()),
capture_output=True,
text=True,
)def is_container_running() -> bool:
"""Check if the context8_db container is running under docker or podman."""
resolved = _resolve_compose()
if resolved is None:
return False
_cmd, runtime = resolved
try:
result = subprocess.run(
[runtime, "ps", "--filter", f"name={CONTAINER_NAME}", "--format", "{{.Status}}"],
capture_output=True,
text=True,
timeout=5,
)
return bool(result.stdout.strip()) and "Up" in result.stdout
except Exception:
return Falsedef ensure_running(timeout_secs: int = 30) -> tuple[bool, str]:
"""Start the container if not running, wait for it to be healthy."""
if is_container_running():
return True, "already running"
resolved = _resolve_compose()
runtime = resolved[1] if resolved is not None else "container runtime"
logger.info(f"Starting Actian VectorAI DB container via {runtime}...")
result = run_compose(["up", "-d"])
if result.returncode != 0:
return False, f"compose up failed: {result.stderr.strip()}"
...This keeps all existing functionality (Docker + Podman, the specific error message and
The behavior becomes: “try a small ordered set of compose commands, pick the first working one, infer runtime from that”, which matches the reviewer’s suggested mental model and is easier to reason about and test. |
||
| _compose_cache: list[str] | None = None | ||
|
|
||
|
|
||
| def _compose_dir() -> Path: | ||
| """Return ~/.context8/ — where we keep the generated compose file.""" | ||
|
|
@@ -55,36 +58,106 @@ def _ensure_compose_file() -> Path: | |
| return compose_path | ||
|
|
||
|
|
||
| def _docker_compose_cmd() -> list[str]: | ||
| """Return the docker compose command (v2 first, v1 fallback).""" | ||
| def _probe(cmd: list[str]) -> bool: | ||
| try: | ||
| subprocess.run( | ||
| ["docker", "compose", "version"], | ||
| capture_output=True, | ||
| check=True, | ||
| ) | ||
| return ["docker", "compose"] | ||
| except (subprocess.CalledProcessError, FileNotFoundError): | ||
| return ["docker-compose"] | ||
| subprocess.run(cmd, capture_output=True, check=True, timeout=5) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. Source: opengrep |
||
| return True | ||
| except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired): | ||
| return False | ||
|
|
||
|
|
||
| def detect_runtime() -> str | None: | ||
| """Return 'docker' or 'podman' — the first runtime whose daemon is reachable. | ||
|
|
||
| Prefers a *working* runtime over a merely installed one: if docker is | ||
| installed but its daemon is down, falls through to podman. Returns None | ||
| if neither is usable. | ||
| """ | ||
| global _runtime_cache | ||
| if _runtime_cache is not None: | ||
| return _runtime_cache or None | ||
|
|
||
| # First pass: pick a runtime whose daemon is actually reachable. | ||
| for candidate in ("docker", "podman"): | ||
| if _probe([candidate, "info"]): | ||
| _runtime_cache = candidate | ||
| return candidate | ||
|
|
||
| # Second pass: any installed runtime, even if the daemon is down — so | ||
| # callers can still surface a useful error message. | ||
| for candidate in ("docker", "podman"): | ||
| if _probe([candidate, "--version"]): | ||
| _runtime_cache = candidate | ||
| return candidate | ||
|
|
||
| _runtime_cache = "" | ||
| return None | ||
|
|
||
|
|
||
| def _compose_cmd() -> list[str] | None: | ||
| """Return the compose command for the detected runtime, or None. | ||
|
|
||
| Tries (in order): `docker compose`, `docker-compose`, `podman compose`, | ||
| `podman-compose`. | ||
| """ | ||
| global _compose_cache | ||
| if _compose_cache is not None: | ||
| return _compose_cache or None | ||
|
|
||
| candidates: list[list[str]] = [] | ||
| runtime = detect_runtime() | ||
| if runtime == "docker": | ||
| candidates = [["docker", "compose"], ["docker-compose"]] | ||
| elif runtime == "podman": | ||
| candidates = [["podman", "compose"], ["podman-compose"]] | ||
| else: | ||
| # Try everything anyway in case the runtime probe missed something | ||
| candidates = [ | ||
| ["docker", "compose"], | ||
| ["podman", "compose"], | ||
| ["podman-compose"], | ||
| ["docker-compose"], | ||
| ] | ||
|
|
||
| for cmd in candidates: | ||
| if _probe(cmd + ["version"]): | ||
| _compose_cache = cmd | ||
| return cmd | ||
|
|
||
| _compose_cache = [] | ||
| return None | ||
|
|
||
|
|
||
| def run_compose(args: list[str]) -> subprocess.CompletedProcess: | ||
| """Run a docker compose command against the Context8 compose file.""" | ||
| """Run a compose command against the Context8 compose file.""" | ||
| _ensure_compose_file() | ||
| cmd = _docker_compose_cmd() + args | ||
| cmd = _compose_cmd() | ||
| if cmd is None: | ||
| return subprocess.CompletedProcess( | ||
| args=args, | ||
| returncode=1, | ||
| stdout="", | ||
| stderr=( | ||
| "no compose tool found — install one of: " | ||
| "`docker compose`, `podman compose`, `podman-compose`, `docker-compose`" | ||
| ), | ||
| ) | ||
|
Comment on lines
+136
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'CompletedProcess' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. Source: opengrep |
||
| return subprocess.run( | ||
| cmd, | ||
| cmd + args, | ||
| cwd=str(_compose_dir()), | ||
| capture_output=True, | ||
| text=True, | ||
|
Comment on lines
145
to
149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. Source: opengrep |
||
| ) | ||
|
|
||
|
|
||
| def is_container_running() -> bool: | ||
| """Check if the context8_db container is running.""" | ||
| """Check if the context8_db container is running under docker or podman.""" | ||
| runtime = detect_runtime() | ||
| if runtime is None: | ||
| return False | ||
| try: | ||
| result = subprocess.run( | ||
| ["docker", "ps", "--filter", f"name={CONTAINER_NAME}", "--format", "{{{{.Status}}}}"], | ||
| [runtime, "ps", "--filter", f"name={CONTAINER_NAME}", "--format", "{{.Status}}"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
|
Comment on lines
159
to
163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. Source: opengrep |
||
|
|
@@ -102,11 +175,12 @@ def ensure_running(timeout_secs: int = 30) -> tuple[bool, str]: | |
| if is_container_running(): | ||
| return True, "already running" | ||
|
Comment on lines
175
to
176
|
||
|
|
||
| logger.info("Starting Actian VectorAI DB container...") | ||
| runtime = detect_runtime() or "container runtime" | ||
| logger.info(f"Starting Actian VectorAI DB container via {runtime}...") | ||
| result = run_compose(["up", "-d"]) | ||
|
|
||
| if result.returncode != 0: | ||
| return False, f"docker compose up failed: {result.stderr.strip()}" | ||
| return False, f"compose up failed: {result.stderr.strip()}" | ||
|
|
||
| # Wait for the DB to accept connections | ||
| for _ in range(timeout_secs): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep