fix(docker): remove undefined-variable reconnect that broke all container status/start/stop/restart
Some checks are pending
Test and SonarQube / quality (push) Waiting to run
Some checks are pending
Test and SonarQube / quality (push) Waiting to run
_docker_get_json/_docker_post called sock.connect(sock_path) a second time after _docker_connection() had already connected the socket -- sock_path was never defined in either function, so every call raised NameError. Surfaced in the UI as every engine card reporting "Not installed" and the Active OCR Engine model dropdown reporting "Not Found" regardless of what was actually running, found while diagnosing a user report of that dropdown. Docker socket API calls (list/status/ start/stop/restart) were affected for every engine card, not just OCR. Added tests/test_docker_client.py, which did not exist before, to catch this class of regression going forward. Also records a request to make the magazine ingestion "watching it work" panel draw detected region boxes live over the page image as each is classified, instead of today's static page-image-plus-text- list (TODO.md; no code change for that yet). 254 tests passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
531cb50cae
commit
53624d470b
10
CHANGELOG.md
10
CHANGELOG.md
@ -5,6 +5,16 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
|
||||
|
||||
---
|
||||
|
||||
## [1.29.1] — 2026-09-14
|
||||
|
||||
### Fixed
|
||||
- **All Docker container status checks, start, stop and restart were silently broken** (`core/docker_client.py`): `_docker_get_json`/`_docker_post` called `sock.connect(sock_path)` a second time after `_docker_connection()` had already connected the socket, referencing a `sock_path` that was never defined anywhere in either function — every call raised `NameError`, surfaced to the UI as every engine card showing "Not installed"/"Not Found" regardless of what was actually running (found via the OCR engine cards and the Active OCR Engine model dropdown both showing this). Removed the redundant second connect call. Added `tests/test_docker_client.py` so this can't regress silently again.
|
||||
|
||||
### Docs
|
||||
- Recorded a request to make the magazine ingestion "watching it work" panel more visually engaging — draw detected region boxes live over the page image as each is classified, instead of the current static page-image-plus-text-list (TODO.md).
|
||||
|
||||
---
|
||||
|
||||
## [1.29.0] — 2026-09-14
|
||||
|
||||
### Added
|
||||
|
||||
10
TODO.md
10
TODO.md
@ -369,6 +369,16 @@ casting. The original PDF remains the visual source of truth while approved arti
|
||||
|
||||
- [x] Generate per-article podcast scripts from a connected LLM with two or three user-picked host voices,
|
||||
with a Studio panel to pick hosts, generate, and play the script back turn by turn.
|
||||
- [ ] **Make the ingestion "watching it work" panel more visually appealing** — user-requested 2026-09-14. Today it's a
|
||||
static side-by-side of the current page image and a plain text/label list (`#mag-ingest-preview`,
|
||||
`static/sections/s-magazine.html`, populated by `routes/magazine.py`'s preview-HTML builder around line 158).
|
||||
Wanted instead: draw the actual detected region boxes (image/table/code/figure/etc.) live, overlaid directly on
|
||||
the page image as each region is classified — watching the layout engine work through the page, not just a
|
||||
before/after. Likely an SVG or absolutely-positioned overlay using each block's `bbox` (already computed and
|
||||
already used for the review screen's region boxes, `magReviewMarkup`-adjacent code in `static/js/magazine.js`)
|
||||
drawn incrementally, one box per region as it's classified, rather than all boxes appearing at once at the end.
|
||||
Purely cosmetic/decorative per the existing comment on that panel — must not become the only place the
|
||||
essential progress info (percentage, page count, done/failed) lives; that stays in `#mag-ingest-status`.
|
||||
|
||||
### P1 — Magazine listening experience
|
||||
|
||||
|
||||
@ -60,7 +60,6 @@ def _docker_get_json(path: str) -> tuple[int, dict | list | None]:
|
||||
chunks: list[bytes] = []
|
||||
with _docker_connection() as sock:
|
||||
sock.settimeout(10)
|
||||
sock.connect(sock_path)
|
||||
sock.sendall(request)
|
||||
while True:
|
||||
data = sock.recv(65536)
|
||||
@ -99,7 +98,6 @@ def _docker_post(path: str) -> tuple[int, str]:
|
||||
chunks = []
|
||||
with _docker_connection() as sock:
|
||||
sock.settimeout(20)
|
||||
sock.connect(sock_path)
|
||||
sock.sendall(request)
|
||||
while True:
|
||||
data = sock.recv(8192)
|
||||
|
||||
75
tests/test_docker_client.py
Normal file
75
tests/test_docker_client.py
Normal file
@ -0,0 +1,75 @@
|
||||
"""Regression coverage for the low-level Docker socket client.
|
||||
|
||||
_docker_get_json/_docker_post previously called sock.connect(sock_path) a
|
||||
second time after _docker_connection() had already connected the socket --
|
||||
sock_path was never defined in either function, so every container status
|
||||
check, start, stop and restart raised NameError. These tests exercise both
|
||||
functions against a fake connected socket so that mistake can't ship again
|
||||
silently.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from unittest import mock
|
||||
|
||||
from core.docker_client import _docker_get_json, _docker_post
|
||||
|
||||
|
||||
class _FakeSocket:
|
||||
"""Minimal stand-in for socket.socket, pre-"connected"."""
|
||||
|
||||
def __init__(self, response: bytes):
|
||||
self._chunks = [response]
|
||||
self.sent = b""
|
||||
|
||||
def settimeout(self, _timeout):
|
||||
pass
|
||||
|
||||
def sendall(self, data: bytes):
|
||||
self.sent += data
|
||||
|
||||
def recv(self, _bufsize: int) -> bytes:
|
||||
return self._chunks.pop(0) if self._chunks else b""
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
return False
|
||||
|
||||
|
||||
class DockerGetJsonTests(unittest.TestCase):
|
||||
def test_returns_parsed_json_body(self):
|
||||
response = b'HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n{"ok": true}'
|
||||
fake = _FakeSocket(response)
|
||||
with mock.patch("core.docker_client._docker_available", return_value=True), \
|
||||
mock.patch("core.docker_client._docker_connection", return_value=fake):
|
||||
code, data = _docker_get_json("/containers/foo/json")
|
||||
self.assertEqual(code, 200)
|
||||
self.assertEqual(data, {"ok": True})
|
||||
|
||||
def test_does_not_reconnect_the_socket(self):
|
||||
# A regression guard for the exact bug: a second sock.connect(sock_path)
|
||||
# call would raise NameError before ever reaching sendall().
|
||||
response = b"HTTP/1.1 404 Not Found\r\n\r\n"
|
||||
fake = _FakeSocket(response)
|
||||
with mock.patch("core.docker_client._docker_available", return_value=True), \
|
||||
mock.patch("core.docker_client._docker_connection", return_value=fake):
|
||||
code, _ = _docker_get_json("/containers/missing/json")
|
||||
self.assertEqual(code, 404)
|
||||
self.assertIn(b"GET /containers/missing/json", fake.sent)
|
||||
|
||||
|
||||
class DockerPostTests(unittest.TestCase):
|
||||
def test_returns_status_code(self):
|
||||
response = b"HTTP/1.1 204 No Content\r\n\r\n"
|
||||
fake = _FakeSocket(response)
|
||||
with mock.patch("core.docker_client._docker_available", return_value=True), \
|
||||
mock.patch("core.docker_client._docker_connection", return_value=fake):
|
||||
code, _raw = _docker_post("/containers/foo/start")
|
||||
self.assertEqual(code, 204)
|
||||
self.assertIn(b"POST /containers/foo/start", fake.sent)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Loading…
Reference in New Issue
Block a user