From 431f58bb10887dc34ea502331a157d3e32c329b0 Mon Sep 17 00:00:00 2001 From: jremitz Date: Mon, 20 Apr 2026 19:35:46 -0500 Subject: [PATCH 1/2] feat: scheduled publish support, upload improvements Co-Authored-By: Claude --- reeln_cloudflare_plugin/plugin.py | 107 ++++++++++---- tests/unit/test_plugin.py | 226 ++++++++++++++++++++++++++++++ 2 files changed, 306 insertions(+), 27 deletions(-) diff --git a/reeln_cloudflare_plugin/plugin.py b/reeln_cloudflare_plugin/plugin.py index 7f47c88..6307440 100644 --- a/reeln_cloudflare_plugin/plugin.py +++ b/reeln_cloudflare_plugin/plugin.py @@ -4,10 +4,12 @@ import logging import os +from pathlib import Path from typing import Any from reeln.models.auth import AuthCheckResult, AuthStatus from reeln.models.plugin_schema import ConfigField, PluginConfigSchema +from reeln.plugins.capabilities import UploaderSkipped from reeln.plugins.hooks import Hook, HookContext from reeln.plugins.registry import HookRegistry @@ -109,42 +111,56 @@ def register(self, registry: HookRegistry) -> None: registry.register(Hook.ON_GAME_FINISH, self.on_game_finish) registry.register(Hook.ON_POST_GAME_FINISH, self.on_post_game_finish) - def on_post_render(self, context: HookContext) -> None: - """Handle ``POST_RENDER`` — upload rendered video to R2.""" - if not self._config.get("upload_video"): - return + def upload( + self, path: Path, *, metadata: dict[str, Any] | None = None + ) -> str: + """Upload a rendered video to Cloudflare R2 and return its public URL. + + Implements the :class:`reeln.plugins.capabilities.Uploader` protocol + so the plugin can be used by ``reeln queue publish`` for truthful + per-target status reporting. + + Raises: + UploaderSkipped: when ``upload_video`` is disabled in the plugin + config. The publish orchestrator maps this to + ``PublishStatus.SKIPPED`` (distinct from failure). + FileNotFoundError: when ``path`` does not exist on disk. + RuntimeError: when R2 credentials cannot be resolved from the + configured environment variables. + r2.R2Error: when the underlying R2 upload fails. + """ + del metadata # Not used today; accepted to satisfy Uploader protocol. - result = context.data.get("result") - if result is None: - log.warning("Cloudflare plugin: no result in context data, skipping") - return + if not self._config.get("upload_video"): + raise UploaderSkipped( + "upload_video disabled in cloudflare plugin config" + ) - output = getattr(result, "output", None) - if output is None or not output.exists(): - log.warning( - "Cloudflare plugin: output file not found: %s, skipping", output + if not path.exists(): + raise FileNotFoundError( + f"Cloudflare upload source not found: {path}" ) - return credentials = self._resolve_credentials() if credentials is None: - return - + raise RuntimeError( + "Cloudflare R2 credentials not configured " + "(environment variables missing)" + ) access_key_id, secret_access_key = credentials prefix = self._config.get("upload_prefix", "") - filename = output.name - key = f"{prefix}/{filename}" if prefix else filename + key = f"{prefix}/{path.name}" if prefix else path.name if self._config.get("dry_run"): base = self._config.get("public_url_base", "").rstrip("/") - url = f"{base}/{key}" + synthetic_url = f"{base}/{key}" log.info( "Cloudflare plugin: [DRY RUN] would upload %s → %s", - output, - url, + path, + synthetic_url, ) - return + return synthetic_url config = r2.R2Config( endpoint=self._config.get("r2_endpoint", ""), @@ -156,15 +172,52 @@ def on_post_render(self, context: HookContext) -> None: upload_max_kbps=int(self._config.get("upload_max_kbps", 0)), ) + public_url = r2.upload_file(config, path, key) + self._uploaded_keys.append(key) + log.info("Cloudflare plugin: uploaded %s → %s", path, public_url) + return public_url + + def on_post_render(self, context: HookContext) -> None: + """Handle ``POST_RENDER`` — delegate to :meth:`upload`. + + Preserves the auto-publish-during-render contract: exceptions are + swallowed (an upload failure must never break the render pipeline) + and ``context.shared["video_url"]`` is populated on real success + so downstream plugins (e.g. meta reels) can consume it. + Dry-run behavior is preserved: the synthetic URL is logged but + NOT written to ``context.shared`` — downstream plugins should not + believe a real object exists when one does not. + """ + result = context.data.get("result") + if result is None: + log.warning( + "Cloudflare plugin: no result in context data, skipping" + ) + return + + output = getattr(result, "output", None) + if output is None or not output.exists(): + log.warning( + "Cloudflare plugin: output file not found: %s, skipping", + output, + ) + return + try: - public_url = r2.upload_file(config, output, key) - except r2.R2Error as exc: - log.warning("Cloudflare plugin: upload failed (non-fatal): %s", exc) + public_url = self.upload( + output, metadata=context.data.get("publish_metadata") + ) + except UploaderSkipped as exc: + log.info("Cloudflare plugin: %s", exc) + return + except Exception as exc: + log.warning( + "Cloudflare plugin: upload failed (non-fatal): %s", exc + ) return - self._uploaded_keys.append(key) - context.shared["video_url"] = public_url - log.info("Cloudflare plugin: uploaded %s → %s", output, public_url) + if not self._config.get("dry_run"): + context.shared["video_url"] = public_url def _resolve_credentials(self) -> tuple[str, str] | None: """Read R2 credentials from environment variables named in config. diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index fb4611b..5175570 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -137,6 +137,40 @@ def test_feature_flag_missing(self) -> None: plugin.on_post_render(context) assert "video_url" not in context.shared + def test_feature_flag_disabled_with_valid_result_swallows_skipped( + self, + video_file: Path, + r2_env: None, + caplog: pytest.LogCaptureFixture, + ) -> None: + """on_post_render catches UploaderSkipped from upload() and returns. + + This covers the auto-publish-during-render path: when upload_video + is disabled but the render pipeline still reaches on_post_render + with a valid result, the wrapper must swallow UploaderSkipped + (a render failure would break the pipeline) and not populate + video_url. + """ + plugin = CloudflarePlugin( + { + "upload_video": False, + "r2_endpoint": "https://x.r2.cloudflarestorage.com", + "r2_bucket": "b", + "r2_access_key_env": "R2_ACCESS_KEY_ID", + "r2_secret_key_env": "R2_SECRET_ACCESS_KEY", + "public_url_base": "https://cdn.example.com", + } + ) + result = FakeRenderResult(output=video_file) + context = _make_context(data={"result": result}) + + with caplog.at_level(logging.INFO): + plugin.on_post_render(context) + + assert "video_url" not in context.shared + # The UploaderSkipped message should be logged at INFO. + assert "upload_video" in caplog.text + class TestOnPostRenderMissingData: def test_missing_result( @@ -777,3 +811,195 @@ def test_hint_mentions_env_vars(self) -> None: assert "environment variables" in results[0].hint assert "r2_access_key_env" in results[0].hint assert "r2_secret_key_env" in results[0].hint + + +# ------------------------------------------------------------------ +# upload() — Uploader protocol implementation (manual publish path) +# ------------------------------------------------------------------ + + +class TestUpload: + """Tests for the ``upload()`` method used by ``reeln queue publish``. + + These cover the new path where the publish orchestrator calls the + plugin directly (instead of emitting POST_RENDER). They must raise on + failure and return the public URL on success — see + ``reeln.core.queue.publish_queue_item``. + """ + + def test_upload_video_disabled_raises_skipped( + self, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + from reeln.plugins.capabilities import UploaderSkipped + + plugin_config["upload_video"] = False + plugin = CloudflarePlugin(plugin_config) + + with pytest.raises(UploaderSkipped, match="upload_video"): + plugin.upload(video_file) + + def test_upload_video_missing_raises_skipped( + self, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + from reeln.plugins.capabilities import UploaderSkipped + + del plugin_config["upload_video"] + plugin = CloudflarePlugin(plugin_config) + + with pytest.raises(UploaderSkipped, match="upload_video"): + plugin.upload(video_file) + + def test_upload_missing_source_raises_file_not_found( + self, + plugin_config: dict[str, Any], + tmp_path: Path, + r2_env: None, + ) -> None: + missing = tmp_path / "nonexistent.mp4" + plugin = CloudflarePlugin(plugin_config) + + with pytest.raises(FileNotFoundError, match="nonexistent.mp4"): + plugin.upload(missing) + + def test_upload_missing_credentials_raises_runtime_error( + self, + plugin_config: dict[str, Any], + video_file: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.delenv("R2_ACCESS_KEY_ID", raising=False) + monkeypatch.delenv("R2_SECRET_ACCESS_KEY", raising=False) + plugin = CloudflarePlugin(plugin_config) + + with pytest.raises(RuntimeError, match="credentials"): + plugin.upload(video_file) + + def test_upload_dry_run_returns_synthetic_url( + self, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + plugin_config["dry_run"] = True + plugin = CloudflarePlugin(plugin_config) + + with patch("reeln_cloudflare_plugin.r2.upload_file") as mock_upload: + url = plugin.upload(video_file) + + assert url == "https://cdn.example.com/highlight.mp4" + mock_upload.assert_not_called() + assert plugin._uploaded_keys == [] + + def test_upload_dry_run_strips_trailing_slash_on_base( + self, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + plugin_config["dry_run"] = True + plugin_config["public_url_base"] = "https://cdn.example.com/" + plugin = CloudflarePlugin(plugin_config) + + url = plugin.upload(video_file) + + assert url == "https://cdn.example.com/highlight.mp4" + + @patch("reeln_cloudflare_plugin.r2.upload_file") + def test_upload_success_returns_real_url( + self, + mock_upload: Any, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + mock_upload.return_value = "https://cdn.example.com/highlight.mp4" + plugin = CloudflarePlugin(plugin_config) + + url = plugin.upload(video_file) + + assert url == "https://cdn.example.com/highlight.mp4" + mock_upload.assert_called_once() + assert plugin._uploaded_keys == ["highlight.mp4"] + + @patch("reeln_cloudflare_plugin.r2.upload_file") + def test_upload_with_prefix_uses_prefixed_key( + self, + mock_upload: Any, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + mock_upload.return_value = "https://cdn.example.com/reels/highlight.mp4" + plugin_config["upload_prefix"] = "reels" + plugin = CloudflarePlugin(plugin_config) + + plugin.upload(video_file) + + call_args = mock_upload.call_args + assert call_args[0][2] == "reels/highlight.mp4" + assert plugin._uploaded_keys == ["reels/highlight.mp4"] + + @patch("reeln_cloudflare_plugin.r2.upload_file") + def test_upload_accepts_metadata_kwarg( + self, + mock_upload: Any, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + """The Uploader protocol requires a metadata kwarg; we accept and ignore it.""" + mock_upload.return_value = "https://cdn.example.com/highlight.mp4" + plugin = CloudflarePlugin(plugin_config) + + url = plugin.upload( + video_file, + metadata={"title": "Goal!", "description": "What a shot"}, + ) + + assert url == "https://cdn.example.com/highlight.mp4" + + @patch("reeln_cloudflare_plugin.r2.upload_file") + def test_upload_r2_error_propagates( + self, + mock_upload: Any, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + mock_upload.side_effect = R2Error("bucket not found") + plugin = CloudflarePlugin(plugin_config) + + with pytest.raises(R2Error, match="bucket not found"): + plugin.upload(video_file) + + assert plugin._uploaded_keys == [] + + @patch("reeln_cloudflare_plugin.r2.upload_file") + def test_upload_passes_config_to_r2( + self, + mock_upload: Any, + plugin_config: dict[str, Any], + video_file: Path, + r2_env: None, + ) -> None: + mock_upload.return_value = "https://cdn.example.com/highlight.mp4" + plugin_config["r2_region"] = "us-east-1" + plugin_config["upload_max_kbps"] = 1000 + plugin = CloudflarePlugin(plugin_config) + + plugin.upload(video_file) + + r2_config = mock_upload.call_args[0][0] + assert r2_config.region == "us-east-1" + assert r2_config.upload_max_kbps == 1000 + assert r2_config.endpoint == "https://account-id.r2.cloudflarestorage.com" + assert r2_config.bucket == "test-bucket" + assert r2_config.public_url_base == "https://cdn.example.com" + assert r2_config.access_key_id == "test-access-key" + assert r2_config.secret_access_key == "test-secret-key" From 6f1a36c5dd7a2edb1f78cd4441363d7b318e7633 Mon Sep 17 00:00:00 2001 From: jremitz Date: Mon, 20 Apr 2026 20:09:55 -0500 Subject: [PATCH 2/2] fix: resolve ruff RUF043 and SIM117 lint errors Co-Authored-By: Claude --- tests/unit/test_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index 5175570..4a49267 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -864,7 +864,7 @@ def test_upload_missing_source_raises_file_not_found( missing = tmp_path / "nonexistent.mp4" plugin = CloudflarePlugin(plugin_config) - with pytest.raises(FileNotFoundError, match="nonexistent.mp4"): + with pytest.raises(FileNotFoundError, match=r"nonexistent\.mp4"): plugin.upload(missing) def test_upload_missing_credentials_raises_runtime_error(