From 563e30ca33b03836d83ce22bcd05577f936dc8dc Mon Sep 17 00:00:00 2001 From: Renn F Date: Mon, 6 Jul 2026 02:10:25 +0200 Subject: [PATCH] refactor(video): rename render client to video_renderer_client (renderer-agnostic) --- roboco/config.py | 8 +-- roboco/runtime/orchestrator.py | 4 +- ...ion_client.py => video_renderer_client.py} | 38 +++++++------ tests/e2e_smoke/test_video_pipeline.py | 2 +- tests/unit/runtime/test_video_render_loop.py | 2 +- ...lient.py => test_video_renderer_client.py} | 56 ++++++++++--------- 6 files changed, 57 insertions(+), 53 deletions(-) rename roboco/services/{remotion_client.py => video_renderer_client.py} (84%) rename tests/unit/services/{test_remotion_client.py => test_video_renderer_client.py} (82%) diff --git a/roboco/config.py b/roboco/config.py index d23d1569..2b0d52c1 100644 --- a/roboco/config.py +++ b/roboco/config.py @@ -886,7 +886,7 @@ class Settings(BaseSettings): description="Seconds between feature-spotlight exploration cycles.", ) - # Video generation (Remotion) — a UX/UI dev authors a bespoke motion-video + # Video generation (HyperFrames) — a UX/UI dev authors a bespoke motion-video # composition per release/spotlight/on-demand trigger through the normal # delivery lifecycle; a later render pass renders it to MP4 and holds the # clip as a CEO-approval draft (mirrors the X engine's held-draft shape). @@ -934,10 +934,10 @@ class Settings(BaseSettings): gt=0, description="Per-request timeout for outbound video-engine HTTP calls.", ) - remotion_base_url: str = Field( - default="http://roboco-remotion:3001", + video_renderer_base_url: str = Field( + default="http://roboco-video-renderer:3001", description=( - "Base URL of the remotion-renderer sidecar. The orchestrator tars " + "Base URL of the video-renderer sidecar. The orchestrator tars " "the merged motion/ source and POSTs it here; the sidecar returns " "MP4 bytes in the response (no cross-container shared volume)." ), diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 190a7f9e..1dd81da1 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -7696,14 +7696,14 @@ Start by: """Render the vertical + square cuts from the roboco project's merged read-clone's motion/ dir; returns {"vertical": path, "square": path}. ``render_key`` (the source task id) scopes each cut's output path.""" - from roboco.services.remotion_client import get_remotion_renderer + from roboco.services.video_renderer_client import get_video_renderer from roboco.services.workspace import get_workspace_service slug = (settings.self_heal_project_slug or "roboco-api").strip() workspace = await get_workspace_service(db).ensure_read_clone(slug) motion_dir = str(workspace / "motion") input_props = draft.get("input_props") or {} - renderer = get_remotion_renderer() + renderer = get_video_renderer() cuts: dict[str, str] = {} for orientation in ("vertical", "square"): cuts[orientation] = await renderer.render( diff --git a/roboco/services/remotion_client.py b/roboco/services/video_renderer_client.py similarity index 84% rename from roboco/services/remotion_client.py rename to roboco/services/video_renderer_client.py index fd82787a..60c505eb 100644 --- a/roboco/services/remotion_client.py +++ b/roboco/services/video_renderer_client.py @@ -1,4 +1,4 @@ -"""RemotionRenderer — HTTP client for the remotion-renderer sidecar. +"""VideoRenderer — HTTP client for the video-renderer sidecar. No cross-container shared volume: the orchestrator tars the merged motion/ source directory from its read-clone and POSTs it to the sidecar; the sidecar @@ -7,9 +7,11 @@ this client writes to an orchestrator-local directory. The sidecar itself stays credential-free and git-free — it only ever sees a tarball plus a JSON side-channel of render parameters. -`NullRemotionRenderer` (an unconfigured `remotion_base_url`) fails the same -clean way an unreachable sidecar does — a `RemotionRendererError`, never a -raw transport crash — mirroring the `NullXClient` graceful-degradation shape. +An unconfigured ``video_renderer_base_url`` makes :func:`get_video_renderer` +return a :class:`NullVideoRenderer` whose :meth:`render` raises a typed +:class:`VideoRendererError` (never a raw transport crash) — fail-fast on a +misconfigured sidecar is correct, and the render loop already handles it as a +bounded retry. """ from __future__ import annotations @@ -30,11 +32,11 @@ from roboco.services import minio_client log = structlog.get_logger(__name__) -class RemotionRendererError(Exception): +class VideoRendererError(Exception): """A render call failed: unconfigured sidecar, unreachable, or non-2xx.""" -class RemotionRenderer: +class VideoRenderer: """Tar the composition source, POST it to the sidecar, save the MP4.""" def __init__( @@ -66,12 +68,12 @@ class RemotionRenderer: an earlier, not-yet-posted draft's clip. Fails fast (no tar, no network attempt) when unconfigured — the same - guard covers both ``NullRemotionRenderer`` and a directly-constructed - ``RemotionRenderer(base_url="")``. + guard covers both ``NullVideoRenderer`` and a directly-constructed + ``VideoRenderer(base_url="")``. """ if not self._base_url: - raise RemotionRendererError( - "remotion sidecar not configured (remotion_base_url unset)" + raise VideoRendererError( + "video-renderer sidecar not configured (video_renderer_base_url unset)" ) tar_bytes = await asyncio.to_thread(self._tar_source, source_dir) mp4_bytes = await self._post( @@ -131,9 +133,9 @@ class RemotionRenderer: timeout=timeout, ) except httpx.HTTPError as exc: - raise RemotionRendererError(f"render request failed: {exc}") from exc + raise VideoRendererError(f"render request failed: {exc}") from exc if not response.is_success: - raise RemotionRendererError( + raise VideoRendererError( f"render failed: HTTP {response.status_code}: {response.text[:200]}" ) return response.content @@ -170,7 +172,7 @@ class RemotionRenderer: return str(path) -class NullRemotionRenderer(RemotionRenderer): +class NullVideoRenderer(VideoRenderer): """No sidecar configured — inherits render()'s empty-base_url guard, so every call raises immediately: no tar, no network call.""" @@ -178,9 +180,9 @@ class NullRemotionRenderer(RemotionRenderer): super().__init__(base_url="") -def get_remotion_renderer() -> RemotionRenderer: - """RemotionRenderer bound to settings.remotion_base_url; Null when unset.""" - base_url = settings.remotion_base_url.strip() +def get_video_renderer() -> VideoRenderer: + """VideoRenderer bound to settings.video_renderer_base_url; Null when unset.""" + base_url = settings.video_renderer_base_url.strip() if not base_url: - return NullRemotionRenderer() - return RemotionRenderer(base_url=base_url) + return NullVideoRenderer() + return VideoRenderer(base_url=base_url) diff --git a/tests/e2e_smoke/test_video_pipeline.py b/tests/e2e_smoke/test_video_pipeline.py index 30639e08..66541d8c 100644 --- a/tests/e2e_smoke/test_video_pipeline.py +++ b/tests/e2e_smoke/test_video_pipeline.py @@ -178,7 +178,7 @@ def _render_completed_task(stack: E2EStack, task_id: UUID) -> None: ).scalar_one() with ( patch( - "roboco.services.remotion_client.get_remotion_renderer", + "roboco.services.video_renderer_client.get_video_renderer", _FakeRenderer, ), patch( diff --git a/tests/unit/runtime/test_video_render_loop.py b/tests/unit/runtime/test_video_render_loop.py index 91b8322a..bfa0ae1c 100644 --- a/tests/unit/runtime/test_video_render_loop.py +++ b/tests/unit/runtime/test_video_render_loop.py @@ -170,7 +170,7 @@ async def _make_completed_video_task( def _render_patches(renderer: _FakeRenderer, workspace: Any) -> Any: return ( patch( - "roboco.services.remotion_client.get_remotion_renderer", + "roboco.services.video_renderer_client.get_video_renderer", lambda: renderer, ), patch( diff --git a/tests/unit/services/test_remotion_client.py b/tests/unit/services/test_video_renderer_client.py similarity index 82% rename from tests/unit/services/test_remotion_client.py rename to tests/unit/services/test_video_renderer_client.py index b098cdb1..d2207db6 100644 --- a/tests/unit/services/test_remotion_client.py +++ b/tests/unit/services/test_video_renderer_client.py @@ -1,4 +1,4 @@ -"""RemotionRenderer coverage: tar/post/save happy path against a mocked httpx +"""VideoRenderer coverage: tar/post/save happy path against a mocked httpx transport, plus unconfigured/unreachable graceful failure (never a crash). """ @@ -10,11 +10,11 @@ import httpx import pytest from roboco.config import settings as cfg from roboco.services import minio_client -from roboco.services.remotion_client import ( - NullRemotionRenderer, - RemotionRenderer, - RemotionRendererError, - get_remotion_renderer, +from roboco.services.video_renderer_client import ( + NullVideoRenderer, + VideoRenderer, + VideoRendererError, + get_video_renderer, ) @@ -44,7 +44,7 @@ async def test_render_posts_tar_and_saves_mp4( transport = httpx.MockTransport(handler) http_client = httpx.AsyncClient(transport=transport) - renderer = RemotionRenderer(base_url="http://fake-remotion", client=http_client) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) path = await renderer.render( source_dir=str(source), @@ -55,7 +55,7 @@ async def test_render_posts_tar_and_saves_mp4( ) await http_client.aclose() - assert captured["url"] == "http://fake-remotion/render" + assert captured["url"] == "http://fake-video-renderer/render" assert str(captured["content_type"]).startswith("multipart/form-data") body = captured["body"] assert isinstance(body, bytes) @@ -84,9 +84,9 @@ async def test_render_non_success_response_raises_clear_error( transport = httpx.MockTransport(handler) http_client = httpx.AsyncClient(transport=transport) - renderer = RemotionRenderer(base_url="http://fake-remotion", client=http_client) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) - with pytest.raises(RemotionRendererError, match="500"): + with pytest.raises(VideoRendererError, match="500"): await renderer.render( source_dir=str(source), composition_id="Intro", @@ -109,9 +109,9 @@ async def test_render_unreachable_sidecar_raises_clear_error( transport = httpx.MockTransport(handler) http_client = httpx.AsyncClient(transport=transport) - renderer = RemotionRenderer(base_url="http://fake-remotion", client=http_client) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) - with pytest.raises(RemotionRendererError, match="render request failed"): + with pytest.raises(VideoRendererError, match="render request failed"): await renderer.render( source_dir=str(source), composition_id="Intro", @@ -127,8 +127,8 @@ async def test_unconfigured_renderer_raises_without_network_call( tmp_path: Path, ) -> None: source = _make_source(tmp_path) - renderer = RemotionRenderer(base_url="") - with pytest.raises(RemotionRendererError, match="not configured"): + renderer = VideoRenderer(base_url="") + with pytest.raises(VideoRendererError, match="not configured"): await renderer.render( source_dir=str(source), composition_id="Intro", @@ -141,8 +141,8 @@ async def test_unconfigured_renderer_raises_without_network_call( @pytest.mark.asyncio async def test_null_renderer_raises_without_network_call(tmp_path: Path) -> None: source = _make_source(tmp_path) - renderer = NullRemotionRenderer() - with pytest.raises(RemotionRendererError, match="not configured"): + renderer = NullVideoRenderer() + with pytest.raises(VideoRendererError, match="not configured"): await renderer.render( source_dir=str(source), composition_id="Intro", @@ -152,21 +152,23 @@ async def test_null_renderer_raises_without_network_call(tmp_path: Path) -> None ) -def test_get_remotion_renderer_returns_null_when_unset( +def test_get_video_renderer_returns_null_when_unset( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setattr(cfg, "remotion_base_url", "") - renderer = get_remotion_renderer() - assert isinstance(renderer, NullRemotionRenderer) + monkeypatch.setattr(cfg, "video_renderer_base_url", "") + renderer = get_video_renderer() + assert isinstance(renderer, NullVideoRenderer) -def test_get_remotion_renderer_returns_real_client_when_set( +def test_get_video_renderer_returns_real_client_when_set( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setattr(cfg, "remotion_base_url", "http://roboco-remotion:3001") - renderer = get_remotion_renderer() - assert isinstance(renderer, RemotionRenderer) - assert not isinstance(renderer, NullRemotionRenderer) + monkeypatch.setattr( + cfg, "video_renderer_base_url", "http://roboco-video-renderer:3001" + ) + renderer = get_video_renderer() + assert isinstance(renderer, VideoRenderer) + assert not isinstance(renderer, NullVideoRenderer) @pytest.mark.asyncio @@ -196,7 +198,7 @@ async def test_save_puts_to_minio_when_configured( transport = httpx.MockTransport(handler) http_client = httpx.AsyncClient(transport=transport) - renderer = RemotionRenderer(base_url="http://fake-remotion", client=http_client) + renderer = VideoRenderer(base_url="http://fake-video-renderer", client=http_client) try: path = await renderer.render( @@ -244,7 +246,7 @@ async def test_save_swallows_minio_put_failure( try: # _save is a sync @staticmethod; call it directly (no httpx needed). - path = RemotionRenderer._save( + path = VideoRenderer._save( b"fake-mp4-bytes", render_key="task-88", orientation="square" ) finally: