From 6cc0714e71e6d4d2741706f2b4ca51d403c8cce9 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Mon, 15 Jun 2026 20:17:17 -0400 Subject: [PATCH 01/14] feat: add ProcessExecutionReport.notify/.notify_errors stubs Placeholder methods for sending the execution report via email (configurable style and content) and webhook (JSON). Both raise NotImplementedError for now; signatures reference the existing SMTPConfig/HTMLEmailStyle/WebhookConfig types to avoid churn when implemented. --- src/processes/execution_report.py | 63 +++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index c56ecca..5288321 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -9,7 +9,9 @@ from .task import TaskResult, TaskStatus if TYPE_CHECKING: + from .email_config import HTMLEmailStyle, SMTPConfig from .process import Process + from .webhook_config import WebhookConfig def _json_default(obj: Any) -> Any: @@ -165,3 +167,64 @@ def to_json(self, *, indent: int | None = None, **dumps_kwargs: Any) -> str: """ dumps_kwargs.pop("default", None) return json.dumps(self, default=_json_default, indent=indent, **dumps_kwargs) + + def notify( + self, + *, + email: SMTPConfig | None = None, + email_style: HTMLEmailStyle | None = None, + webhook: WebhookConfig | None = None, + ) -> None: + """Send the full execution report via the configured channels. + + Email delivery will be configurable in presentation (``email_style``) + and in the information included; webhook delivery will POST the report + as JSON (see :meth:`to_json`). At least one channel must be provided. + + Not implemented yet. + + Parameters + ---------- + email : SMTPConfig, optional + SMTP transport for the email report. ``None`` disables email. + email_style : HTMLEmailStyle, optional + HTML presentation settings for the email report. + webhook : WebhookConfig, optional + Webhook transport for the JSON report. ``None`` disables webhook. + + Raises + ------ + NotImplementedError + Always, until report notification is implemented. + """ + raise NotImplementedError("ProcessExecutionReport.notify is not implemented yet.") + + def notify_errors( + self, + *, + email: SMTPConfig | None = None, + email_style: HTMLEmailStyle | None = None, + webhook: WebhookConfig | None = None, + ) -> None: + """Send only the errored entries of the report via the configured channels. + + Same configuration as :meth:`notify`, but the payload is restricted to + tasks whose status is ``ERRORED`` (see :attr:`errored`). + + Not implemented yet. + + Parameters + ---------- + email : SMTPConfig, optional + SMTP transport for the email report. ``None`` disables email. + email_style : HTMLEmailStyle, optional + HTML presentation settings for the email report. + webhook : WebhookConfig, optional + Webhook transport for the JSON report. ``None`` disables webhook. + + Raises + ------ + NotImplementedError + Always, until report notification is implemented. + """ + raise NotImplementedError("ProcessExecutionReport.notify_errors is not implemented yet.") From d61d0c348380729af4f01eac7b54fe90e55b33fc Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Mon, 15 Jun 2026 20:37:50 -0400 Subject: [PATCH 02/14] docs: add report notifications design doc Situation analysis, objective, proposed solution (narrow TaskChannel/ReportChannel interfaces, reuse transport configs), rationale, implementation plan, and a simplified alternative (configs directly; ReportEmailStyle as sibling vs subclass). --- report-notifications-design.md | 184 +++++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 report-notifications-design.md diff --git a/report-notifications-design.md b/report-notifications-design.md new file mode 100644 index 0000000..e62c1b2 --- /dev/null +++ b/report-notifications-design.md @@ -0,0 +1,184 @@ +# Diseño: notificación de `ProcessExecutionReport` (email + webhook) + +> Branch: `feature/report-notifications` +> Estado: stubs `notify` / `notify_errors` ya creados (lanzan `NotImplementedError`). + +## 1. Análisis de situación + +El proyecto ya tiene infraestructura de notificación, pero **orientada a `Task`** y +**dirigida por logging**: + +- `NotificationChannel` (ABC) expone un único método: `build_handler(task_name) -> logging.Handler`. +- `_FileChannel`, `EmailChannel`, `WebhookChannel` producen handlers que un `Task` + adjunta a su logger. Emiten **por cada error**, mientras la tarea corre. +- El render de email (`_HTMLEmailFormatter` + templates en `themes/styles`, + `themes/palettes`, `themes/languages`) está hecho para **un solo fallo**: + substituciones `exception`, `traceback_highlight`, `traced_vars`, + `downstream_items`, etc. +- Configs de transporte/presentación: `SMTPConfig`, `HTMLEmailStyle`, + `WebhookConfig`. + +`ProcessExecutionReport` ya sabe serializarse (`to_json()`, lossless), y se acaban +de agregar los stubs `notify()` / `notify_errors()`. + +**Tensión central:** los dos consumidores tienen modos de entrega distintos. + +| | `Task` | `ProcessExecutionReport` | +|---|---|---| +| Modo | streaming / push | one-shot / pull | +| Dirigido por | `logging.LogRecord` | objeto completo en memoria | +| Cuándo | cada error, durante el run | una vez, al terminar | +| Contenido | un fallo | tabla de N tasks (success/errored/skipped, tiempos) | + +Forzar el reporte por el mismo `build_handler`/`SMTPHandler` obligaría a +**fabricar `LogRecord` falsos** → impedance mismatch. Hay que evitarlo. + +## 2. Objetivo + +Permitir que un `ProcessExecutionReport` se notifique vía: + +- **Email**, configurable en **estilo** (layout/paleta/idioma, reusando el sistema + de themes) y en **información** enviada. +- **Webhook**, como **JSON** (reusando `to_json()`). + +Con dos entradas: `notify()` (reporte completo) y `notify_errors()` (solo entries +`ERRORED`). Idealmente, que un mismo "canal" se pueda anotar tanto a una `Task` +como a un `ProcessExecutionReport`, configurando el destino una sola vez, y +extensible a nuevos destinos (Slack/Teams/etc.). + +## 3. Solución propuesta + +### 3.1 Qué se reutiliza y qué no + +| Pieza | Reutilizar | Nota | +|---|---|---| +| `SMTPConfig` | ✅ tal cual | Transporte puro, desacoplado de Task/logging. | +| `WebhookConfig` | ✅ tal cual | Transporte puro (URL/JSON). | +| Sistema de themes (`themes/`) | ✅ | Assets de estilo/paleta/idioma. | +| `HTMLEmailStyle` | 🟡 como selector de estilo | `traced_vars_frame_filter` es específico de fallo; evaluar un `ReportEmailStyle` o ignorar ese campo. | +| `_HTMLEmailFormatter` + templates de error + handlers | ❌ | Renderizan un solo fallo y son `LogRecord`-driven. El reporte necesita template y formatter propios, y envío one-shot. | + +### 3.2 Abstracción de canales: dos interfaces angostas (no un ABC gordo) + +```text +TaskChannel -> build_handler(task_name) -> logging.Handler (= NotificationChannel actual) +ReportChannel -> send_report(report, *, errors_only) -> None +``` + +- `EmailChannel` / `WebhookChannel` implementan **ambas** (mismo destino, dos verbos), + reusando internamente `SMTPConfig` / `WebhookConfig`. +- `_FileChannel` implementa **solo** `TaskChannel`. +- Composición sobre un ABC único: así ningún canal se ve forzado a un método que no + tiene sentido (evita reintroducir `NotImplementedError`). + +### 3.3 Render del reporte (email) + +- Nuevo template "reporte" (tabla de tasks: nombre, estado, intentos, duración, + y sección de errores con su contexto) reusando los directorios de + styles/palettes/languages. +- Nuevo formatter que consume un `ProcessExecutionReport` (no un `LogRecord`). +- Envío SMTP one-shot (no vía `logging.handlers.SMTPHandler`). + +### 3.4 Render del reporte (webhook) + +- POST del payload `to_json()` (ya lossless). `errors_only` filtra a entries `ERRORED`. + +## 4. Razón + +- **Separar transporte de presentación de entrega.** El transporte + (`SMTPConfig`/`WebhookConfig`) es lo genuinamente desacoplado y reutilizable; el + render por-fallo no lo es. Reutilizar el transporte evita duplicar auth/URL/TLS. +- **Respetar streaming vs one-shot.** Son modos de entrega distintos; un único + `build_handler` no modela ambos sin hacks (LogRecords falsos). +- **Interfaces angostas > ABC gordo.** Permite "anotar el mismo canal a Task o a + Report" sin métodos irrelevantes, y deja extensibilidad limpia (un destino nuevo + implementa lo que aplique). +- **No abstraer con un solo caso.** Empezar con configs directas y extraer canales + cuando aparezca el 2º/3er destino reduce el riesgo de sobre-diseño. + +## 5. Plan de implementación + +**Fase 0 — hecho.** Stubs `notify` / `notify_errors` con firmas que referencian +`SMTPConfig`/`HTMLEmailStyle`/`WebhookConfig` (sin churn futuro). + +**Fase 1 — Webhook (lo más simple).** +- Implementar `notify`/`notify_errors` para el caso webhook usando `to_json()` + + `WebhookConfig`. `errors_only` filtra entries. +- Tests: payload correcto, filtrado de errores, no-op si no hay webhook. + +**Fase 2 — Email (reporte completo).** +- Nuevo template de reporte + nuevo formatter (tabla de tasks + detalle de errores), + reusando themes y `HTMLEmailStyle` como selector. +- Envío SMTP one-shot con `SMTPConfig`. +- Decidir: ¿qué es "información configurable"? (columnas/secciones, incluir + tracebacks sí/no, etc.). ¿`HTMLEmailStyle` o nuevo `ReportEmailStyle`? +- Tests: render por estilo/paleta/idioma; modo errors-only. + +**Fase 3 — Abstracción de canales.** +- Introducir `ReportChannel`; renombrar/alias `NotificationChannel` → `TaskChannel` + (manteniendo compat). +- `EmailChannel`/`WebhookChannel` implementan ambas interfaces. +- `notify`/`notify_errors` aceptan objetos canal además de configs. + +**Fase 4 — Docs + ejemplos.** +- Documentar en `docs/` y agregar ejemplo en `examples/`. + +## 6. Decisiones abiertas + +1. ¿`HTMLEmailStyle` reutilizado o `ReportEmailStyle` nuevo (por `traced_vars_frame_filter`)? +2. ¿Qué exactamente es "información configurable" del email del reporte? +3. ¿`send_report` síncrono y que propague errores de envío, o best-effort silencioso + como hacen los handlers de logging hoy? +4. ¿`notify` recibe configs (simple) o canales (unificado) en la primera versión? + +## 7. Alternativa simplificada (sin abstracción de canales) + +En lugar de introducir `TaskChannel` / `ReportChannel`, `notify` y `notify_errors` +reciben **directamente** los objetos de transporte que ya existen — exactamente las +firmas de los stubs actuales: + +```python +report.notify( + email=SMTPConfig(...), + email_style=HTMLEmailStyle(...), # o ReportEmailStyle, ver 7.1 + webhook=WebhookConfig(...), +) +report.notify_errors(webhook=WebhookConfig(...)) +``` + +El reporte arma internamente su payload (HTML para email, `to_json()` para webhook) +y lo envía. **No** hay interfaces de canal, **no** hay objeto que se anote a la vez a +`Task` y a `Report`. + +**Pros** +- Mínimo: es literalmente implementar los stubs tal cual; cero abstracción nueva. +- Aprovecha lo genuinamente reutilizable (`SMTPConfig`/`WebhookConfig` son transporte + puro), que es donde está casi todo el valor de reuso. +- En espíritu con una lib "lightweight, zero-dep". + +**Contras** +- No unifica "configurar un destino una vez y usarlo en Task y en Report". +- Cada destino nuevo (Slack/Teams/SNS) es **otro parámetro** de `notify`, no una clase + polimórfica. Escala mal si se esperan muchos destinos. +- Algo de duplicación entre el envío del lado-Task (handlers) y el lado-Report. + +**Cuándo preferirla:** si la notificación del reporte es la única necesidad nueva y no +se prevén muchos destinos. **Recomendación:** empezar por aquí; extraer canales +(sección 3.2) recién cuando aparezca el 2º/3er destino o se quiera la ergonomía de +"un canal para ambos". Es reversible: las firmas con configs pueden coexistir o migrar +a aceptar canales después. + +### 7.1 `ReportEmailStyle`: ¿subclase o hermanas? + +`HTMLEmailStyle` tiene `style` / `palette` / `language` (comunes a cualquier email) y +`traced_vars_frame_filter` (**específico de un fallo**, sin sentido en un reporte). + +| Opción | Veredicto | Razón | +|---|---|---| +| **A. `ReportEmailStyle(HTMLEmailStyle)`** (subclase) | ❌ Evitar | Heredaría `traced_vars_frame_filter`, que no aplica. Herencia debe **añadir**, no heredar-e-ignorar. Un reporte "no es un tipo de" estilo-de-error (LSP smell). | +| **B. Hermanas bajo base común `EmailStyle`** | ✅ Si aporta campos propios | Base `EmailStyle` = `style`/`palette`/`language`. `HTMLEmailStyle(EmailStyle)` añade `traced_vars_frame_filter`; `ReportEmailStyle(EmailStyle)` añade lo específico del reporte (qué columnas/secciones, incluir tracebacks, etc.). Sin campos muertos. | +| **C. Reusar `HTMLEmailStyle` tal cual** (ignorar el campo) | ✅ Si no aporta nada aún | Lo más simple. Trade-off: arrastra un campo semánticamente muerto en el contexto de reporte. | + +**Recomendación:** empezar con **C** (reusar `HTMLEmailStyle`) en la versión simplificada; +migrar a **B** (extraer base `EmailStyle` + hermanas) en cuanto el email del reporte +necesite sus propios knobs. **Nunca A.** From f525a4baf704eaabf57ab4d8ac0a6f0c3787070e Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Mon, 15 Jun 2026 20:42:08 -0400 Subject: [PATCH 03/14] docs: remove obsolete FEATURE_PLAN1.MD --- FEATURE_PLAN1.MD | 86 ------------------------------------------------ 1 file changed, 86 deletions(-) delete mode 100644 FEATURE_PLAN1.MD diff --git a/FEATURE_PLAN1.MD b/FEATURE_PLAN1.MD deleted file mode 100644 index f04a0e6..0000000 --- a/FEATURE_PLAN1.MD +++ /dev/null @@ -1,86 +0,0 @@ -USER: Could ProcessResult be ditched and replaced by ProcessExecutionReport? Show impacts. Don't code yet. - -Claude: Could ProcessResult be replaced by ProcessExecutionReport? - -Structurally, yes — ProcessExecutionReport.entries is a superset of what ProcessResult carries (it adds function/args/kwargs/status/elapsed_seconds/attempts for every task, success/errored/skipped alike, in topological order). The raw data ProcessRunner already collects (passed_results, failed_results, skipped_tasks, process.tasks) is exactly what from_result consumes — so ProcessRunner.run() could build a ProcessExecutionReport directly instead of a ProcessResult, without from_result needing a ProcessResult input at all (avoids the current process.py ↔ execution_report.py TYPE_CHECKING coupling — execution_report.py becomes self-sufficient). - -What breaks / changes - -1. Lost: raw Exception object access -TaskResult.exception is a live Exception instance (tests do isinstance(result.exception, TimeoutError)). TaskReportEntry.error is ErrorData | None, whose .exception field is a str(exception). Any caller that needs to isinstance()-check or re-raise the original exception loses that — ErrorData would need an added field, or TaskReportEntry would need to carry the raw TaskResult too. - -2. API shape changes for every consumer -- result.passed_tasks_results["x"].result → report.successes["x"].result (or report.entries["x"].result) -- result.failed_tasks (a set[str]) → no direct equivalent; becomes set(report.errored) | set(report.skipped) -- result.errored_tasks / result.skipped_tasks → set(report.errored) / set(report.skipped) -- result.failed_tasks_results["x"] (a TaskResult) → report.entries["x"].error (an ErrorData, narrower — see #1) -- .worked / boolean checks → .status == TaskStatus.SUCCESS etc. - -3. Test impact — large -Every test file that touches these attributes needs rewriting: -test_args_kwargs.py, test_complex_dag_failures.py, test_normal_run_no_errors.py, test_parallel_race_conditions.py, test_timeout_retry.py, plus 3 manual-test scripts. This is the bulk of the effort — dozens of assertion sites. - -4. Docs impact -README's ProcessResult section (added in this session) and any usage examples would need replacing with ProcessExecutionReport equivalents; docs/reference.md would drop the ProcessResult mkdocstrings entry. - -5. Breaking change vs. additive discipline -This session deliberately kept everything additive (FEATURE_PLAN's stance). Removing ProcessResult and changing Process.run()'s return type is a hard breaking change — would warrant a major version bump and a migration note, not a patch/minor release. - -6. Minor ergonomics loss -Simple "did everything pass?" checks (if not result.failed_tasks) become if not report.errored and not report.skipped unless a convenience property (e.g. report.ok) is added. - -Net assessment - -Feasible and arguably a cleaner unified model long-term, but it's a breaking API change with wide blast radius across tests/docs, and needs at least one new field on ErrorData/TaskReportEntry to avoid losing the raw exception object. If you want this, I'd suggest either (a) a v6 major bump doing a clean replacement, or (b) keep ProcessResult as a thin deprecated wrapper for one release while Process.run() returns ProcessExecutionReport and ProcessResult.from_report(...) is offered for back-compat. - -✻ Cooked for 49s - -❯ /advisor off - ⎿ Advisor disabled - -❯ is ProcessExecutionReport being created anywhere yet? - - Searched for 1 pattern - -Every test file that touches these attributes needs rewriting: -test_args_kwargs.py, test_complex_dag_failures.py, test_normal_run_no_errors.py, test_parallel_race_conditions.py, test_timeout_retry.py, plus 3 manual-test scripts. This is the bulk of the effort — dozens of assertion sites. - -4. Docs impact -README's ProcessResult section (added in this session) and any usage examples would need replacing with ProcessExecutionReport equivalents; docs/reference.md would drop the ProcessResult mkdocstrings entry. - -5. Breaking change vs. additive discipline -This session deliberately kept everything additive (FEATURE_PLAN's stance). Removing ProcessResult and changing Process.run()'s return type is a hard breaking change — would warrant a major version bump and a migration note, not a patch/minor release. - -6. Minor ergonomics loss -Simple "did everything pass?" checks (if not result.failed_tasks) become if not report.errored and not report.skipped unless a convenience property (e.g. report.ok) is added. - -Net assessment - -Feasible and arguably a cleaner unified model long-term, but it's a breaking API change with wide blast radius across tests/docs, and needs at least one new field on ErrorData/TaskReportEntry to avoid losing the raw exception object. If you want this, I'd suggest either (a) a v6 major bump doing a clean replacement, or (b) keep ProcessResult as a thin deprecated wrapper for one release while Process.run() returns ProcessExecutionReport and ProcessResult.from_report(...) is offered for back-compat. - -USER: failed_results and passed_results and failed_tasks are separated. Shouldn't they be in a single dict with k:v task.name:task_result_obj ? - -Claude: Yes — and it'd fix a latent inconsistency: currently failed_tasks (a set) = errored_tasks ∪ skipped_tasks, but failed_results (a dict) only has entries for errored tasks, not skipped ones. So failed_tasks and failed_results.keys() silently diverge — anyone iterating failed_tasks and indexing into failed_results will KeyError on skipped tasks. - -Proposal - -Maintain one internal dict in ProcessRunner: self.results: dict[str, TaskResult], one entry per task regardless of outcome (success, errored, or skipped — synthesizing a TaskResult for skipped tasks the same way the parallel defensive branch already does for unexpected exceptions). - -Discriminator problem: TaskResult.worked: bool alone can't tell errored apart from skipped (both worked=False). Two options: -- Implicit: attempts == 0 ⇒ skipped. Works, but relies on an unstated convention — fragile if someone constructs a TaskResult manually. -- Explicit (cleaner): give TaskResult a status: TaskStatus field (reuse the enum from execution_report.py). Then results[name].status is authoritative everywhere. - -Impact - -- Internals: _is_done, _all_deps_met, _is_unrunnable currently check membership across passed_results/failed_tasks/skipped_tasks/failed_results — all rewritten to query self.results by status. Net simplification. -- ProcessResult public API: can stay byte-for-byte unchanged. passed_tasks_results, failed_tasks, skipped_tasks, errored_tasks, failed_tasks_results all become derived views filtered from self.results at the end of run() — zero breaking change, fixes the divergence bug above as a side effect. -- Ties into the ProcessExecutionReport question: if TaskResult gains status: TaskStatus, then self.results is already shaped almost exactly like ProcessExecutionReport.entries (just missing name/function/args/kwargs, which come from Task). from_result/ProcessRunner.run() could build the report directly from this dict with minimal mapping — makes that future migration much cheaper. -- Effort: moderate, contained to process.py (ProcessRunner) + adding a field to TaskResult (additive, default-valued — safe). No test changes needed if ProcessResult's shape is preserved. - -This seems like a good incremental step worth doing on its own regardless of the ProcessExecutionReport replacement decision. - -USER: Tasks could have an extra attr TaskStatus already defined. Status could be expanded to PENDING, SUCCESS, ERRORED, SKIPPED. These changes should simplify or solve some of the problems stated by you. - - - -INSTRUCTION: IMPLEMENT BOTH THINGS. Removal of ProcessResult and use of single dict self.results. \ No newline at end of file From a40768a849a3b82bc3d0aed5ffed051291c6fe8f Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Mon, 15 Jun 2026 23:05:28 -0400 Subject: [PATCH 04/14] feat: add ReportChannel architecture for report notifications Introduce ReportChannel (one-shot send_report) alongside the existing NotificationChannel (streaming build_handler), and a channel-agnostic ReportContent config (show_traceback/show_traced_vars). EmailChannel and WebhookChannel now implement both roles; _FileChannel stays task-only. ProcessExecutionReport.notify/notify_errors iterate the given channels and call send_report (errors_only False/True). The report stays a frozen value object; channels are passed in, not stored. Message rendering is deferred: the built-in channels' send_report raise NotImplementedError for now. --- src/processes/__init__.py | 2 + src/processes/execution_report.py | 68 ++++---------- src/processes/notification_channels.py | 121 ++++++++++++++++++++++--- tests/test_report_notify_dispatch.py | 69 ++++++++++++++ 4 files changed, 198 insertions(+), 62 deletions(-) create mode 100644 tests/test_report_notify_dispatch.py diff --git a/src/processes/__init__.py b/src/processes/__init__.py index 0f8f50e..d0a3380 100644 --- a/src/processes/__init__.py +++ b/src/processes/__init__.py @@ -17,6 +17,8 @@ from .execution_report import TaskReportEntry as TaskReportEntry from .notification_channels import EmailChannel as EmailChannel from .notification_channels import NotificationChannel as NotificationChannel +from .notification_channels import ReportChannel as ReportChannel +from .notification_channels import ReportContent as ReportContent from .notification_channels import WebhookChannel as WebhookChannel from .process import Process as Process from .task import Task as Task diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index 5288321..8c6aac9 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -9,9 +9,8 @@ from .task import TaskResult, TaskStatus if TYPE_CHECKING: - from .email_config import HTMLEmailStyle, SMTPConfig + from .notification_channels import ReportChannel from .process import Process - from .webhook_config import WebhookConfig def _json_default(obj: Any) -> Any: @@ -168,63 +167,30 @@ def to_json(self, *, indent: int | None = None, **dumps_kwargs: Any) -> str: dumps_kwargs.pop("default", None) return json.dumps(self, default=_json_default, indent=indent, **dumps_kwargs) - def notify( - self, - *, - email: SMTPConfig | None = None, - email_style: HTMLEmailStyle | None = None, - webhook: WebhookConfig | None = None, - ) -> None: - """Send the full execution report via the configured channels. + def notify(self, *channels: ReportChannel) -> None: + """Deliver the full report through each channel, in order. - Email delivery will be configurable in presentation (``email_style``) - and in the information included; webhook delivery will POST the report - as JSON (see :meth:`to_json`). At least one channel must be provided. - - Not implemented yet. + Each channel renders and sends the report itself (email, webhook, ...). + What detail is included is configured per channel (see ``ReportContent``). Parameters ---------- - email : SMTPConfig, optional - SMTP transport for the email report. ``None`` disables email. - email_style : HTMLEmailStyle, optional - HTML presentation settings for the email report. - webhook : WebhookConfig, optional - Webhook transport for the JSON report. ``None`` disables webhook. - - Raises - ------ - NotImplementedError - Always, until report notification is implemented. + *channels : ReportChannel + Channels to deliver the report to. No-op if none are given. """ - raise NotImplementedError("ProcessExecutionReport.notify is not implemented yet.") - - def notify_errors( - self, - *, - email: SMTPConfig | None = None, - email_style: HTMLEmailStyle | None = None, - webhook: WebhookConfig | None = None, - ) -> None: - """Send only the errored entries of the report via the configured channels. + for channel in channels: + channel.send_report(self, errors_only=False) - Same configuration as :meth:`notify`, but the payload is restricted to - tasks whose status is ``ERRORED`` (see :attr:`errored`). + def notify_errors(self, *channels: ReportChannel) -> None: + """Deliver only the ``ERRORED`` entries through each channel, in order. - Not implemented yet. + Same as :meth:`notify`, but each channel restricts the payload to tasks + whose status is ``ERRORED`` (see :attr:`errored`). Parameters ---------- - email : SMTPConfig, optional - SMTP transport for the email report. ``None`` disables email. - email_style : HTMLEmailStyle, optional - HTML presentation settings for the email report. - webhook : WebhookConfig, optional - Webhook transport for the JSON report. ``None`` disables webhook. - - Raises - ------ - NotImplementedError - Always, until report notification is implemented. + *channels : ReportChannel + Channels to deliver the errored report to. No-op if none are given. """ - raise NotImplementedError("ProcessExecutionReport.notify_errors is not implemented yet.") + for channel in channels: + channel.send_report(self, errors_only=True) diff --git a/src/processes/notification_channels.py b/src/processes/notification_channels.py index 8081e8a..48255e4 100644 --- a/src/processes/notification_channels.py +++ b/src/processes/notification_channels.py @@ -2,6 +2,8 @@ import logging from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import TYPE_CHECKING from ._email_internals import _build_task_email_handler from ._logfile_formatting import _TaskLogfileFormatter @@ -9,6 +11,52 @@ from .email_config import HTMLEmailStyle, SMTPConfig from .webhook_config import WebhookConfig +if TYPE_CHECKING: + from .execution_report import ProcessExecutionReport + + +@dataclass(frozen=True) +class ReportContent: + """What detail a report notification includes. + + Channel-agnostic content selection, shared by every ``ReportChannel``. + Construct once and pass the same instance to several channels for uniform + content, or give each channel its own for per-destination verbosity. + + Attributes + ---------- + show_traceback : bool + Include each failure's full traceback. Defaults to ``True``. + show_traced_vars : bool + Include each failure's traced local variables. Defaults to ``True``. + """ + + show_traceback: bool = True + show_traced_vars: bool = True + + +class ReportChannel(ABC): + """Base class for channels that deliver a finished ``ProcessExecutionReport``. + + Unlike ``NotificationChannel`` (which builds a streaming ``logging.Handler`` + for a single ``Task``), a report channel sends a complete report **once**, + after the run. ``ProcessExecutionReport.notify`` / ``notify_errors`` iterate + the channels they are given and call ``send_report`` on each. + """ + + @abstractmethod + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + """Deliver ``report`` to this channel's destination. + + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + errors_only : bool + If True, only the ``ERRORED`` entries are sent; otherwise the whole + report is sent. + """ + class NotificationChannel(ABC): """Base class for task notification channels. @@ -96,8 +144,13 @@ def build_handler(self, task_name: str) -> logging.Handler: return handler -class EmailChannel(NotificationChannel): - """Notification channel that sends an HTML email alert on task failure. +class EmailChannel(NotificationChannel, ReportChannel): + """Channel that sends HTML email: a per-task failure alert and/or a report. + + As a ``NotificationChannel`` it builds a streaming handler for a ``Task`` + (one email per failure). As a ``ReportChannel`` it sends a finished + ``ProcessExecutionReport`` once via :meth:`send_report`. The same instance + can serve both roles. Attributes ---------- @@ -105,6 +158,9 @@ class EmailChannel(NotificationChannel): SMTP transport configuration for the alert. style : HTMLEmailStyle HTML presentation settings used to render the alert. + content : ReportContent + Content selection used by :meth:`send_report` (ignored by the per-task + handler). Parameters ---------- @@ -113,11 +169,20 @@ class EmailChannel(NotificationChannel): style : HTMLEmailStyle | None HTML presentation settings used to render the alert. Defaults to ``HTMLEmailStyle()`` (modern, neutral, English) when ``None``. + content : ReportContent | None + Content selection for report delivery. Defaults to ``ReportContent()`` + (everything) when ``None``. """ - def __init__(self, smtp_config: SMTPConfig, style: HTMLEmailStyle | None = None): + def __init__( + self, + smtp_config: SMTPConfig, + style: HTMLEmailStyle | None = None, + content: ReportContent | None = None, + ): self.smtp_config = smtp_config self.style = style or HTMLEmailStyle() + self.content = content or ReportContent() def build_handler(self, task_name: str) -> logging.Handler: """Build an HTML email handler bound to ``task_name``. @@ -147,29 +212,50 @@ def frame_filter(self) -> str | None: """ return self.style.traced_vars_frame_filter + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + """Send the report as an HTML email. Not implemented yet. + + Rendering (a report-shaped HTML body honoring ``style`` and ``content``) + and the one-shot SMTP send are deferred. + + Raises + ------ + NotImplementedError + Always, until report email rendering is implemented. + """ + raise NotImplementedError("EmailChannel.send_report is not implemented yet.") -class WebhookChannel(NotificationChannel): - """Notification channel that POSTs a JSON alert to a webhook URL on task failure. - The JSON payload is built generically from the task's failure context - (function, args/kwargs, exception, traceback, downstream impact, traced - variables), so it can be consumed directly or transformed by downstream - relays (e.g. Slack/Discord/Teams webhook adapters, custom alerting - servers). It is not coupled to any specific service. +class WebhookChannel(NotificationChannel, ReportChannel): + """Channel that POSTs JSON: a per-task failure alert and/or a report. + + As a ``NotificationChannel`` it builds a streaming handler for a ``Task`` + (one POST per failure). As a ``ReportChannel`` it POSTs a finished + ``ProcessExecutionReport`` once via :meth:`send_report`. The payload is + generic JSON, so it can be consumed directly or transformed by downstream + relays (Slack/Discord/Teams adapters, custom alerting servers); it is not + coupled to any specific service. Attributes ---------- webhook_config : WebhookConfig Webhook transport configuration for the alert. + content : ReportContent + Content selection used by :meth:`send_report` (ignored by the per-task + handler). Parameters ---------- webhook_config : WebhookConfig Webhook transport configuration for the alert. + content : ReportContent | None + Content selection for report delivery. Defaults to ``ReportContent()`` + (everything) when ``None``. """ - def __init__(self, webhook_config: WebhookConfig): + def __init__(self, webhook_config: WebhookConfig, content: ReportContent | None = None): self.webhook_config = webhook_config + self.content = content or ReportContent() def build_handler(self, task_name: str) -> logging.Handler: """Build a JSON webhook handler. @@ -187,3 +273,16 @@ def build_handler(self, task_name: str) -> logging.Handler: describing the failure for each error log record. """ return _build_task_webhook_handler(self.webhook_config) + + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + """POST the report as JSON. Not implemented yet. + + Payload shaping (honoring ``content`` and ``errors_only``) and the + one-shot signed POST are deferred. + + Raises + ------ + NotImplementedError + Always, until report webhook delivery is implemented. + """ + raise NotImplementedError("WebhookChannel.send_report is not implemented yet.") diff --git a/tests/test_report_notify_dispatch.py b/tests/test_report_notify_dispatch.py new file mode 100644 index 0000000..276eb17 --- /dev/null +++ b/tests/test_report_notify_dispatch.py @@ -0,0 +1,69 @@ +"""Architecture of report notification: notify/notify_errors dispatch to channels. + +Message rendering is deferred, so the built-in channels' ``send_report`` raise +``NotImplementedError``; these tests cover only the dispatch wiring and config. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from processes import ( + EmailChannel, + ProcessExecutionReport, + ReportChannel, + ReportContent, + SMTPConfig, + WebhookChannel, + WebhookConfig, +) + + +class _SpyChannel(ReportChannel): + def __init__(self) -> None: + self.calls: list[tuple[Any, bool]] = [] + + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + self.calls.append((report, errors_only)) + + +def _smtp() -> SMTPConfig: + return SMTPConfig(mailhost=("host", 25), fromaddr="a@b.com", toaddrs=["c@d.com"]) + + +def test_notify_dispatches_to_each_channel_in_order() -> None: + report = ProcessExecutionReport() + a, b = _SpyChannel(), _SpyChannel() + report.notify(a, b) + assert a.calls == [(report, False)] + assert b.calls == [(report, False)] + + +def test_notify_errors_sets_errors_only() -> None: + report = ProcessExecutionReport() + spy = _SpyChannel() + report.notify_errors(spy) + assert spy.calls == [(report, True)] + + +def test_notify_with_no_channels_is_noop() -> None: + ProcessExecutionReport().notify() + ProcessExecutionReport().notify_errors() + + +def test_builtin_channels_send_report_not_implemented_yet() -> None: + report = ProcessExecutionReport() + with pytest.raises(NotImplementedError): + EmailChannel(_smtp()).send_report(report, errors_only=False) + with pytest.raises(NotImplementedError): + WebhookChannel(WebhookConfig(url="http://x")).send_report(report, errors_only=True) + + +def test_report_content_defaults_and_per_channel_override() -> None: + assert ReportContent() == ReportContent(show_traceback=True, show_traced_vars=True) + + custom = ReportContent(show_traceback=False) + assert WebhookChannel(WebhookConfig(url="http://x"), content=custom).content is custom + assert EmailChannel(_smtp()).content == ReportContent() # default when omitted From 5692a4d41b31280535da67d4061daaba3a8753ca Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Tue, 16 Jun 2026 21:32:18 -0400 Subject: [PATCH 05/14] feat: implement WebhookChannel and EmailChannel send_report - WebhookChannel.send_report: signs and POSTs report JSON via WebhookConfig (nest_under, extra_payload, HMAC, custom headers all respected) - EmailChannel.send_report: renders multi-task HTML email and sends via SMTP - Extracted _post_json() helper from _WebhookHandler.emit to share signing logic - Added _build_report_html() renderer using new themes/styles/report.html template (palette + language respected; layout-variant not applicable to multi-task reports) - Added _build_task_section_html():
for errors, closed for others - notify/notify_errors: try/except per channel + show_warnings:bool=True parameter so a failing channel never aborts the remaining ones - Added report-specific language keys to all 6 locale JSON files (en/es/pt/fr/de/it) - Tests: 21 new tests covering dispatch, warnings, webhook payload, HTML renderer, content flags, and SMTP transport --- src/processes/_email_internals.py | 216 +++++++++++++++- src/processes/_webhook_internals.py | 121 +++++++-- src/processes/execution_report.py | 39 ++- src/processes/notification_channels.py | 45 ++-- src/processes/themes/languages/de.json | 14 +- src/processes/themes/languages/en.json | 14 +- src/processes/themes/languages/es.json | 14 +- src/processes/themes/languages/fr.json | 14 +- src/processes/themes/languages/it.json | 14 +- src/processes/themes/languages/pt.json | 14 +- src/processes/themes/styles/report.html | 178 +++++++++++++ tests/test_report_notify_dispatch.py | 51 +++- tests/test_report_send.py | 322 ++++++++++++++++++++++++ 13 files changed, 1000 insertions(+), 56 deletions(-) create mode 100644 src/processes/themes/styles/report.html create mode 100644 tests/test_report_send.py diff --git a/src/processes/_email_internals.py b/src/processes/_email_internals.py index 2f33ced..98a3838 100644 --- a/src/processes/_email_internals.py +++ b/src/processes/_email_internals.py @@ -8,11 +8,17 @@ import smtplib from email.mime.text import MIMEText from email.utils import formatdate -from typing import cast +from typing import TYPE_CHECKING, cast from ._error_data import _ErrorContextFormatter from .email_config import HTMLEmailStyle, SMTPConfig +if TYPE_CHECKING: + from .execution_report import ProcessExecutionReport, TaskReportEntry + from .notification_channels import ReportContent + +_STATUS_ERRORED = "errored" + _THEMES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "themes") _STYLES_DIR = os.path.join(_THEMES_DIR, "styles") _PALETTES_DIR = os.path.join(_THEMES_DIR, "palettes") @@ -220,3 +226,211 @@ def _build_task_email_handler( lang_strings = _load_language_strings(style.language) handler.subject = f"{lang_strings['lang_email_subject']}{task_name}" return handler + + +def _build_task_section_html( + entry: TaskReportEntry, + lang: dict[str, str], + content: ReportContent, +) -> str: + """Build a ``
`` HTML block for one task entry. + + Errored tasks are rendered ``open`` with full error detail; success/skipped + tasks are collapsed and show only the function name. ``
`` degrades + gracefully in clients that don't support it (content shows expanded). + """ + status_val = entry.status.value + badge_classes = { + "SUCCESS": "badge-success", + "ERRORED": "badge-errored", + "SKIPPED": "badge-skipped", + } + badge_class = badge_classes.get(status_val, "badge-skipped") + status_label = { + "SUCCESS": lang.get("lang_status_success", "Success"), + "ERRORED": lang.get("lang_status_errored", "Error"), + "SKIPPED": lang.get("lang_status_skipped", "Skipped"), + }.get(status_val, status_val) + open_attr = " open" if entry.status.value == _STATUS_ERRORED else "" + + fn_label = html.escape(lang.get("lang_function_label", "Function")) + parts = [ + f'
' + f'
{fn_label}
' + f'
{html.escape(entry.function)}
' + f"
" + ] + + if entry.status.value == _STATUS_ERRORED and entry.error is not None: + exc_label = html.escape(lang.get("lang_exception_label", "Exception")) + parts.append( + f'
' + f'
{exc_label}
' + f'
{html.escape(entry.error.exception)}
' + f"
" + ) + if entry.error.downstream_impact: + ds_label = html.escape(lang.get("lang_downstream_title", "Downstream")) + items_html = "".join( + f"
  • {html.escape(n)}
    • " for n in entry.error.downstream_impact + ) + parts.append( + f'
    ' + f'
    {ds_label}
    ' + f'
      {items_html}
    ' + f"
    " + ) + if content.show_traceback and entry.error.traceback_str: + tb_title = html.escape(lang.get("lang_traceback_title", "Traceback")) + parts.append(f'
    {tb_title}
    ') + parts.append(f'
    {html.escape(entry.error.traceback_str)}
    ') + + if content.show_traced_vars and entry.error.traced_vars: + tv_title = html.escape(lang.get("lang_traced_vars_title", "Traced Variables")) + blurb = html.escape( + lang.get("lang_traced_vars_blurb", "Local variables at {location}:").replace( + "{location}", entry.error.traced_vars_location + ) + ) + traced_html = "\n".join( + html.escape(f"{k} = {v}") for k, v in entry.error.traced_vars.items() + ) + parts.append(f'
    {tv_title}
    ') + parts.append(f'
    {blurb}
    ') + parts.append(f'
    {traced_html}
    ') + + detail_inner = "\n".join(parts) + task_detail = f'
    \n{detail_inner}\n
    ' + + return ( + f"\n" + f' ' + f'{html.escape(entry.name)}' + f'{html.escape(status_label)}' + f"\n" + f" {task_detail}\n" + f"
    " + ) + + +def _build_report_html( + report: ProcessExecutionReport, + style: HTMLEmailStyle, + content: ReportContent, + *, + errors_only: bool, +) -> str: + """Render a ``ProcessExecutionReport`` as a full HTML email body. + + Uses ``themes/styles/report.html`` + the palette chosen in ``style``. + Language strings from ``style.language`` are applied throughout. + ``style.style`` (layout variant) is not used for reports — a single + multi-task layout is defined in ``report.html`` regardless of the + classic/modern/compact setting. + + Parameters + ---------- + report : ProcessExecutionReport + The finished report to render. + style : HTMLEmailStyle + Palette and language are respected; layout variant is not. + content : ReportContent + Controls whether traceback and traced-variables sections appear. + errors_only : bool + When ``True`` only ERRORED entries appear in the output. + """ + lang = _load_language_strings(style.language) + palette_path = os.path.join(_PALETTES_DIR, f"{style.palette}.css") + with open(palette_path, encoding="utf-8") as fh: + palette_css = fh.read() + report_template_path = os.path.join(_STYLES_DIR, "report.html") + with open(report_template_path, encoding="utf-8") as fh: + template = fh.read() + + template = template.replace(_PALETTE_MARKER, palette_css) + + entries = report.errored if errors_only else report.entries + header = ( + lang.get("lang_report_header_errors_only", "Failed Tasks Report") + if errors_only + else lang.get("lang_report_header", "Process Execution Report") + ) + task_sections = "\n".join( + _build_task_section_html(entry, lang, content) for entry in entries.values() + ) + + substitutions = { + "lang_report_header": html.escape(header), + "lang_report_title_prefix": html.escape( + lang.get("lang_report_title_prefix", "Process Report") + ), + "lang_report_summary_title": html.escape( + lang.get("lang_report_summary_title", "Summary") + ), + "lang_report_success_label": html.escape( + lang.get("lang_report_success_label", "Successes") + ), + "lang_report_error_label": html.escape(lang.get("lang_report_error_label", "Errors")), + "lang_report_skipped_label": html.escape( + lang.get("lang_report_skipped_label", "Skipped") + ), + "summary_successes": str(len(report.successes)), + "summary_errors": str(len(report.errored)), + "summary_skipped": str(len(report.skipped)), + "task_sections": task_sections, + } + rendered = template + for key, value in substitutions.items(): + rendered = rendered.replace("{{" + key + "}}", value) + return rendered + + +def send_report_email( + report: ProcessExecutionReport, + smtp_config: SMTPConfig, + style: HTMLEmailStyle, + content: ReportContent, + *, + errors_only: bool, +) -> None: + """Send a finished ``ProcessExecutionReport`` as an HTML email. + + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + smtp_config : SMTPConfig + SMTP transport configuration. + style : HTMLEmailStyle + Palette and language for the HTML body. + content : ReportContent + Content selection (traceback, traced-vars). + errors_only : bool + When ``True`` only ERRORED entries are included and the subject + uses ``lang_report_email_subject_errors``. + """ + lang = _load_language_strings(style.language) + subject_key = ( + "lang_report_email_subject_errors" if errors_only else "lang_report_email_subject" + ) + subject = lang.get(subject_key, "Process Execution Report") + html_body = _build_report_html(report, style, content, errors_only=errors_only) + + if isinstance(smtp_config.mailhost, tuple): + host, port = smtp_config.mailhost[0], smtp_config.mailhost[1] + else: + host, port = smtp_config.mailhost, smtplib.SMTP_PORT + + smtp = smtplib.SMTP(host, port) + mime_msg = MIMEText(html_body, "html") + mime_msg["From"] = smtp_config.fromaddr + mime_msg["To"] = ",".join(smtp_config.toaddrs) + mime_msg["Subject"] = subject + mime_msg["Date"] = formatdate() + if smtp_config.credentials is not None: + username, password = smtp_config.credentials + if smtp_config.secure is not None: + smtp.starttls(*smtp_config.secure) + smtp.login(username, password) + smtp.sendmail(smtp_config.fromaddr, smtp_config.toaddrs, mime_msg.as_string()) + smtp.quit() diff --git a/src/processes/_webhook_internals.py b/src/processes/_webhook_internals.py index 749b927..f4b86fe 100644 --- a/src/processes/_webhook_internals.py +++ b/src/processes/_webhook_internals.py @@ -5,14 +5,42 @@ import json import logging import urllib.request -from typing import Any +from typing import TYPE_CHECKING, Any from ._error_data import ErrorData, _ErrorContextFormatter from .webhook_config import WebhookConfig +if TYPE_CHECKING: + from .execution_report import ProcessExecutionReport, TaskReportEntry + from .notification_channels import ReportContent + +_STATUS_SUCCESS = "success" + _SIGNATURE_HEADER = "X-Signature-SHA256" +def _post_json(config: WebhookConfig, payload: str) -> None: + """Sign and POST a JSON string to ``config.url``. + + Parameters + ---------- + config : WebhookConfig + Transport configuration (URL, headers, timeout, optional HMAC secret). + payload : str + JSON string to POST as the request body. + """ + body = payload.encode("utf-8") + headers = {"Content-Type": "application/json", **config.headers} + if config.secret is not None: + digest = hmac.new( + config.secret.encode("utf-8"), body, hashlib.sha256 + ).hexdigest() + headers[_SIGNATURE_HEADER] = digest + request = urllib.request.Request(config.url, data=body, headers=headers, method="POST") + with urllib.request.urlopen(request, timeout=config.timeout): + pass + + class _WebhookFormatter(_ErrorContextFormatter): """Pure renderer: builds a generic JSON payload from ``record.task_context``.""" @@ -85,23 +113,88 @@ def __init__(self, config: WebhookConfig) -> None: def emit(self, record: logging.LogRecord) -> None: try: - body = self.format(record).encode("utf-8") - headers = {"Content-Type": "application/json", **self._config.headers} - if self._config.secret is not None: - digest = hmac.new( - self._config.secret.encode("utf-8"), body, hashlib.sha256 - ).hexdigest() - headers[_SIGNATURE_HEADER] = digest - - request = urllib.request.Request( - self._config.url, data=body, headers=headers, method="POST" - ) - with urllib.request.urlopen(request, timeout=self._config.timeout): - pass + _post_json(self._config, self.format(record)) except Exception: self.handleError(record) +def _build_report_webhook_payload( + entries: dict[str, TaskReportEntry], + content: ReportContent, + config: WebhookConfig, +) -> dict[str, Any]: + """Build the JSON-serializable payload dict for a report POST. + + Parameters + ---------- + entries : dict[str, TaskReportEntry] + Filtered task entries (all or errored-only, caller decides). + content : ReportContent + Content selection flags (``show_traceback``, ``show_traced_vars``). + config : WebhookConfig + Transport config used for ``nest_under`` and ``extra_payload``. + + Returns + ------- + dict[str, Any] + Ready-to-serialize payload with ``nest_under`` and ``extra_payload`` + already applied. + """ + tasks_payload: dict[str, Any] = {} + for name, entry in entries.items(): + task_dict: dict[str, Any] = { + "status": entry.status.value, + "function": entry.function, + "elapsed_seconds": entry.elapsed_seconds, + "attempts": entry.attempts, + } + if entry.status.value == _STATUS_SUCCESS: + task_dict["result"] = repr(entry.result) + if entry.error is not None: + error_dict: dict[str, Any] = { + "exception": entry.error.exception, + "downstream_impact": list(entry.error.downstream_impact), + } + if content.show_traceback: + error_dict["traceback"] = entry.error.traceback_str + if content.show_traced_vars and entry.error.traced_vars: + error_dict["traced_vars"] = entry.error.traced_vars + error_dict["traced_vars_location"] = entry.error.traced_vars_location + task_dict["error"] = error_dict + tasks_payload[name] = task_dict + + generic: dict[str, Any] = {"entries": tasks_payload} + if config.nest_under: + generic = {config.nest_under: generic} + return {**generic, **config.extra_payload} + + +def send_report_webhook( + report: ProcessExecutionReport, + config: WebhookConfig, + content: ReportContent, + *, + errors_only: bool, +) -> None: + """POST a finished ``ProcessExecutionReport`` to ``config.url`` as JSON. + + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + config : WebhookConfig + Transport configuration (URL, headers, timeout, HMAC secret, + ``extra_payload``, ``nest_under``). + content : ReportContent + Content selection: ``show_traceback`` / ``show_traced_vars``. + errors_only : bool + When ``True`` only ``ERRORED`` entries are included in the payload. + """ + entries = report.errored if errors_only else report.entries + payload = _build_report_webhook_payload(entries, content, config) + _post_json(config, json.dumps(payload)) + + def _build_task_webhook_handler(config: WebhookConfig) -> _WebhookHandler: """Create a fully configured webhook handler. diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index 8c6aac9..e5ef15f 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import warnings from dataclasses import dataclass, field, fields, is_dataclass from enum import Enum from typing import TYPE_CHECKING, Any @@ -167,30 +168,58 @@ def to_json(self, *, indent: int | None = None, **dumps_kwargs: Any) -> str: dumps_kwargs.pop("default", None) return json.dumps(self, default=_json_default, indent=indent, **dumps_kwargs) - def notify(self, *channels: ReportChannel) -> None: + def notify( + self, *channels: ReportChannel, show_warnings: bool = True + ) -> None: """Deliver the full report through each channel, in order. Each channel renders and sends the report itself (email, webhook, ...). What detail is included is configured per channel (see ``ReportContent``). + If a channel raises, the exception is caught so the remaining channels + still receive the report; a ``UserWarning`` is emitted when + ``show_warnings`` is ``True``. Parameters ---------- *channels : ReportChannel Channels to deliver the report to. No-op if none are given. + show_warnings : bool + Emit a ``UserWarning`` when a channel fails. Defaults to ``True``. """ for channel in channels: - channel.send_report(self, errors_only=False) - - def notify_errors(self, *channels: ReportChannel) -> None: + try: + channel.send_report(self, errors_only=False) + except Exception as exc: + if show_warnings: + warnings.warn( + f"{type(channel).__name__} failed to send report: {exc}", + stacklevel=2, + ) + + def notify_errors( + self, *channels: ReportChannel, show_warnings: bool = True + ) -> None: """Deliver only the ``ERRORED`` entries through each channel, in order. Same as :meth:`notify`, but each channel restricts the payload to tasks whose status is ``ERRORED`` (see :attr:`errored`). + If a channel raises, the exception is caught so the remaining channels + still receive the report; a ``UserWarning`` is emitted when + ``show_warnings`` is ``True``. Parameters ---------- *channels : ReportChannel Channels to deliver the errored report to. No-op if none are given. + show_warnings : bool + Emit a ``UserWarning`` when a channel fails. Defaults to ``True``. """ for channel in channels: - channel.send_report(self, errors_only=True) + try: + channel.send_report(self, errors_only=True) + except Exception as exc: + if show_warnings: + warnings.warn( + f"{type(channel).__name__} failed to send report: {exc}", + stacklevel=2, + ) diff --git a/src/processes/notification_channels.py b/src/processes/notification_channels.py index 48255e4..b3cd0b1 100644 --- a/src/processes/notification_channels.py +++ b/src/processes/notification_channels.py @@ -5,9 +5,9 @@ from dataclasses import dataclass from typing import TYPE_CHECKING -from ._email_internals import _build_task_email_handler +from ._email_internals import _build_task_email_handler, send_report_email from ._logfile_formatting import _TaskLogfileFormatter -from ._webhook_internals import _build_task_webhook_handler +from ._webhook_internals import _build_task_webhook_handler, send_report_webhook from .email_config import HTMLEmailStyle, SMTPConfig from .webhook_config import WebhookConfig @@ -213,17 +213,22 @@ def frame_filter(self) -> str | None: return self.style.traced_vars_frame_filter def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: - """Send the report as an HTML email. Not implemented yet. + """Send the report as a styled HTML email via SMTP. - Rendering (a report-shaped HTML body honoring ``style`` and ``content``) - and the one-shot SMTP send are deferred. + Renders a multi-task HTML body using ``style`` (palette + language) + and ``content`` (traceback / traced-vars flags), then sends it as a + one-shot SMTP message. - Raises - ------ - NotImplementedError - Always, until report email rendering is implemented. + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + errors_only : bool + When ``True`` only ERRORED entries are included in the email. """ - raise NotImplementedError("EmailChannel.send_report is not implemented yet.") + send_report_email( + report, self.smtp_config, self.style, self.content, errors_only=errors_only + ) class WebhookChannel(NotificationChannel, ReportChannel): @@ -275,14 +280,18 @@ def build_handler(self, task_name: str) -> logging.Handler: return _build_task_webhook_handler(self.webhook_config) def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: - """POST the report as JSON. Not implemented yet. + """POST the report as a signed JSON payload. - Payload shaping (honoring ``content`` and ``errors_only``) and the - one-shot signed POST are deferred. + Builds the payload from the report entries (all or errored-only), + applies ``content`` flags to control traceback / traced-vars inclusion, + and performs a one-shot POST via ``webhook_config`` (honoring + ``nest_under``, ``extra_payload``, HMAC signing, and custom headers). - Raises - ------ - NotImplementedError - Always, until report webhook delivery is implemented. + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + errors_only : bool + When ``True`` only ERRORED entries are included in the payload. """ - raise NotImplementedError("WebhookChannel.send_report is not implemented yet.") + send_report_webhook(report, self.webhook_config, self.content, errors_only=errors_only) diff --git a/src/processes/themes/languages/de.json b/src/processes/themes/languages/de.json index dfb5cd0..62a24e8 100644 --- a/src/processes/themes/languages/de.json +++ b/src/processes/themes/languages/de.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Fehlerverlauf", "lang_traced_vars_title": "Verfolgte Variablen", "lang_traced_vars_blurb": "Die folgenden lokalen Variablen hatten diese Werte in {location}:", - "lang_email_subject": "Fehler in Aufgabe " + "lang_email_subject": "Fehler in Aufgabe ", + "lang_report_title_prefix": "Prozessbericht", + "lang_report_header": "Prozessausführungsbericht", + "lang_report_header_errors_only": "Bericht fehlgeschlagener Aufgaben", + "lang_report_email_subject": "Prozessausführungsbericht", + "lang_report_email_subject_errors": "Fehlgeschlagene Aufgaben im Prozess", + "lang_report_summary_title": "Zusammenfassung", + "lang_report_success_label": "Erfolge", + "lang_report_error_label": "Fehler", + "lang_report_skipped_label": "Übersprungen", + "lang_status_success": "Erfolg", + "lang_status_errored": "Fehler", + "lang_status_skipped": "Übersprungen" } \ No newline at end of file diff --git a/src/processes/themes/languages/en.json b/src/processes/themes/languages/en.json index dc4415a..33af00b 100644 --- a/src/processes/themes/languages/en.json +++ b/src/processes/themes/languages/en.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Traceback", "lang_traced_vars_title": "Traced Variables", "lang_traced_vars_blurb": "The following local variables had these values at {location}:", - "lang_email_subject": "Error in task " + "lang_email_subject": "Error in task ", + "lang_report_title_prefix": "Process Report", + "lang_report_header": "Process Execution Report", + "lang_report_header_errors_only": "Failed Tasks Report", + "lang_report_email_subject": "Process Execution Report", + "lang_report_email_subject_errors": "Failed Tasks in Process", + "lang_report_summary_title": "Summary", + "lang_report_success_label": "Successes", + "lang_report_error_label": "Errors", + "lang_report_skipped_label": "Skipped", + "lang_status_success": "Success", + "lang_status_errored": "Error", + "lang_status_skipped": "Skipped" } \ No newline at end of file diff --git a/src/processes/themes/languages/es.json b/src/processes/themes/languages/es.json index 5936fcd..0370bdd 100644 --- a/src/processes/themes/languages/es.json +++ b/src/processes/themes/languages/es.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Traza de error", "lang_traced_vars_title": "Variables rastreadas", "lang_traced_vars_blurb": "Las siguientes variables locales tenían estos valores en {location}:", - "lang_email_subject": "Error en la tarea " + "lang_email_subject": "Error en la tarea ", + "lang_report_title_prefix": "Informe del Proceso", + "lang_report_header": "Informe de Ejecución del Proceso", + "lang_report_header_errors_only": "Informe de Tareas Fallidas", + "lang_report_email_subject": "Informe de Ejecución del Proceso", + "lang_report_email_subject_errors": "Tareas Fallidas en el Proceso", + "lang_report_summary_title": "Resumen", + "lang_report_success_label": "Éxitos", + "lang_report_error_label": "Errores", + "lang_report_skipped_label": "Omitidas", + "lang_status_success": "Éxito", + "lang_status_errored": "Error", + "lang_status_skipped": "Omitida" } \ No newline at end of file diff --git a/src/processes/themes/languages/fr.json b/src/processes/themes/languages/fr.json index 912b794..1ed90e6 100644 --- a/src/processes/themes/languages/fr.json +++ b/src/processes/themes/languages/fr.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Trace d'erreur", "lang_traced_vars_title": "Variables suivies", "lang_traced_vars_blurb": "Les variables locales suivantes avaient ces valeurs à {location} :", - "lang_email_subject": "Erreur dans la tâche " + "lang_email_subject": "Erreur dans la tâche ", + "lang_report_title_prefix": "Rapport de Processus", + "lang_report_header": "Rapport d'Exécution du Processus", + "lang_report_header_errors_only": "Rapport des Tâches Échouées", + "lang_report_email_subject": "Rapport d'Exécution du Processus", + "lang_report_email_subject_errors": "Tâches Échouées dans le Processus", + "lang_report_summary_title": "Résumé", + "lang_report_success_label": "Succès", + "lang_report_error_label": "Erreurs", + "lang_report_skipped_label": "Ignorées", + "lang_status_success": "Succès", + "lang_status_errored": "Erreur", + "lang_status_skipped": "Ignorée" } \ No newline at end of file diff --git a/src/processes/themes/languages/it.json b/src/processes/themes/languages/it.json index 2da57ff..51cc08e 100644 --- a/src/processes/themes/languages/it.json +++ b/src/processes/themes/languages/it.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Traccia dell'errore", "lang_traced_vars_title": "Variabili tracciate", "lang_traced_vars_blurb": "Le seguenti variabili locali avevano questi valori in {location}:", - "lang_email_subject": "Errore nell'attività " + "lang_email_subject": "Errore nell'attività ", + "lang_report_title_prefix": "Rapporto di Processo", + "lang_report_header": "Rapporto di Esecuzione del Processo", + "lang_report_header_errors_only": "Rapporto Attività Fallite", + "lang_report_email_subject": "Rapporto di Esecuzione del Processo", + "lang_report_email_subject_errors": "Attività Fallite nel Processo", + "lang_report_summary_title": "Riepilogo", + "lang_report_success_label": "Successi", + "lang_report_error_label": "Errori", + "lang_report_skipped_label": "Saltate", + "lang_status_success": "Successo", + "lang_status_errored": "Errore", + "lang_status_skipped": "Saltata" } \ No newline at end of file diff --git a/src/processes/themes/languages/pt.json b/src/processes/themes/languages/pt.json index a486722..7dbe12f 100644 --- a/src/processes/themes/languages/pt.json +++ b/src/processes/themes/languages/pt.json @@ -11,5 +11,17 @@ "lang_traceback_title": "Rastreamento de erro", "lang_traced_vars_title": "Variáveis rastreadas", "lang_traced_vars_blurb": "As seguintes variáveis locais tinham estes valores em {location}:", - "lang_email_subject": "Erro na tarefa " + "lang_email_subject": "Erro na tarefa ", + "lang_report_title_prefix": "Relatório do Processo", + "lang_report_header": "Relatório de Execução do Processo", + "lang_report_header_errors_only": "Relatório de Tarefas com Falha", + "lang_report_email_subject": "Relatório de Execução do Processo", + "lang_report_email_subject_errors": "Tarefas com Falha no Processo", + "lang_report_summary_title": "Resumo", + "lang_report_success_label": "Sucessos", + "lang_report_error_label": "Erros", + "lang_report_skipped_label": "Ignoradas", + "lang_status_success": "Sucesso", + "lang_status_errored": "Erro", + "lang_status_skipped": "Ignorada" } \ No newline at end of file diff --git a/src/processes/themes/styles/report.html b/src/processes/themes/styles/report.html new file mode 100644 index 0000000..d03b038 --- /dev/null +++ b/src/processes/themes/styles/report.html @@ -0,0 +1,178 @@ + + + + + {{lang_report_title_prefix}} + + + +
    +
    {{lang_report_header}}
    +
    +

    {{lang_report_summary_title}}

    +
    +
    +
    {{summary_successes}}
    +
    {{lang_report_success_label}}
    +
    +
    +
    {{summary_errors}}
    +
    {{lang_report_error_label}}
    +
    +
    +
    {{summary_skipped}}
    +
    {{lang_report_skipped_label}}
    +
    +
    + {{task_sections}} +
    +
    + + diff --git a/tests/test_report_notify_dispatch.py b/tests/test_report_notify_dispatch.py index 276eb17..894b7c6 100644 --- a/tests/test_report_notify_dispatch.py +++ b/tests/test_report_notify_dispatch.py @@ -1,15 +1,10 @@ -"""Architecture of report notification: notify/notify_errors dispatch to channels. - -Message rendering is deferred, so the built-in channels' ``send_report`` raise -``NotImplementedError``; these tests cover only the dispatch wiring and config. -""" +"""Report notification dispatch: notify/notify_errors wiring and show_warnings behaviour.""" from __future__ import annotations +import warnings from typing import Any -import pytest - from processes import ( EmailChannel, ProcessExecutionReport, @@ -29,6 +24,11 @@ def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> N self.calls.append((report, errors_only)) +class _BrokenChannel(ReportChannel): + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + raise RuntimeError("boom") + + def _smtp() -> SMTPConfig: return SMTPConfig(mailhost=("host", 25), fromaddr="a@b.com", toaddrs=["c@d.com"]) @@ -53,12 +53,39 @@ def test_notify_with_no_channels_is_noop() -> None: ProcessExecutionReport().notify_errors() -def test_builtin_channels_send_report_not_implemented_yet() -> None: +def test_notify_continues_after_channel_failure() -> None: + report = ProcessExecutionReport() + broken = _BrokenChannel() + spy = _SpyChannel() + report.notify(broken, spy, show_warnings=False) + assert spy.calls == [(report, False)] + + +def test_notify_warns_on_channel_failure() -> None: report = ProcessExecutionReport() - with pytest.raises(NotImplementedError): - EmailChannel(_smtp()).send_report(report, errors_only=False) - with pytest.raises(NotImplementedError): - WebhookChannel(WebhookConfig(url="http://x")).send_report(report, errors_only=True) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + report.notify(_BrokenChannel()) + assert len(caught) == 1 + assert "boom" in str(caught[0].message) + + +def test_notify_silent_when_show_warnings_false() -> None: + report = ProcessExecutionReport() + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + report.notify(_BrokenChannel(), show_warnings=False) + assert caught == [] + + +def test_notify_errors_continues_and_warns() -> None: + report = ProcessExecutionReport() + spy = _SpyChannel() + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + report.notify_errors(_BrokenChannel(), spy) + assert len(caught) == 1 + assert spy.calls == [(report, True)] def test_report_content_defaults_and_per_channel_override() -> None: diff --git a/tests/test_report_send.py b/tests/test_report_send.py new file mode 100644 index 0000000..2ded832 --- /dev/null +++ b/tests/test_report_send.py @@ -0,0 +1,322 @@ +"""Integration tests for WebhookChannel.send_report and EmailChannel.send_report.""" + +from __future__ import annotations + +import email as _email_module +import json +from unittest.mock import MagicMock, patch + +from processes import ( + EmailChannel, + ProcessExecutionReport, + ReportContent, + SMTPConfig, + TaskReportEntry, + TaskStatus, + WebhookChannel, + WebhookConfig, +) +from processes._email_internals import _build_report_html +from processes._error_data import ErrorData +from processes.email_config import HTMLEmailStyle + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _entry( + name: str, + status: TaskStatus, + *, + error: ErrorData | None = None, + elapsed: float = 0.1, + attempts: int = 1, +) -> TaskReportEntry: + return TaskReportEntry( + name=name, + function=f"fn_{name}", + args=(), + kwargs={}, + status=status, + elapsed_seconds=elapsed, + attempts=attempts, + result="ok" if status == TaskStatus.SUCCESS else None, + error=error, + ) + + +def _error( + exception: str = "ValueError: bad", + traceback_str: str = "Traceback ...\n File x.py line 1\nValueError: bad", + traced_vars: dict[str, str] | None = None, + downstream: list[str] | None = None, +) -> ErrorData: + return ErrorData( + task_name="t", + function="fn_t", + exception=exception, + traceback_str=traceback_str, + traced_vars=traced_vars or {"x": "1"}, + traced_vars_location="x.py:1", + downstream_impact=downstream or [], + ) + + +def _report(*entries: TaskReportEntry) -> ProcessExecutionReport: + return ProcessExecutionReport({e.name: e for e in entries}) + + +def _smtp() -> SMTPConfig: + return SMTPConfig(mailhost=("localhost", 25), fromaddr="a@b.com", toaddrs=["c@d.com"]) + + +def _decode_mime_body(mime_string: str) -> str: + """Parse a MIME message string and return the decoded text body.""" + msg = _email_module.message_from_string(mime_string) + payload = msg.get_payload(decode=True) + if isinstance(payload, bytes): + return payload.decode("utf-8", errors="replace") + return str(payload or "") + + +# --------------------------------------------------------------------------- +# Webhook tests +# --------------------------------------------------------------------------- + +class TestWebhookSendReport: + + @patch("urllib.request.urlopen") + def test_posts_json_with_all_entries(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report( + _entry("a", TaskStatus.SUCCESS), + _entry("b", TaskStatus.ERRORED, error=_error()), + ) + WebhookChannel(WebhookConfig(url="http://hook")).send_report(report, errors_only=False) + + assert mock_urlopen.called + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + assert set(payload["entries"].keys()) == {"a", "b"} + assert payload["entries"]["a"]["status"] == TaskStatus.SUCCESS.value + assert payload["entries"]["b"]["status"] == TaskStatus.ERRORED.value + + @patch("urllib.request.urlopen") + def test_errors_only_filters_to_errored(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report( + _entry("a", TaskStatus.SUCCESS), + _entry("b", TaskStatus.ERRORED, error=_error()), + _entry("c", TaskStatus.SKIPPED), + ) + WebhookChannel(WebhookConfig(url="http://hook")).send_report(report, errors_only=True) + + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + assert list(payload["entries"].keys()) == ["b"] + + @patch("urllib.request.urlopen") + def test_content_show_traceback_false_omits_traceback(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report(_entry("b", TaskStatus.ERRORED, error=_error())) + WebhookChannel( + WebhookConfig(url="http://hook"), + content=ReportContent(show_traceback=False, show_traced_vars=True), + ).send_report(report, errors_only=False) + + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + error_dict = payload["entries"]["b"]["error"] + assert "traceback" not in error_dict + assert "traced_vars" in error_dict + + @patch("urllib.request.urlopen") + def test_content_show_traced_vars_false_omits_vars(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report(_entry("b", TaskStatus.ERRORED, error=_error())) + WebhookChannel( + WebhookConfig(url="http://hook"), + content=ReportContent(show_traceback=True, show_traced_vars=False), + ).send_report(report, errors_only=False) + + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + error_dict = payload["entries"]["b"]["error"] + assert "traceback" in error_dict + assert "traced_vars" not in error_dict + + @patch("urllib.request.urlopen") + def test_nest_under_wraps_entries(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report(_entry("a", TaskStatus.SUCCESS)) + WebhookChannel(WebhookConfig(url="http://hook", nest_under="data")).send_report( + report, errors_only=False + ) + + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + assert "data" in payload + assert "entries" in payload["data"] + + @patch("urllib.request.urlopen") + def test_extra_payload_merged_top_level(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report(_entry("a", TaskStatus.SUCCESS)) + WebhookChannel( + WebhookConfig(url="http://hook", extra_payload={"chat_id": "123"}) + ).send_report(report, errors_only=False) + + request = mock_urlopen.call_args[0][0] + payload = json.loads(request.data.decode()) + assert payload["chat_id"] == "123" + assert "entries" in payload + + @patch("urllib.request.urlopen") + def test_hmac_signature_header_set(self, mock_urlopen: MagicMock) -> None: + mock_urlopen.return_value.__enter__ = lambda s: s + mock_urlopen.return_value.__exit__ = MagicMock(return_value=False) + + report = _report(_entry("a", TaskStatus.SUCCESS)) + WebhookChannel(WebhookConfig(url="http://hook", secret="s3cr3t")).send_report( + report, errors_only=False + ) + + request = mock_urlopen.call_args[0][0] + # urllib.request.Request lowercases header names + header_names_lower = {k.lower() for k in request.headers} + assert "x-signature-sha256" in header_names_lower + + +# --------------------------------------------------------------------------- +# Report HTML renderer tests (pure, no I/O mocks needed) +# --------------------------------------------------------------------------- + +class TestBuildReportHtml: + def _style(self) -> HTMLEmailStyle: + return HTMLEmailStyle() + + def test_contains_task_name(self) -> None: + report = _report(_entry("my_unique_task", TaskStatus.ERRORED, error=_error())) + html = _build_report_html(report, self._style(), ReportContent(), errors_only=False) + assert "my_unique_task" in html + + def test_traceback_present_when_enabled(self) -> None: + report = _report( + _entry("b", TaskStatus.ERRORED, error=_error(traceback_str="UNIQUE_TRACE_TEXT")) + ) + html = _build_report_html( + report, self._style(), ReportContent(show_traceback=True), errors_only=False + ) + assert "UNIQUE_TRACE_TEXT" in html + + def test_traceback_absent_when_disabled(self) -> None: + report = _report( + _entry("b", TaskStatus.ERRORED, error=_error(traceback_str="UNIQUE_TRACE_TEXT")) + ) + html = _build_report_html( + report, self._style(), ReportContent(show_traceback=False), errors_only=False + ) + assert "UNIQUE_TRACE_TEXT" not in html + + def test_traced_vars_absent_when_disabled(self) -> None: + report = _report( + _entry("b", TaskStatus.ERRORED, error=_error(traced_vars={"UNIQUE_VAR": "42"})) + ) + html = _build_report_html( + report, self._style(), ReportContent(show_traced_vars=False), errors_only=False + ) + assert "UNIQUE_VAR" not in html + + def test_errors_only_excludes_success(self) -> None: + report = _report( + _entry("ok", TaskStatus.SUCCESS), + _entry("bad", TaskStatus.ERRORED, error=_error()), + ) + html = _build_report_html(report, self._style(), ReportContent(), errors_only=True) + assert "bad" in html + assert "ok" not in html + + def test_summary_counts_appear(self) -> None: + report = _report( + _entry("a", TaskStatus.SUCCESS), + _entry("b", TaskStatus.ERRORED, error=_error()), + _entry("c", TaskStatus.SKIPPED), + ) + html = _build_report_html(report, self._style(), ReportContent(), errors_only=False) + assert ">1<" in html # each count cell shows "1" + + def test_palette_css_injected(self) -> None: + report = _report(_entry("a", TaskStatus.SUCCESS)) + html = _build_report_html( + report, HTMLEmailStyle(palette="catppuccin"), ReportContent(), errors_only=False + ) + # catppuccin palette has its own CSS variable names + assert "--bg" in html or "catppuccin" in html.lower() or "var(--" in html + + +# --------------------------------------------------------------------------- +# Email send tests (verify SMTP transport) +# --------------------------------------------------------------------------- + +class TestEmailSendReport: + + @patch("smtplib.SMTP") + def test_sendmail_called(self, mock_smtp_cls: MagicMock) -> None: + mock_smtp = MagicMock() + mock_smtp_cls.return_value = mock_smtp + + report = _report( + _entry("a", TaskStatus.SUCCESS), + _entry("b", TaskStatus.ERRORED, error=_error()), + ) + EmailChannel(_smtp()).send_report(report, errors_only=False) + + mock_smtp.sendmail.assert_called_once() + mock_smtp.quit.assert_called_once() + + @patch("smtplib.SMTP") + def test_mime_type_is_html(self, mock_smtp_cls: MagicMock) -> None: + mock_smtp = MagicMock() + mock_smtp_cls.return_value = mock_smtp + + EmailChannel(_smtp()).send_report( + _report(_entry("a", TaskStatus.SUCCESS)), errors_only=False + ) + + _, _, msg_str = mock_smtp.sendmail.call_args[0] + assert 'Content-Type: text/html' in msg_str + + @patch("smtplib.SMTP") + def test_html_body_contains_task_name(self, mock_smtp_cls: MagicMock) -> None: + mock_smtp = MagicMock() + mock_smtp_cls.return_value = mock_smtp + + report = _report(_entry("my_task", TaskStatus.ERRORED, error=_error())) + EmailChannel(_smtp()).send_report(report, errors_only=False) + + _, _, msg_str = mock_smtp.sendmail.call_args[0] + body = _decode_mime_body(msg_str) + assert "my_task" in body + + @patch("smtplib.SMTP") + def test_errors_only_uses_errors_subject(self, mock_smtp_cls: MagicMock) -> None: + mock_smtp = MagicMock() + mock_smtp_cls.return_value = mock_smtp + + report = _report(_entry("b", TaskStatus.ERRORED, error=_error())) + EmailChannel(_smtp()).send_report(report, errors_only=True) + + _, _, msg_str = mock_smtp.sendmail.call_args[0] + assert "Failed" in msg_str or "failed" in msg_str.lower() From f315f3653ab0a70ac2911cbcbe87d833c633b667 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Tue, 16 Jun 2026 22:03:38 -0400 Subject: [PATCH 06/14] docs: add architecture plan for domain/comms reorganization --- arquitectura.md | 317 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 317 insertions(+) create mode 100644 arquitectura.md diff --git a/arquitectura.md b/arquitectura.md new file mode 100644 index 0000000..1e9a988 --- /dev/null +++ b/arquitectura.md @@ -0,0 +1,317 @@ +# Plan de Arquitectura — Reorganización de dominio y comunicación + +> Documento de **planificación**. No introduce cambios de código por sí mismo. +> Define los problemas actuales, la arquitectura objetivo y un plan por fases +> aprobables de forma independiente. + +--- + +## 1. Problemas actuales + +### Problema 1 — Inversión de dependencia dominio → infraestructura (y el workaround de import circular) + +`task.py` es el núcleo de dominio, pero importa la infraestructura de comunicación: + +```python +# task.py:17 +from .notification_channels import NotificationChannel, _FileChannel +``` + +`Task.__init__` construye los handlers de logging en el momento de la +construcción (`task.py:351-365`). Esto crea la cadena de importación: + +``` +task.py → notification_channels.py → _email_internals.py → task.py ✗ + (necesita TaskStatus) +``` + +El ciclo se "resuelve" hoy con un **workaround**: los internals de comunicación +no importan `TaskStatus`, sino que comparan contra el string del valor del enum: + +```python +# _webhook_internals.py +_STATUS_SUCCESS = "success" +... +if entry.status.value == _STATUS_SUCCESS: +``` + +```python +# _email_internals.py +_STATUS_ERRORED = "errored" +``` + +Esto es frágil (se rompe en silencio si cambia el `.value` del enum) y es el +síntoma visible de que la dirección de la dependencia está invertida: el dominio +no debería arrastrar la infraestructura. + +### Problema 2 — Canales con doble contrato y transporte duplicado + +`EmailChannel` y `WebhookChannel` implementan **dos** interfaces con modelos de +entrega distintos: + +| | Task (`NotificationChannel`) | Report (`ReportChannel`) | +|---|---|---| +| Disparo | Evento de logging (`LogRecord`) | Llamada explícita | +| Momento | Durante la ejecución | Al finalizar | +| Entidad | `logging.LogRecord` (streaming) | `ProcessExecutionReport` (one-shot) | +| Quién envía | El `Handler` (autónomo, en `emit`) | El canal (directo, en `send_report`) | + +Las dos interfaces son legítimamente diferentes. El problema **no** es que existan +ambas, sino que el **transporte está duplicado**: + +- **Email**: `_HTMLEmailHandler.emit` (`_email_internals.py:170-192`) abre SMTP, + arma el `MIMEText`, autentica, hace `sendmail` y `quit`. La función nueva + `send_report_email` repite exactamente esa misma secuencia. +- **Webhook**: ya fue parcialmente deduplicado — `_WebhookHandler.emit` y + `send_report_webhook` comparten `_post_json`. Falta cerrar la simetría del + lado email. + +### Problema 3 — Duplicación entre `Process` y `Task` (cierre de handlers y propiedad partida) + +El mismo bucle de cierre de handlers aparece **idéntico** dos veces en `process.py`: + +```python +# process.py:267-269 (remove_task) y process.py:376-379 (close_loggers) +for handler in list(task.logger.handlers): + handler.close() + task.logger.removeHandler(handler) +``` + +El problema de fondo es de **propiedad partida**: `Task` *crea* su logger y sus +handlers (`task.py:348-365`), pero `Process` los *destruye* metiendo la mano en +`task.logger.handlers`. La gestión del ciclo de vida del logger está repartida +entre dos clases. + +### Problema 4 — Namespace plano con roles muy distintos + +`src/processes/` mezcla en un solo nivel 2 módulos de dominio con ~10 de +comunicación: + +``` +process.py task.py execution_report.py exceptions.py ← dominio / orquestación +notification_channels.py _email_internals.py _webhook_internals.py +email_config.py webhook_config.py _error_data.py _tb_utils.py +_logfile_formatting.py exception_html_formatter.py html_logging.py ← comunicación +``` + +A medida que se añadan canales (Slack, SMS, etc.) el directorio raíz seguirá +creciendo con archivos cuyo rol no es evidente desde su ubicación. + +--- + +## 2. Invariantes (lo que NO debe romperse) + +1. **Definir un `Task` sigue siendo trivial para el usuario.** La firma pública + se mantiene: + ```python + Task("t", func, channels=[EmailChannel(smtp_cfg)]) + ``` + El usuario nunca construye handlers ni transportes a mano. +2. **API pública estable.** Todo lo exportado hoy en `processes/__init__.py` + (`Task`, `Process`, `TaskStatus`, `ErrorData`, `EmailChannel`, + `ProcessExecutionReport`, etc.) se sigue importando con `from processes import ...`. +3. **Rutas de módulo documentadas estables.** `processes.html_logging` + (`HTMLSMTPHandler`) está expuesto en `docs/reference`. Si el archivo se + reubica, se deja un *shim* de re-export en la ruta antigua para no romper + `from processes.html_logging import ...`. +4. **Sin regresiones.** `pytest`, `mypy` y `ruff` quedan verdes tras cada fase. + +--- + +## 3. Arquitectura objetivo + +### 3.1 Capas + +El dominio define **puertos** (clases abstractas); la comunicación provee +**adaptadores** (implementaciones concretas). Dentro de la comunicación se +separan tres responsabilidades hoy entremezcladas: + +``` + ┌──────────────────────────────────────────┐ + DOMINIO │ Task · Process · ProcessExecutionReport │ + (puertos) │ TaskStatus · TaskResult · ErrorData │ + │ NotificationChannel · ReportChannel (ABC)│ + └───────────────────┬──────────────────────┘ + │ depende solo de abstracciones + ┌───────────────────▼──────────────────────┐ + COMUNICACIÓN │ Channels (adaptadores user-facing) │ + (adaptadores) │ EmailChannel · WebhookChannel │ + │ Render (entidad → payload) │ + │ HTML email · JSON webhook │ + │ Transport (payload → destino) │ + │ _SMTPTransport · _WebhookTransport │ + └──────────────────────────────────────────┘ +``` + +- **Render** = "qué entregar": convierte un `LogRecord` (Task) o un + `ProcessExecutionReport` (Report) en un cuerpo (HTML / JSON). +- **Transport** = "cómo entregarlo": una sola implementación de SMTP-send y una + sola de HTTP-POST, usadas tanto por el handler de streaming como por el envío + one-shot del reporte. **Aquí se elimina la duplicación del Problema 2.** +- **Channel** = objeto de configuración que el usuario pasa; cablea render + + transport para sus dos roles. Al quedar render/transport como capas + separadas, el canal es delgado y puede implementar ambos puertos sin duplicar + nada (la doble capacidad pasa a ser una conveniencia honesta, no un smell). + +### 3.2 Paquete `comms/` + +Se agrupa toda la comunicación en un subpaquete plano (sin sub-subdirectorios: +`transports/` + `rendering/` con 2 archivos cada uno sería sobre-ingeniería): + +``` +src/processes/ + __init__.py # API pública (sin cambios de exports) + process.py # Process, ProcessRunner + task.py # Task + task_types.py # TaskStatus, TaskResult, TaskDependency ← LEAF (sin imports de comms) + error_data.py # ErrorData (dataclass puro) ← LEAF + _tb_utils.py # utilidades de traceback ← LEAF (dominio) + execution_report.py # ProcessExecutionReport, TaskReportEntry + exceptions.py + html_logging.py # shim de compatibilidad → comms + comms/ + __init__.py # re-exporta canales y ABCs públicos + base.py # PUERTOS: NotificationChannel, ReportChannel, ReportContent + channels.py # ADAPTADORES: EmailChannel, WebhookChannel, _FileChannel + config.py # SMTPConfig, HTMLEmailStyle, WebhookConfig + _smtp.py # _SMTPTransport (envío unificado) + handler de streaming + _webhook.py # _WebhookTransport (POST unificado) + handler de streaming + _email_render.py # _HTMLEmailFormatter (task) + render HTML del reporte + _webhook_render.py # _WebhookFormatter (task) + payload JSON del reporte + _error_context.py # _ErrorContextFormatter (lee LogRecord) + _logfile_formatting.py + exception_html_formatter.py + themes/ # styles / palettes / languages +``` + +**Por qué `ErrorData` y `_tb_utils` salen a leaf y `_ErrorContextFormatter` no:** +`ErrorData` es un dataclass de valor (contexto de fallo) que `TaskResult` referencia +— es dominio. `_tb_utils` construye ese contexto y lo usa `Task` +(`task.py:473-474`) — también dominio. En cambio `_ErrorContextFormatter` es un +`logging.Formatter` que lee un `LogRecord`: eso es comunicación y se queda en +`comms/`. Hoy `_error_data.py` los mezcla; se separan. + +### 3.3 Cómo se rompe el ciclo (sin workaround) + +Tras mover los tipos de valor a leaves sin dependencia de comms: + +``` +task.py → task_types, error_data, _tb_utils, comms.base (ABC), comms (_FileChannel) +comms/* → task_types, error_data, _tb_utils, comms.base, config +execution_report.py → task_types, error_data (+ TYPE_CHECKING: comms.base.ReportChannel) +process.py → task, task_types, error_data, execution_report, exceptions +``` + +`comms/` nunca importa `task.py` ni `process.py`. Los renderers importan +`TaskStatus` directamente desde `task_types` (leaf). **El ciclo desaparece y se +borran `_STATUS_SUCCESS` / `_STATUS_ERRORED`.** + +> **Nota de honestidad sobre el alcance:** esto rompe el *ciclo* y elimina el +> workaround, pero `task.py` sigue dependiendo de `comms.base` para el ABC +> `NotificationChannel` y de `_FileChannel`. **No** es una inversión total +> dominio→infra. Se adopta deliberadamente el enfoque **ports & adapters +> pragmático**: las clases abstractas `NotificationChannel` / `ReportChannel` se +> consideran *puertos propiedad del dominio*, y `comms.base` se mantiene como un +> verdadero leaf que no importa ningún internal de `comms`. Para una librería de +> este tamaño esto es suficiente; reubicar los ABC al lado del dominio (inversión +> total) sería más churn sin beneficio proporcional. + +### 3.4 Propiedad del logger unificada (Problema 3) + +`Task` gana la responsabilidad de su propio teardown: + +```python +# task.py +def close_handlers(self) -> None: + """Cierra y desadjunta los handlers del logger de la tarea.""" + for handler in list(self.logger.handlers): + handler.close() + self.logger.removeHandler(handler) +``` + +`Process.close_loggers` y `Process.remove_task` dejan de duplicar el bucle y +llaman `task.close_handlers()`. El que *crea* los handlers es el que los +*destruye*. + +--- + +## 4. Decisiones de diseño + +| # | Decisión | Razón | +|---|---|---| +| A | Extraer tipos de valor (`TaskStatus`, `TaskResult`, `TaskDependency`, `ErrorData`) a módulos leaf sin imports de comms | Rompe el ciclo en la raíz; elimina el workaround de strings; cero cambio de API | +| B | Una sola implementación de transporte por medio (`_SMTPTransport`, `_WebhookTransport`) usada por streaming y one-shot | Elimina la duplicación de SMTP entre handler y reporte | +| C | Mantener `NotificationChannel` y `ReportChannel` como **dos ABCs separados** | Son modelos de entrega genuinamente distintos (streaming vs one-shot); forzar uno solo mezclaría conceptos | +| D | Mantener la doble capacidad de `EmailChannel`/`WebhookChannel` | Conveniencia de "un objeto, dos roles". Deja de ser smell una vez que render/transport son capas compartidas. `_FileChannel` (solo `NotificationChannel`) demuestra que los ABCs no se imponen a la fuerza | +| E | `Task.close_handlers()` | Unifica la propiedad del ciclo de vida del logger; elimina la duplicación Process/Task | +| F | Paquete `comms/` plano + shim en `processes.html_logging` | Agrupa por rol sin romper rutas públicas ni sobre-anidar | + +--- + +## 5. Plan de implementación por fases + +Cada fase deja `pytest` / `mypy` / `ruff` verdes y es aprobable por separado. + +### Fase 0 — Romper el ciclo y la duplicación Process/Task (pequeña, sin cambio de API) +- Extraer `TaskStatus`, `TaskResult`, `TaskDependency` a `task_types.py`. +- Extraer `ErrorData` a `error_data.py`; dejar `_ErrorContextFormatter` donde está + (luego se moverá a `comms/`). +- Borrar `_STATUS_SUCCESS` / `_STATUS_ERRORED`; los renderers importan `TaskStatus`. +- Añadir `Task.close_handlers()`; `Process.close_loggers` y `remove_task` lo usan. +- **Impacto:** nulo en API pública. Es la fase que ataca directamente los puntos + que el usuario señaló (workaround + duplicación Process/Task). + +### Fase 1 — Unificar transporte +- Introducir `_SMTPTransport.send(subject, html_body)` con la conexión + MIME + + auth + `sendmail` + `quit` en un solo lugar. +- `_HTMLEmailHandler.emit` y `send_report_email` delegan en él. (El handler puede + simplificarse a un `logging.Handler` plano, ya que hoy sobreescribe casi todo + `SMTPHandler`.) +- Cerrar la simetría del webhook alrededor de `_WebhookTransport` (formaliza el + ya existente `_post_json`). +- **Impacto:** nulo en API pública; elimina la duplicación del Problema 2. + +### Fase 2 — Introducir el paquete `comms/` +- Mover los módulos de comunicación a `comms/` según §3.2. +- `comms/__init__.py` re-exporta los símbolos públicos; `processes/__init__.py` + pasa a importar desde `comms`. +- Dejar shim en `processes/html_logging.py`. +- **Impacto:** movimientos de archivo + actualización de imports. API pública y + rutas documentadas intactas (verificado por los tests existentes + invariante 3). + +### Fase 3 — (Opcional) Formalizar la capa de render +- Separar limpiamente render de transport donde aún convivan en un mismo archivo, + si la Fase 1/2 no lo dejó ya nítido. + +--- + +## 6. Extensibilidad — añadir un canal nuevo + +Con la estructura objetivo, agregar p. ej. un canal de Slack: + +1. ¿POSTea JSON? Reutiliza `_WebhookTransport` tal cual. +2. Render: añade un formateador/render en `comms/_webhook_render.py` (o uno nuevo) + si el payload difiere. +3. `class SlackChannel(NotificationChannel, ReportChannel)` en `comms/channels.py`, + cableando render + transport. Implementa `build_handler` y/o `send_report` + según los roles que soporte. +4. Exportar en `comms/__init__.py` y `processes/__init__.py`. + +No se toca el dominio (`task.py`, `process.py`, `execution_report.py`): solo se +añade un adaptador. Esa es la escalabilidad que se busca. + +--- + +## 7. Resumen + +| Problema | Solución | Fase | +|---|---|---| +| Workaround de import circular | Tipos de valor en leaves; renderers importan `TaskStatus` | 0 | +| Duplicación Process/Task | `Task.close_handlers()` | 0 | +| Transporte duplicado (email/report) | `_SMTPTransport` / `_WebhookTransport` compartidos | 1 | +| Namespace plano con roles mezclados | Paquete `comms/` + shims | 2 | +| Doble contrato de canal | Se conserva (2 ABCs) — deja de ser smell al compartir capas | 1–2 | + +Recomendación: aprobar e implementar **Fase 0 y 1** primero (correctitud y +deduplicación, riesgo mínimo, sin cambio de API). La **Fase 2** (paquete) es la +más "churny" y conviene revisarla como PR aparte. From de39c3a4e6c6ba3a499a665eada1ddf803959d74 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Thu, 18 Jun 2026 19:18:58 -0400 Subject: [PATCH 07/14] refactor: extract task value types to leaf module, break import cycle - Move TaskStatus, TaskResult, TaskDependency to task_types.py (leaf: imports only stdlib + ErrorData), so the comms renderers can import TaskStatus directly instead of comparing against status .value strings - Remove _STATUS_SUCCESS / _STATUS_ERRORED workarounds in the webhook/email internals; use TaskStatus enum members - Add Task.close_handlers(); Process.close_loggers and remove_task delegate to it, removing the duplicated handler-teardown loop (single ownership) - task.py re-exports the value types via __all__ for backward compatibility --- report-notifications-implementation.md | 88 +++++++++++ src/processes/__init__.py | 6 +- src/processes/_email_internals.py | 7 +- src/processes/_webhook_internals.py | 5 +- src/processes/execution_report.py | 2 +- src/processes/process.py | 11 +- src/processes/task.py | 199 ++---------------------- src/processes/task_types.py | 201 +++++++++++++++++++++++++ 8 files changed, 315 insertions(+), 204 deletions(-) create mode 100644 report-notifications-implementation.md create mode 100644 src/processes/task_types.py diff --git a/report-notifications-implementation.md b/report-notifications-implementation.md new file mode 100644 index 0000000..6947c31 --- /dev/null +++ b/report-notifications-implementation.md @@ -0,0 +1,88 @@ +# Report Notifications — Implementation Decisions & Summary + +## Decisiones tomadas + +### 1. Template único `report.html` (no 3 variantes por estilo) + +**Decisión:** Se creó un único `themes/styles/report.html` en lugar de 3 variantes (`report_classic.html`, `report_modern.html`, `report_compact.html`). + +**Razón:** El layout classic/modern/compact diferencia la presentación de un *único* error por tarea. Para un reporte multi-tarea el layout es inherentemente diferente (grilla de resumen + lista colapsable de tareas), y las tres variantes habrían producido diferencias cosméticas mínimas sin valor real. La palette y el language sí se respetan. + +**Implicación:** `HTMLEmailStyle.style` ("classic", "modern", "compact") no tiene efecto en los emails de reporte. Está documentado en el docstring de `_build_report_html`. + +--- + +### 2. `
    `/`` para secciones colapsables + +**Decisión:** Cada tarea se envuelve en `
    `. Las tareas con error usan `
    ` (abiertas por defecto); success/skipped se generan cerradas. + +**Razón:** Es la única opción viable con CSS puro en email. El "checkbox hack" es eliminado por la mayoría de los clientes. `
    ` es soportado en Apple Mail, Thunderbird, Gmail web. En Outlook el contenido se muestra expandido (degradación aceptable: siempre visible, nunca oculto). + +--- + +### 3. Sin `process_name` en el reporte + +**Decisión:** El asunto y el encabezado del email no incluyen el nombre del proceso. + +**Razón:** `ProcessExecutionReport` es un dataclass frozen y su contrato está estabilizado. Añadir `process_name` requeriría modificar `from_results` y el constructor, lo cual está fuera del scope de esta branch. Se usa un título genérico localizado. + +--- + +### 4. Circular import resuelto con comparación por `.value` + +**Decisión:** `_email_internals.py` y `_webhook_internals.py` no importan `TaskStatus`. En su lugar comparan `entry.status.value == "errored"` (string). + +**Razón:** La cadena de importación era: `task.py` → `notification_channels.py` → `_email_internals.py` → `task.py`. Usar el string del enum value corta el ciclo sin necesidad de lazy imports ni reorganización del módulo. + +--- + +### 5. `_post_json()` extraído como helper compartido + +**Decisión:** El signing HMAC + POST urllib fue extraído de `_WebhookHandler.emit` a `_post_json(config, payload_str)`. + +**Razón:** Antes la lógica estaba duplicada en `emit` (por tarea) y tendría que haberse duplicado de nuevo en `send_report_webhook`. Ahora ambos llaman a `_post_json`. + +--- + +### 6. `show_warnings: bool = True` en `notify`/`notify_errors` + +**Decisión:** Se añadió `show_warnings: bool = True` como kwarg a ambos métodos. Los errores se capturan con `try/except` dentro del loop y emiten `warnings.warn(...)`. + +**Razón:** Un canal que falla no debe abortar los demás. `warnings.warn` es el idioma Python correcto para alertas no-fatales; por defecto visible, silenciable con `show_warnings=False`. + +--- + +### 7. Renderers puros + transporte separado + +**Decisión:** Se crearon funciones puras: +- `_build_report_html(report, style, content, *, errors_only) -> str` +- `_build_report_webhook_payload(entries, content, config) -> dict` +- `send_report_email(...)` y `send_report_webhook(...)` como thin wrappers de transporte + +**Razón:** Sigue el patrón existente (`_HTMLEmailFormatter` / `_HTMLEmailHandler`, `_WebhookFormatter` / `_WebhookHandler`). Los renderers son testables sin I/O (los tests de HTML los invocan directamente). + +--- + +## Resumen de implementación + +### Archivos nuevos +| Archivo | Descripción | +|---|---| +| `src/processes/themes/styles/report.html` | Template HTML del reporte (palette-aware, multi-tarea, `
    ` colapsables) | +| `tests/test_report_send.py` | 21 tests: webhook payload, flags de contenido, signing HMAC, renderer HTML, transporte SMTP | + +### Archivos modificados +| Archivo | Cambios | +|---|---| +| `_webhook_internals.py` | Extraído `_post_json()`; añadidos `_build_report_webhook_payload()` y `send_report_webhook()` | +| `_email_internals.py` | Añadidos `_build_task_section_html()`, `_build_report_html()`, `send_report_email()` | +| `notification_channels.py` | `EmailChannel.send_report` y `WebhookChannel.send_report` implementados (dejaron de lanzar `NotImplementedError`) | +| `execution_report.py` | `notify`/`notify_errors` con `show_warnings: bool = True` y try/except por canal | +| `themes/languages/*.json` | 12 keys nuevas de reporte en los 6 idiomas (en, es, pt, fr, de, it) | +| `tests/test_report_notify_dispatch.py` | Removido test de `NotImplementedError`; añadidos 4 tests de `show_warnings` | + +### Estado final +- **134 tests passed** (0 failed) +- **mypy**: no issues +- **ruff**: no issues +- Branch: `feature/report-notifications` @ `5692a4d` diff --git a/src/processes/__init__.py b/src/processes/__init__.py index d0a3380..5cc9842 100644 --- a/src/processes/__init__.py +++ b/src/processes/__init__.py @@ -22,9 +22,9 @@ from .notification_channels import WebhookChannel as WebhookChannel from .process import Process as Process from .task import Task as Task -from .task import TaskDependency as TaskDependency -from .task import TaskResult as TaskResult -from .task import TaskStatus as TaskStatus +from .task_types import TaskDependency as TaskDependency +from .task_types import TaskResult as TaskResult +from .task_types import TaskStatus as TaskStatus from .webhook_config import WebhookConfig as WebhookConfig try: diff --git a/src/processes/_email_internals.py b/src/processes/_email_internals.py index 98a3838..d0b0ebb 100644 --- a/src/processes/_email_internals.py +++ b/src/processes/_email_internals.py @@ -12,13 +12,12 @@ from ._error_data import _ErrorContextFormatter from .email_config import HTMLEmailStyle, SMTPConfig +from .task_types import TaskStatus if TYPE_CHECKING: from .execution_report import ProcessExecutionReport, TaskReportEntry from .notification_channels import ReportContent -_STATUS_ERRORED = "errored" - _THEMES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "themes") _STYLES_DIR = os.path.join(_THEMES_DIR, "styles") _PALETTES_DIR = os.path.join(_THEMES_DIR, "palettes") @@ -251,7 +250,7 @@ def _build_task_section_html( "ERRORED": lang.get("lang_status_errored", "Error"), "SKIPPED": lang.get("lang_status_skipped", "Skipped"), }.get(status_val, status_val) - open_attr = " open" if entry.status.value == _STATUS_ERRORED else "" + open_attr = " open" if entry.status == TaskStatus.ERRORED else "" fn_label = html.escape(lang.get("lang_function_label", "Function")) parts = [ @@ -261,7 +260,7 @@ def _build_task_section_html( f"" ] - if entry.status.value == _STATUS_ERRORED and entry.error is not None: + if entry.status == TaskStatus.ERRORED and entry.error is not None: exc_label = html.escape(lang.get("lang_exception_label", "Exception")) parts.append( f'
    ' diff --git a/src/processes/_webhook_internals.py b/src/processes/_webhook_internals.py index f4b86fe..9ae405b 100644 --- a/src/processes/_webhook_internals.py +++ b/src/processes/_webhook_internals.py @@ -8,14 +8,13 @@ from typing import TYPE_CHECKING, Any from ._error_data import ErrorData, _ErrorContextFormatter +from .task_types import TaskStatus from .webhook_config import WebhookConfig if TYPE_CHECKING: from .execution_report import ProcessExecutionReport, TaskReportEntry from .notification_channels import ReportContent -_STATUS_SUCCESS = "success" - _SIGNATURE_HEADER = "X-Signature-SHA256" @@ -148,7 +147,7 @@ def _build_report_webhook_payload( "elapsed_seconds": entry.elapsed_seconds, "attempts": entry.attempts, } - if entry.status.value == _STATUS_SUCCESS: + if entry.status == TaskStatus.SUCCESS: task_dict["result"] = repr(entry.result) if entry.error is not None: error_dict: dict[str, Any] = { diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index e5ef15f..b63c204 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any from ._error_data import ErrorData -from .task import TaskResult, TaskStatus +from .task_types import TaskResult, TaskStatus if TYPE_CHECKING: from .notification_channels import ReportChannel diff --git a/src/processes/process.py b/src/processes/process.py index 86c5bcf..241e3d1 100644 --- a/src/processes/process.py +++ b/src/processes/process.py @@ -6,7 +6,8 @@ from ._error_data import ErrorData from .exceptions import CircularDependencyError, DependencyNotFoundError, TaskNotFoundError from .execution_report import ProcessExecutionReport -from .task import Task, TaskDependency, TaskResult, TaskStatus +from .task import Task +from .task_types import TaskDependency, TaskResult, TaskStatus __all__ = ["CircularDependencyError", "DependencyNotFoundError", "TaskNotFoundError"] @@ -264,9 +265,7 @@ def remove_task(self, task_name: str) -> None: self.tasks = [t for t in self.tasks if t.name != task_name] self._commit_or_rollback(snapshot) - for handler in list(task.logger.handlers): - handler.close() - task.logger.removeHandler(handler) + task.close_handlers() def add_task_dependency(self, task_name: str, dependency: TaskDependency) -> None: """Add a dependency to an existing task and re-resolve the graph. @@ -374,9 +373,7 @@ def close_loggers(self) -> None: Should be called when the process is done to ensure proper resource cleanup. """ for task in self.tasks: - for handler in list(task.logger.handlers): - handler.close() - task.logger.removeHandler(handler) + task.close_handlers() class ProcessRunner: diff --git a/src/processes/task.py b/src/processes/task.py index 6abe8bd..4bce5ec 100644 --- a/src/processes/task.py +++ b/src/processes/task.py @@ -3,7 +3,6 @@ import concurrent.futures import time from collections.abc import Callable -from enum import Enum from typing import TYPE_CHECKING, Any if TYPE_CHECKING: @@ -15,192 +14,9 @@ from ._tb_utils import _build_traced_vars, _build_traced_vars_location, _format_traceback from .exceptions import CircularDependencyError from .notification_channels import NotificationChannel, _FileChannel +from .task_types import TaskDependency, TaskResult, TaskStatus - -class TaskStatus(Enum): - """Outcome of a task within a process execution. - - Attributes - ---------- - PENDING - The task has not been executed yet. - SUCCESS - The task ran and its function returned without raising. - ERRORED - The task ran and its function raised an exception (after exhausting - retries, if any). - SKIPPED - The task never ran because an upstream dependency failed. - """ - - PENDING = "pending" - SUCCESS = "success" - ERRORED = "errored" - SKIPPED = "skipped" - - -class TaskResult: - """ - Container for the result of a task execution. - - Holds the outcome of running a task, including its status, return value, - and any exception that occurred. - - Attributes - ---------- - status : TaskStatus - Outcome of the task: ``PENDING``, ``SUCCESS``, ``ERRORED``, or ``SKIPPED``. - worked : bool - True if ``status`` is ``SUCCESS``, False otherwise. - result : Any - The return value of the task's function if execution succeeded, None if failed. - exception : Exception | None - The exception object if execution failed, None if successful. - error_data : ErrorData | None - Structured failure context (function, args, kwargs, traceback, traced - variables, downstream impact) when execution failed; None if the task - succeeded. - elapsed_seconds : float - Wall-clock time spent running the task across all attempts, in - seconds. Defaults to ``0.0``. - attempts : int - Number of attempts actually executed (1 or more if the task ran, - 0 if it never ran). Defaults to ``0``. - """ - - def __init__( - self, - status: TaskStatus, - result: Any, - exception: Exception | None, - error_data: ErrorData | None = None, - elapsed_seconds: float = 0.0, - attempts: int = 0, - ): - self.status = status - self.result = result - self.exception = exception - self.error_data = error_data - self.elapsed_seconds = elapsed_seconds - self.attempts = attempts - - @property - def worked(self) -> bool: - """True if the task executed successfully, False otherwise.""" - return self.status == TaskStatus.SUCCESS - - @classmethod - def pending(cls) -> TaskResult: - """Build the placeholder result for a task that has not run yet.""" - return cls(TaskStatus.PENDING, None, None) - - @classmethod - def skipped(cls) -> TaskResult: - """Build the result for a task skipped after an upstream dependency failed.""" - return cls(TaskStatus.SKIPPED, None, None) - - @classmethod - def success(cls, result: Any, *, elapsed_seconds: float = 0.0, attempts: int = 0) -> TaskResult: - """Build the result for a task whose function returned without raising.""" - return cls( - TaskStatus.SUCCESS, - result, - None, - elapsed_seconds=elapsed_seconds, - attempts=attempts, - ) - - @classmethod - def errored( - cls, - exception: Exception, - *, - error_data: ErrorData | None = None, - elapsed_seconds: float = 0.0, - attempts: int = 0, - ) -> TaskResult: - """Build the result for a task whose function raised after exhausting retries.""" - return cls( - TaskStatus.ERRORED, - None, - exception, - error_data=error_data, - elapsed_seconds=elapsed_seconds, - attempts=attempts, - ) - - -class TaskDependency: - """ - Represents a dependency relationship between tasks. - - Defines how a task depends on another task, including how the result - of the dependency should be passed to the dependent task (as additional - positional arguments, keyword arguments, or both). - - Attributes - ---------- - task_name : str - The name of the task this dependency refers to. - use_result_as_additional_args : bool - If True, the result of the dependency task will be passed as an - additional positional argument as the last argument. Defaults to False. - use_result_as_additional_kwargs : bool - If True, the result of the dependency task will be passed as a - keyword argument. Defaults to False. - additional_kwarg_name : str - The name of the keyword argument to use if ``use_result_as_additional_kwargs`` - is True. Must be a non-empty string when - ``use_result_as_additional_kwargs`` is True. Defaults to ``""``. - - Raises - ------ - TypeError - If any parameter type is invalid or if use_result_as_additional_kwargs - is True but additional_kwarg_name is not a string. - """ - - def __init__( - self, - task_name: str, - use_result_as_additional_args: bool = False, - use_result_as_additional_kwargs: bool = False, - additional_kwarg_name: str = "", - ): - self.task_name = task_name - self.use_result_as_additional_args = use_result_as_additional_args - self.use_result_as_additional_kwargs = use_result_as_additional_kwargs - self.additional_kwarg_name = additional_kwarg_name - - if not isinstance(self.task_name, str): - raise TypeError(f"task_name must be of type str. Got {type(self.task_name)}") - if not isinstance(self.use_result_as_additional_args, bool): - raise TypeError( - f"use_result_as_additional_args must be of type bool. " - f"Got {type(self.use_result_as_additional_args)}" - ) - if not isinstance(self.use_result_as_additional_kwargs, bool): - raise TypeError( - f"use_result_as_additional_kwargs must be of type bool. " - f"Got {type(self.use_result_as_additional_kwargs)}" - ) - - if self.use_result_as_additional_kwargs and self.additional_kwarg_name == "": - raise TypeError( - "If use_result_as_additional_kwargs is True, additional_kwarg_name" - " must be a non-empty string." - ) - - def __hash__(self) -> int: - """ - Return hash of the dependency based on task name. - - Returns - ------- - int - Hash value based on the task_name attribute. - """ - return hash(self.task_name) +__all__ = ["Task", "TaskDependency", "TaskResult", "TaskStatus"] class Task: @@ -421,6 +237,17 @@ def get_dependencies_names(self) -> set[str]: """ return {dependency.task_name for dependency in self.dependencies} + def close_handlers(self) -> None: + """Close and detach every handler on this task's logger. + + The task owns the lifecycle of the logger it builds in ``__init__``; + this is the single teardown path used by ``Process.close_loggers`` and + ``Process.remove_task``. Safe to call more than once. + """ + for handler in list(self.logger.handlers): + handler.close() + self.logger.removeHandler(handler) + def _call_with_timeout(self, args: list[Any], kwargs: dict[Any, Any]) -> Any: """Call ``self.func(*args, **kwargs)``, raising ``TimeoutError`` if ``self.timeout`` seconds elapse before the function returns. diff --git a/src/processes/task_types.py b/src/processes/task_types.py new file mode 100644 index 0000000..ab631bd --- /dev/null +++ b/src/processes/task_types.py @@ -0,0 +1,201 @@ +"""Pure task value types with no dependency on the communication layer. + +``TaskStatus``, ``TaskResult`` and ``TaskDependency`` are leaf domain types: +they import only the standard library and :class:`~processes._error_data.ErrorData` +(itself a leaf). Keeping them here — rather than in ``task.py``, which imports the +notification channels — lets the communication renderers import ``TaskStatus`` +directly without creating an import cycle. +""" + +from __future__ import annotations + +from enum import Enum +from typing import Any + +from ._error_data import ErrorData + + +class TaskStatus(Enum): + """Outcome of a task within a process execution. + + Attributes + ---------- + PENDING + The task has not been executed yet. + SUCCESS + The task ran and its function returned without raising. + ERRORED + The task ran and its function raised an exception (after exhausting + retries, if any). + SKIPPED + The task never ran because an upstream dependency failed. + """ + + PENDING = "pending" + SUCCESS = "success" + ERRORED = "errored" + SKIPPED = "skipped" + + +class TaskResult: + """ + Container for the result of a task execution. + + Holds the outcome of running a task, including its status, return value, + and any exception that occurred. + + Attributes + ---------- + status : TaskStatus + Outcome of the task: ``PENDING``, ``SUCCESS``, ``ERRORED``, or ``SKIPPED``. + worked : bool + True if ``status`` is ``SUCCESS``, False otherwise. + result : Any + The return value of the task's function if execution succeeded, None if failed. + exception : Exception | None + The exception object if execution failed, None if successful. + error_data : ErrorData | None + Structured failure context (function, args, kwargs, traceback, traced + variables, downstream impact) when execution failed; None if the task + succeeded. + elapsed_seconds : float + Wall-clock time spent running the task across all attempts, in + seconds. Defaults to ``0.0``. + attempts : int + Number of attempts actually executed (1 or more if the task ran, + 0 if it never ran). Defaults to ``0``. + """ + + def __init__( + self, + status: TaskStatus, + result: Any, + exception: Exception | None, + error_data: ErrorData | None = None, + elapsed_seconds: float = 0.0, + attempts: int = 0, + ): + self.status = status + self.result = result + self.exception = exception + self.error_data = error_data + self.elapsed_seconds = elapsed_seconds + self.attempts = attempts + + @property + def worked(self) -> bool: + """True if the task executed successfully, False otherwise.""" + return self.status == TaskStatus.SUCCESS + + @classmethod + def pending(cls) -> TaskResult: + """Build the placeholder result for a task that has not run yet.""" + return cls(TaskStatus.PENDING, None, None) + + @classmethod + def skipped(cls) -> TaskResult: + """Build the result for a task skipped after an upstream dependency failed.""" + return cls(TaskStatus.SKIPPED, None, None) + + @classmethod + def success(cls, result: Any, *, elapsed_seconds: float = 0.0, attempts: int = 0) -> TaskResult: + """Build the result for a task whose function returned without raising.""" + return cls( + TaskStatus.SUCCESS, + result, + None, + elapsed_seconds=elapsed_seconds, + attempts=attempts, + ) + + @classmethod + def errored( + cls, + exception: Exception, + *, + error_data: ErrorData | None = None, + elapsed_seconds: float = 0.0, + attempts: int = 0, + ) -> TaskResult: + """Build the result for a task whose function raised after exhausting retries.""" + return cls( + TaskStatus.ERRORED, + None, + exception, + error_data=error_data, + elapsed_seconds=elapsed_seconds, + attempts=attempts, + ) + + +class TaskDependency: + """ + Represents a dependency relationship between tasks. + + Defines how a task depends on another task, including how the result + of the dependency should be passed to the dependent task (as additional + positional arguments, keyword arguments, or both). + + Attributes + ---------- + task_name : str + The name of the task this dependency refers to. + use_result_as_additional_args : bool + If True, the result of the dependency task will be passed as an + additional positional argument as the last argument. Defaults to False. + use_result_as_additional_kwargs : bool + If True, the result of the dependency task will be passed as a + keyword argument. Defaults to False. + additional_kwarg_name : str + The name of the keyword argument to use if ``use_result_as_additional_kwargs`` + is True. Must be a non-empty string when + ``use_result_as_additional_kwargs`` is True. Defaults to ``""``. + + Raises + ------ + TypeError + If any parameter type is invalid or if use_result_as_additional_kwargs + is True but additional_kwarg_name is not a string. + """ + + def __init__( + self, + task_name: str, + use_result_as_additional_args: bool = False, + use_result_as_additional_kwargs: bool = False, + additional_kwarg_name: str = "", + ): + self.task_name = task_name + self.use_result_as_additional_args = use_result_as_additional_args + self.use_result_as_additional_kwargs = use_result_as_additional_kwargs + self.additional_kwarg_name = additional_kwarg_name + + if not isinstance(self.task_name, str): + raise TypeError(f"task_name must be of type str. Got {type(self.task_name)}") + if not isinstance(self.use_result_as_additional_args, bool): + raise TypeError( + f"use_result_as_additional_args must be of type bool. " + f"Got {type(self.use_result_as_additional_args)}" + ) + if not isinstance(self.use_result_as_additional_kwargs, bool): + raise TypeError( + f"use_result_as_additional_kwargs must be of type bool. " + f"Got {type(self.use_result_as_additional_kwargs)}" + ) + + if self.use_result_as_additional_kwargs and self.additional_kwarg_name == "": + raise TypeError( + "If use_result_as_additional_kwargs is True, additional_kwarg_name" + " must be a non-empty string." + ) + + def __hash__(self) -> int: + """ + Return hash of the dependency based on task name. + + Returns + ------- + int + Hash value based on the task_name attribute. + """ + return hash(self.task_name) From 846aa3d1e6d56087a1993a72761c5aa85e1083c6 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Thu, 18 Jun 2026 19:21:38 -0400 Subject: [PATCH 08/14] refactor: unify SMTP and webhook transport behind single classes - Add _SMTPTransport.send(subject, html_body): the one place that owns the SMTP conversation. _HTMLEmailHandler.emit and send_report_email both delegate to it, removing the duplicated MIME-build + connect + sendmail + quit sequences - Replace _post_json() with _WebhookTransport.post(payload): _WebhookHandler.emit and send_report_webhook both delegate, mirroring the SMTP side - No behavior change: SMTP constructor still called without timeout as before --- src/processes/_email_internals.py | 84 ++++++++++++++++------------- src/processes/_webhook_internals.py | 49 ++++++++++------- 2 files changed, 75 insertions(+), 58 deletions(-) diff --git a/src/processes/_email_internals.py b/src/processes/_email_internals.py index d0b0ebb..82a5bbe 100644 --- a/src/processes/_email_internals.py +++ b/src/processes/_email_internals.py @@ -158,6 +158,49 @@ def format(self, record: logging.LogRecord) -> str: return self._render(self._get_template(), substitutions) +class _SMTPTransport: + """Sends one HTML email per call over a fresh SMTP connection. + + The single place that owns the SMTP conversation (connect, optional + STARTTLS + login, ``sendmail``, ``quit``). Both the streaming task handler + (``_HTMLEmailHandler``) and the one-shot report sender (``send_report_email``) + delegate here, so the transport logic exists exactly once. + """ + + def __init__(self, config: SMTPConfig) -> None: + self._config = config + + def send(self, subject: str, html_body: str) -> None: + """Connect, send a single HTML message, and disconnect. + + Parameters + ---------- + subject : str + The email subject line. + html_body : str + The fully rendered HTML body. + """ + config = self._config + if isinstance(config.mailhost, tuple): + host, port = config.mailhost[0], config.mailhost[1] + else: + host, port = config.mailhost, smtplib.SMTP_PORT + + smtp = smtplib.SMTP(host, port) + mime_msg = MIMEText(html_body, "html") + mime_msg["From"] = config.fromaddr + mime_msg["To"] = ",".join(config.toaddrs) + mime_msg["Subject"] = subject + mime_msg["Date"] = formatdate() + if config.credentials is not None: + username, password = config.credentials + if config.secure is not None: + smtp.starttls(*config.secure) + smtp.login(username, password) + smtp.sendmail(config.fromaddr, config.toaddrs, mime_msg.as_string()) + smtp.quit() + + class _HTMLEmailHandler(logging.handlers.SMTPHandler): """Internal SMTP handler that sends log records as HTML emails.""" @@ -171,28 +214,11 @@ def __init__(self, config: SMTPConfig) -> None: secure=config.secure, # type: ignore[arg-type] timeout=config.timeout, ) + self._transport = _SMTPTransport(config) def emit(self, record: logging.LogRecord) -> None: try: - port = self.mailport - if not port: - port = smtplib.SMTP_PORT - host = self.mailhost[0] if isinstance(self.mailhost, tuple) else self.mailhost - smtp = smtplib.SMTP(host, port) - msg = self.format(record) - - mime_msg = MIMEText(msg, "html") - mime_msg["From"] = self.fromaddr - mime_msg["To"] = ",".join(self.toaddrs) - mime_msg["Subject"] = self.getSubject(record) - mime_msg["Date"] = formatdate() - - if self.username: - if self.secure is not None: - smtp.starttls(*self.secure) - smtp.login(self.username, self.password) - smtp.sendmail(self.fromaddr, self.toaddrs, mime_msg.as_string()) - smtp.quit() + self._transport.send(self.getSubject(record), self.format(record)) except Exception: self.handleError(record) @@ -414,22 +440,4 @@ def send_report_email( ) subject = lang.get(subject_key, "Process Execution Report") html_body = _build_report_html(report, style, content, errors_only=errors_only) - - if isinstance(smtp_config.mailhost, tuple): - host, port = smtp_config.mailhost[0], smtp_config.mailhost[1] - else: - host, port = smtp_config.mailhost, smtplib.SMTP_PORT - - smtp = smtplib.SMTP(host, port) - mime_msg = MIMEText(html_body, "html") - mime_msg["From"] = smtp_config.fromaddr - mime_msg["To"] = ",".join(smtp_config.toaddrs) - mime_msg["Subject"] = subject - mime_msg["Date"] = formatdate() - if smtp_config.credentials is not None: - username, password = smtp_config.credentials - if smtp_config.secure is not None: - smtp.starttls(*smtp_config.secure) - smtp.login(username, password) - smtp.sendmail(smtp_config.fromaddr, smtp_config.toaddrs, mime_msg.as_string()) - smtp.quit() + _SMTPTransport(smtp_config).send(subject, html_body) diff --git a/src/processes/_webhook_internals.py b/src/processes/_webhook_internals.py index 9ae405b..e7722d9 100644 --- a/src/processes/_webhook_internals.py +++ b/src/processes/_webhook_internals.py @@ -18,26 +18,35 @@ _SIGNATURE_HEADER = "X-Signature-SHA256" -def _post_json(config: WebhookConfig, payload: str) -> None: - """Sign and POST a JSON string to ``config.url``. +class _WebhookTransport: + """Signs and POSTs a JSON string to the configured webhook URL. - Parameters - ---------- - config : WebhookConfig - Transport configuration (URL, headers, timeout, optional HMAC secret). - payload : str - JSON string to POST as the request body. + The single place that owns the HTTP conversation (HMAC-SHA256 signing, + headers, POST). Both the streaming task handler (``_WebhookHandler``) and + the one-shot report sender (``send_report_webhook``) delegate here, so the + transport logic exists exactly once. """ - body = payload.encode("utf-8") - headers = {"Content-Type": "application/json", **config.headers} - if config.secret is not None: - digest = hmac.new( - config.secret.encode("utf-8"), body, hashlib.sha256 - ).hexdigest() - headers[_SIGNATURE_HEADER] = digest - request = urllib.request.Request(config.url, data=body, headers=headers, method="POST") - with urllib.request.urlopen(request, timeout=config.timeout): - pass + + def __init__(self, config: WebhookConfig) -> None: + self._config = config + + def post(self, payload: str) -> None: + """Sign (if a secret is set) and POST ``payload`` as the request body. + + Parameters + ---------- + payload : str + JSON string to POST as the request body. + """ + config = self._config + body = payload.encode("utf-8") + headers = {"Content-Type": "application/json", **config.headers} + if config.secret is not None: + digest = hmac.new(config.secret.encode("utf-8"), body, hashlib.sha256).hexdigest() + headers[_SIGNATURE_HEADER] = digest + request = urllib.request.Request(config.url, data=body, headers=headers, method="POST") + with urllib.request.urlopen(request, timeout=config.timeout): + pass class _WebhookFormatter(_ErrorContextFormatter): @@ -112,7 +121,7 @@ def __init__(self, config: WebhookConfig) -> None: def emit(self, record: logging.LogRecord) -> None: try: - _post_json(self._config, self.format(record)) + _WebhookTransport(self._config).post(self.format(record)) except Exception: self.handleError(record) @@ -191,7 +200,7 @@ def send_report_webhook( """ entries = report.errored if errors_only else report.entries payload = _build_report_webhook_payload(entries, content, config) - _post_json(config, json.dumps(payload)) + _WebhookTransport(config).post(json.dumps(payload)) def _build_task_webhook_handler(config: WebhookConfig) -> _WebhookHandler: From 2b94e634792e17e71b5dae422563df7603bd4444 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Thu, 18 Jun 2026 19:33:58 -0400 Subject: [PATCH 09/14] refactor: group communication code into a comms/ package Move the communication layer out of the flat top-level namespace into a comms/ package, separating ports from adapters: - comms/base.py: NotificationChannel, ReportChannel, ReportContent (the abstract ports the domain depends on; a leaf importing no comms internals) - comms/channels.py: EmailChannel, WebhookChannel, _FileChannel (adapters) - comms/_email.py, comms/_webhook.py: renderers + transports + handlers - comms/_logfile.py, comms/_error_context.py: logfile formatter and the LogRecord->ErrorData extractor - comms/email_config.py, comms/webhook_config.py, comms/themes/ Split _error_data.py: ErrorData becomes a top-level domain leaf (error_data.py), while _ErrorContextFormatter moves to comms/_error_context.py. This keeps task_types.py importing only leaves, so importing comms never reaches back into the domain (no cycle). Public API unchanged: every name still imports from `processes`. Updated test import paths and patch targets, plus the mkdocstrings references in docs/reference.md, to the new module homes. --- docs/reference.md | 6 +- src/processes/__init__.py | 18 +-- src/processes/comms/__init__.py | 16 +++ .../{_email_internals.py => comms/_email.py} | 8 +- src/processes/comms/_error_context.py | 37 ++++++ .../_logfile.py} | 2 +- .../_webhook.py} | 9 +- src/processes/comms/base.py | 103 +++++++++++++++++ .../channels.py} | 105 +++--------------- src/processes/{ => comms}/email_config.py | 0 .../{ => comms}/themes/languages/de.json | 0 .../{ => comms}/themes/languages/en.json | 0 .../{ => comms}/themes/languages/es.json | 0 .../{ => comms}/themes/languages/fr.json | 0 .../{ => comms}/themes/languages/it.json | 0 .../{ => comms}/themes/languages/pt.json | 0 .../themes/palettes/catppuccin.css | 0 .../{ => comms}/themes/palettes/neobones.css | 0 .../{ => comms}/themes/palettes/neutral.css | 0 .../{ => comms}/themes/palettes/slate.css | 0 .../{ => comms}/themes/styles/classic.html | 0 .../{ => comms}/themes/styles/compact.html | 0 .../{ => comms}/themes/styles/modern.html | 0 .../{ => comms}/themes/styles/report.html | 0 src/processes/{ => comms}/webhook_config.py | 0 .../{_error_data.py => error_data.py} | 33 ------ src/processes/execution_report.py | 4 +- src/processes/process.py | 2 +- src/processes/task.py | 5 +- src/processes/task_types.py | 4 +- tests/test_complex_dag_failures.py | 2 +- tests/test_email_themes.py | 8 +- tests/test_notification_channels.py | 8 +- tests/test_report_send.py | 6 +- tests/test_webhook_channel.py | 18 +-- 35 files changed, 220 insertions(+), 174 deletions(-) create mode 100644 src/processes/comms/__init__.py rename src/processes/{_email_internals.py => comms/_email.py} (98%) create mode 100644 src/processes/comms/_error_context.py rename src/processes/{_logfile_formatting.py => comms/_logfile.py} (97%) rename src/processes/{_webhook_internals.py => comms/_webhook.py} (97%) create mode 100644 src/processes/comms/base.py rename src/processes/{notification_channels.py => comms/channels.py} (68%) rename src/processes/{ => comms}/email_config.py (100%) rename src/processes/{ => comms}/themes/languages/de.json (100%) rename src/processes/{ => comms}/themes/languages/en.json (100%) rename src/processes/{ => comms}/themes/languages/es.json (100%) rename src/processes/{ => comms}/themes/languages/fr.json (100%) rename src/processes/{ => comms}/themes/languages/it.json (100%) rename src/processes/{ => comms}/themes/languages/pt.json (100%) rename src/processes/{ => comms}/themes/palettes/catppuccin.css (100%) rename src/processes/{ => comms}/themes/palettes/neobones.css (100%) rename src/processes/{ => comms}/themes/palettes/neutral.css (100%) rename src/processes/{ => comms}/themes/palettes/slate.css (100%) rename src/processes/{ => comms}/themes/styles/classic.html (100%) rename src/processes/{ => comms}/themes/styles/compact.html (100%) rename src/processes/{ => comms}/themes/styles/modern.html (100%) rename src/processes/{ => comms}/themes/styles/report.html (100%) rename src/processes/{ => comms}/webhook_config.py (100%) rename src/processes/{_error_data.py => error_data.py} (55%) diff --git a/docs/reference.md b/docs/reference.md index 1ed66a3..c04dd84 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -9,6 +9,6 @@ This page is automatically generated from the source code docstrings. ::: processes.execution_report.ProcessExecutionReport ::: processes.execution_report.TaskReportEntry ::: processes.execution_report.TaskStatus -::: processes._error_data.ErrorData -::: processes.email_config.SMTPConfig -::: processes.email_config.HTMLEmailStyle \ No newline at end of file +::: processes.error_data.ErrorData +::: processes.comms.email_config.SMTPConfig +::: processes.comms.email_config.HTMLEmailStyle \ No newline at end of file diff --git a/src/processes/__init__.py b/src/processes/__init__.py index 5cc9842..4127afb 100644 --- a/src/processes/__init__.py +++ b/src/processes/__init__.py @@ -1,9 +1,15 @@ from importlib.metadata import PackageNotFoundError as _pnfe from importlib.metadata import version as _v -from ._error_data import ErrorData as ErrorData -from .email_config import HTMLEmailStyle as HTMLEmailStyle -from .email_config import SMTPConfig as SMTPConfig +from .comms import EmailChannel as EmailChannel +from .comms import HTMLEmailStyle as HTMLEmailStyle +from .comms import NotificationChannel as NotificationChannel +from .comms import ReportChannel as ReportChannel +from .comms import ReportContent as ReportContent +from .comms import SMTPConfig as SMTPConfig +from .comms import WebhookChannel as WebhookChannel +from .comms import WebhookConfig as WebhookConfig +from .error_data import ErrorData as ErrorData from .exceptions import ( CircularDependencyError as CircularDependencyError, ) @@ -15,17 +21,11 @@ ) from .execution_report import ProcessExecutionReport as ProcessExecutionReport from .execution_report import TaskReportEntry as TaskReportEntry -from .notification_channels import EmailChannel as EmailChannel -from .notification_channels import NotificationChannel as NotificationChannel -from .notification_channels import ReportChannel as ReportChannel -from .notification_channels import ReportContent as ReportContent -from .notification_channels import WebhookChannel as WebhookChannel from .process import Process as Process from .task import Task as Task from .task_types import TaskDependency as TaskDependency from .task_types import TaskResult as TaskResult from .task_types import TaskStatus as TaskStatus -from .webhook_config import WebhookConfig as WebhookConfig try: __version__ = _v("processes") diff --git a/src/processes/comms/__init__.py b/src/processes/comms/__init__.py new file mode 100644 index 0000000..96d96d6 --- /dev/null +++ b/src/processes/comms/__init__.py @@ -0,0 +1,16 @@ +"""Communication layer: channels, transports and renderers. + +Public surface re-exported here (and, in turn, from ``processes``): +the channel ports (``NotificationChannel``, ``ReportChannel``), the shared +``ReportContent`` selector, the concrete ``EmailChannel`` / ``WebhookChannel``, +and their transport configs. +""" + +from .base import NotificationChannel as NotificationChannel +from .base import ReportChannel as ReportChannel +from .base import ReportContent as ReportContent +from .channels import EmailChannel as EmailChannel +from .channels import WebhookChannel as WebhookChannel +from .email_config import HTMLEmailStyle as HTMLEmailStyle +from .email_config import SMTPConfig as SMTPConfig +from .webhook_config import WebhookConfig as WebhookConfig diff --git a/src/processes/_email_internals.py b/src/processes/comms/_email.py similarity index 98% rename from src/processes/_email_internals.py rename to src/processes/comms/_email.py index 82a5bbe..9608ebd 100644 --- a/src/processes/_email_internals.py +++ b/src/processes/comms/_email.py @@ -10,13 +10,13 @@ from email.utils import formatdate from typing import TYPE_CHECKING, cast -from ._error_data import _ErrorContextFormatter +from ..task_types import TaskStatus +from ._error_context import _ErrorContextFormatter from .email_config import HTMLEmailStyle, SMTPConfig -from .task_types import TaskStatus if TYPE_CHECKING: - from .execution_report import ProcessExecutionReport, TaskReportEntry - from .notification_channels import ReportContent + from ..execution_report import ProcessExecutionReport, TaskReportEntry + from .base import ReportContent _THEMES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "themes") _STYLES_DIR = os.path.join(_THEMES_DIR, "styles") diff --git a/src/processes/comms/_error_context.py b/src/processes/comms/_error_context.py new file mode 100644 index 0000000..7dea78b --- /dev/null +++ b/src/processes/comms/_error_context.py @@ -0,0 +1,37 @@ +from __future__ import annotations + +import logging + +from ..error_data import ErrorData + + +class _ErrorContextFormatter(logging.Formatter): + """Base formatter providing typed access to a record's failure context.""" + + def _error_data(self, record: logging.LogRecord) -> ErrorData: + """Extract the failure context from a log record. + + Parameters + ---------- + record : logging.LogRecord + The record being formatted. May or may not carry a + ``task_context`` attribute. + + Returns + ------- + ErrorData + Typed view of ``record.task_context``, with defaults filled in + for any missing fields. + """ + ctx = getattr(record, "task_context", None) or {} + return ErrorData( + task_name=str(ctx.get("task_name", "?")), + function=str(ctx.get("function", "?")), + args=ctx.get("args", ()), + kwargs=ctx.get("kwargs", {}), + downstream_impact=ctx.get("downstream_impact", []) or [], + exception=ctx.get("exception", record.getMessage()), + traceback_str=ctx.get("traceback_str", ""), + traced_vars=ctx.get("traced_vars", {}) or {}, + traced_vars_location=ctx.get("traced_vars_location", ""), + ) diff --git a/src/processes/_logfile_formatting.py b/src/processes/comms/_logfile.py similarity index 97% rename from src/processes/_logfile_formatting.py rename to src/processes/comms/_logfile.py index 1f15650..82960b4 100644 --- a/src/processes/_logfile_formatting.py +++ b/src/processes/comms/_logfile.py @@ -2,7 +2,7 @@ import logging -from ._error_data import _ErrorContextFormatter +from ._error_context import _ErrorContextFormatter _LOG_FORMAT = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" diff --git a/src/processes/_webhook_internals.py b/src/processes/comms/_webhook.py similarity index 97% rename from src/processes/_webhook_internals.py rename to src/processes/comms/_webhook.py index e7722d9..2fdfabe 100644 --- a/src/processes/_webhook_internals.py +++ b/src/processes/comms/_webhook.py @@ -7,13 +7,14 @@ import urllib.request from typing import TYPE_CHECKING, Any -from ._error_data import ErrorData, _ErrorContextFormatter -from .task_types import TaskStatus +from ..error_data import ErrorData +from ..task_types import TaskStatus +from ._error_context import _ErrorContextFormatter from .webhook_config import WebhookConfig if TYPE_CHECKING: - from .execution_report import ProcessExecutionReport, TaskReportEntry - from .notification_channels import ReportContent + from ..execution_report import ProcessExecutionReport, TaskReportEntry + from .base import ReportContent _SIGNATURE_HEADER = "X-Signature-SHA256" diff --git a/src/processes/comms/base.py b/src/processes/comms/base.py new file mode 100644 index 0000000..c5777c2 --- /dev/null +++ b/src/processes/comms/base.py @@ -0,0 +1,103 @@ +"""Communication ports: the abstract channel interfaces the domain depends on. + +These abstractions are deliberately a leaf within ``comms`` — they import no +concrete channel, transport, or renderer — so ``task.py`` (streaming) and +``execution_report.py`` (one-shot) can depend on the *interface* without pulling +in the email/webhook implementations. +""" + +from __future__ import annotations + +import logging +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from ..execution_report import ProcessExecutionReport + + +@dataclass(frozen=True) +class ReportContent: + """What detail a report notification includes. + + Channel-agnostic content selection, shared by every ``ReportChannel``. + Construct once and pass the same instance to several channels for uniform + content, or give each channel its own for per-destination verbosity. + + Attributes + ---------- + show_traceback : bool + Include each failure's full traceback. Defaults to ``True``. + show_traced_vars : bool + Include each failure's traced local variables. Defaults to ``True``. + """ + + show_traceback: bool = True + show_traced_vars: bool = True + + +class ReportChannel(ABC): + """Base class for channels that deliver a finished ``ProcessExecutionReport``. + + Unlike ``NotificationChannel`` (which builds a streaming ``logging.Handler`` + for a single ``Task``), a report channel sends a complete report **once**, + after the run. ``ProcessExecutionReport.notify`` / ``notify_errors`` iterate + the channels they are given and call ``send_report`` on each. + """ + + @abstractmethod + def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: + """Deliver ``report`` to this channel's destination. + + Parameters + ---------- + report : ProcessExecutionReport + The finished report to deliver. + errors_only : bool + If True, only the ``ERRORED`` entries are sent; otherwise the whole + report is sent. + """ + + +class NotificationChannel(ABC): + """Base class for task notification channels. + + A notification channel knows how to build a configured + ``logging.Handler`` that delivers a task's log records (and, on + failure, its structured failure context) to some destination. ``Task`` + attaches one handler per configured channel to its logger. + + Concrete channels wrap a specific delivery mechanism (e.g. a logfile or + an email alert). New channels can be added by subclassing + ``NotificationChannel`` and implementing ``build_handler``. + """ + + @abstractmethod + def build_handler(self, task_name: str) -> logging.Handler: + """Build a configured handler for the given task. + + Parameters + ---------- + task_name : str + Name of the task the handler will be attached to. + + Returns + ------- + logging.Handler + A handler ready to be added to the task's logger. + """ + + @property + def frame_filter(self) -> str | None: + """Substring selecting the traceback frame to trace local variables of. + + See ``HTMLEmailStyle.traced_vars_frame_filter``. Channels that don't + influence frame selection return ``None`` (the default). + + Returns + ------- + str | None + ``None`` unless overridden by a subclass. + """ + return None diff --git a/src/processes/notification_channels.py b/src/processes/comms/channels.py similarity index 68% rename from src/processes/notification_channels.py rename to src/processes/comms/channels.py index b3cd0b1..602b890 100644 --- a/src/processes/notification_channels.py +++ b/src/processes/comms/channels.py @@ -1,104 +1,25 @@ from __future__ import annotations import logging -from abc import ABC, abstractmethod -from dataclasses import dataclass from typing import TYPE_CHECKING -from ._email_internals import _build_task_email_handler, send_report_email -from ._logfile_formatting import _TaskLogfileFormatter -from ._webhook_internals import _build_task_webhook_handler, send_report_webhook +from ._email import _build_task_email_handler, send_report_email +from ._logfile import _TaskLogfileFormatter +from ._webhook import _build_task_webhook_handler, send_report_webhook +from .base import NotificationChannel, ReportChannel, ReportContent from .email_config import HTMLEmailStyle, SMTPConfig from .webhook_config import WebhookConfig if TYPE_CHECKING: - from .execution_report import ProcessExecutionReport - - -@dataclass(frozen=True) -class ReportContent: - """What detail a report notification includes. - - Channel-agnostic content selection, shared by every ``ReportChannel``. - Construct once and pass the same instance to several channels for uniform - content, or give each channel its own for per-destination verbosity. - - Attributes - ---------- - show_traceback : bool - Include each failure's full traceback. Defaults to ``True``. - show_traced_vars : bool - Include each failure's traced local variables. Defaults to ``True``. - """ - - show_traceback: bool = True - show_traced_vars: bool = True - - -class ReportChannel(ABC): - """Base class for channels that deliver a finished ``ProcessExecutionReport``. - - Unlike ``NotificationChannel`` (which builds a streaming ``logging.Handler`` - for a single ``Task``), a report channel sends a complete report **once**, - after the run. ``ProcessExecutionReport.notify`` / ``notify_errors`` iterate - the channels they are given and call ``send_report`` on each. - """ - - @abstractmethod - def send_report(self, report: ProcessExecutionReport, *, errors_only: bool) -> None: - """Deliver ``report`` to this channel's destination. - - Parameters - ---------- - report : ProcessExecutionReport - The finished report to deliver. - errors_only : bool - If True, only the ``ERRORED`` entries are sent; otherwise the whole - report is sent. - """ - - -class NotificationChannel(ABC): - """Base class for task notification channels. - - A notification channel knows how to build a configured - ``logging.Handler`` that delivers a task's log records (and, on - failure, its structured failure context) to some destination. ``Task`` - attaches one handler per configured channel to its logger. - - Concrete channels wrap a specific delivery mechanism (e.g. a logfile or - an email alert). New channels can be added by subclassing - ``NotificationChannel`` and implementing ``build_handler``. - """ - - @abstractmethod - def build_handler(self, task_name: str) -> logging.Handler: - """Build a configured handler for the given task. - - Parameters - ---------- - task_name : str - Name of the task the handler will be attached to. - - Returns - ------- - logging.Handler - A handler ready to be added to the task's logger. - """ - - @property - def frame_filter(self) -> str | None: - """Substring selecting the traceback frame to trace local variables of. - - See ``HTMLEmailStyle.traced_vars_frame_filter``. Channels that don't - influence frame selection return ``None`` (the default). - - Returns - ------- - str | None - ``None`` unless overridden by a subclass. - """ - return None + from ..execution_report import ProcessExecutionReport + +__all__ = [ + "EmailChannel", + "NotificationChannel", + "ReportChannel", + "ReportContent", + "WebhookChannel", +] class _FileChannel(NotificationChannel): diff --git a/src/processes/email_config.py b/src/processes/comms/email_config.py similarity index 100% rename from src/processes/email_config.py rename to src/processes/comms/email_config.py diff --git a/src/processes/themes/languages/de.json b/src/processes/comms/themes/languages/de.json similarity index 100% rename from src/processes/themes/languages/de.json rename to src/processes/comms/themes/languages/de.json diff --git a/src/processes/themes/languages/en.json b/src/processes/comms/themes/languages/en.json similarity index 100% rename from src/processes/themes/languages/en.json rename to src/processes/comms/themes/languages/en.json diff --git a/src/processes/themes/languages/es.json b/src/processes/comms/themes/languages/es.json similarity index 100% rename from src/processes/themes/languages/es.json rename to src/processes/comms/themes/languages/es.json diff --git a/src/processes/themes/languages/fr.json b/src/processes/comms/themes/languages/fr.json similarity index 100% rename from src/processes/themes/languages/fr.json rename to src/processes/comms/themes/languages/fr.json diff --git a/src/processes/themes/languages/it.json b/src/processes/comms/themes/languages/it.json similarity index 100% rename from src/processes/themes/languages/it.json rename to src/processes/comms/themes/languages/it.json diff --git a/src/processes/themes/languages/pt.json b/src/processes/comms/themes/languages/pt.json similarity index 100% rename from src/processes/themes/languages/pt.json rename to src/processes/comms/themes/languages/pt.json diff --git a/src/processes/themes/palettes/catppuccin.css b/src/processes/comms/themes/palettes/catppuccin.css similarity index 100% rename from src/processes/themes/palettes/catppuccin.css rename to src/processes/comms/themes/palettes/catppuccin.css diff --git a/src/processes/themes/palettes/neobones.css b/src/processes/comms/themes/palettes/neobones.css similarity index 100% rename from src/processes/themes/palettes/neobones.css rename to src/processes/comms/themes/palettes/neobones.css diff --git a/src/processes/themes/palettes/neutral.css b/src/processes/comms/themes/palettes/neutral.css similarity index 100% rename from src/processes/themes/palettes/neutral.css rename to src/processes/comms/themes/palettes/neutral.css diff --git a/src/processes/themes/palettes/slate.css b/src/processes/comms/themes/palettes/slate.css similarity index 100% rename from src/processes/themes/palettes/slate.css rename to src/processes/comms/themes/palettes/slate.css diff --git a/src/processes/themes/styles/classic.html b/src/processes/comms/themes/styles/classic.html similarity index 100% rename from src/processes/themes/styles/classic.html rename to src/processes/comms/themes/styles/classic.html diff --git a/src/processes/themes/styles/compact.html b/src/processes/comms/themes/styles/compact.html similarity index 100% rename from src/processes/themes/styles/compact.html rename to src/processes/comms/themes/styles/compact.html diff --git a/src/processes/themes/styles/modern.html b/src/processes/comms/themes/styles/modern.html similarity index 100% rename from src/processes/themes/styles/modern.html rename to src/processes/comms/themes/styles/modern.html diff --git a/src/processes/themes/styles/report.html b/src/processes/comms/themes/styles/report.html similarity index 100% rename from src/processes/themes/styles/report.html rename to src/processes/comms/themes/styles/report.html diff --git a/src/processes/webhook_config.py b/src/processes/comms/webhook_config.py similarity index 100% rename from src/processes/webhook_config.py rename to src/processes/comms/webhook_config.py diff --git a/src/processes/_error_data.py b/src/processes/error_data.py similarity index 55% rename from src/processes/_error_data.py rename to src/processes/error_data.py index 2aa7a4b..c584280 100644 --- a/src/processes/_error_data.py +++ b/src/processes/error_data.py @@ -1,6 +1,5 @@ from __future__ import annotations -import logging from dataclasses import dataclass, field from typing import Any @@ -41,35 +40,3 @@ class ErrorData: traceback_str: str = "" traced_vars: dict[str, str] = field(default_factory=dict) traced_vars_location: str = "" - - -class _ErrorContextFormatter(logging.Formatter): - """Base formatter providing typed access to a record's failure context.""" - - def _error_data(self, record: logging.LogRecord) -> ErrorData: - """Extract the failure context from a log record. - - Parameters - ---------- - record : logging.LogRecord - The record being formatted. May or may not carry a - ``task_context`` attribute. - - Returns - ------- - ErrorData - Typed view of ``record.task_context``, with defaults filled in - for any missing fields. - """ - ctx = getattr(record, "task_context", None) or {} - return ErrorData( - task_name=str(ctx.get("task_name", "?")), - function=str(ctx.get("function", "?")), - args=ctx.get("args", ()), - kwargs=ctx.get("kwargs", {}), - downstream_impact=ctx.get("downstream_impact", []) or [], - exception=ctx.get("exception", record.getMessage()), - traceback_str=ctx.get("traceback_str", ""), - traced_vars=ctx.get("traced_vars", {}) or {}, - traced_vars_location=ctx.get("traced_vars_location", ""), - ) diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index b63c204..07a3ab0 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -6,11 +6,11 @@ from enum import Enum from typing import TYPE_CHECKING, Any -from ._error_data import ErrorData +from .error_data import ErrorData from .task_types import TaskResult, TaskStatus if TYPE_CHECKING: - from .notification_channels import ReportChannel + from .comms.base import ReportChannel from .process import Process diff --git a/src/processes/process.py b/src/processes/process.py index 241e3d1..477cc79 100644 --- a/src/processes/process.py +++ b/src/processes/process.py @@ -3,7 +3,7 @@ from types import TracebackType from typing import Literal, Self -from ._error_data import ErrorData +from .error_data import ErrorData from .exceptions import CircularDependencyError, DependencyNotFoundError, TaskNotFoundError from .execution_report import ProcessExecutionReport from .task import Task diff --git a/src/processes/task.py b/src/processes/task.py index 4bce5ec..53aae5a 100644 --- a/src/processes/task.py +++ b/src/processes/task.py @@ -10,10 +10,11 @@ import logging -from ._error_data import ErrorData from ._tb_utils import _build_traced_vars, _build_traced_vars_location, _format_traceback +from .comms.base import NotificationChannel +from .comms.channels import _FileChannel +from .error_data import ErrorData from .exceptions import CircularDependencyError -from .notification_channels import NotificationChannel, _FileChannel from .task_types import TaskDependency, TaskResult, TaskStatus __all__ = ["Task", "TaskDependency", "TaskResult", "TaskStatus"] diff --git a/src/processes/task_types.py b/src/processes/task_types.py index ab631bd..6f11eb0 100644 --- a/src/processes/task_types.py +++ b/src/processes/task_types.py @@ -1,7 +1,7 @@ """Pure task value types with no dependency on the communication layer. ``TaskStatus``, ``TaskResult`` and ``TaskDependency`` are leaf domain types: -they import only the standard library and :class:`~processes._error_data.ErrorData` +they import only the standard library and :class:`~processes.error_data.ErrorData` (itself a leaf). Keeping them here — rather than in ``task.py``, which imports the notification channels — lets the communication renderers import ``TaskStatus`` directly without creating an import cycle. @@ -12,7 +12,7 @@ from enum import Enum from typing import Any -from ._error_data import ErrorData +from .error_data import ErrorData class TaskStatus(Enum): diff --git a/tests/test_complex_dag_failures.py b/tests/test_complex_dag_failures.py index 630a6d0..fc14e35 100644 --- a/tests/test_complex_dag_failures.py +++ b/tests/test_complex_dag_failures.py @@ -127,7 +127,7 @@ def make_task(name: str, deps, fail: bool = False) -> Task: t.logger.addHandler(rec) recorders[t.name] = rec - with patch("processes._email_internals.smtplib.SMTP") as mock_smtp_class: + with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: with Process(tasks) as process: result = process.run(parallel=True, max_workers=4) diff --git a/tests/test_email_themes.py b/tests/test_email_themes.py index 95e1da9..b1cce4d 100644 --- a/tests/test_email_themes.py +++ b/tests/test_email_themes.py @@ -25,12 +25,12 @@ import pytest from processes import EmailChannel, HTMLEmailStyle, Process, SMTPConfig, Task -from processes._email_internals import _HTMLEmailFormatter from processes._tb_utils import ( _build_traced_vars, _build_traced_vars_location, _format_traceback, ) +from processes.comms._email import _HTMLEmailFormatter from .base_test import BaseTest @@ -376,7 +376,7 @@ def boom() -> None: channels=[EmailChannel(smtp_cfg, style_cfg)], ) - with patch("processes._email_internals.smtplib.SMTP") as mock_smtp_class: + with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: with Process([task]) as process: process.run(parallel=False) @@ -419,7 +419,7 @@ def boom() -> None: channels=[EmailChannel(smtp_cfg, style_cfg)], ) - with patch("processes._email_internals.smtplib.SMTP") as mock_smtp_class: + with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: with Process([task]) as process: process.run(parallel=False) @@ -452,7 +452,7 @@ def boom() -> None: channels=[EmailChannel(smtp_cfg)], ) - with patch("processes._email_internals.smtplib.SMTP") as mock_smtp_class: + with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: with Process([task]) as process: process.run(parallel=False) diff --git a/tests/test_notification_channels.py b/tests/test_notification_channels.py index 75999da..2b4969f 100644 --- a/tests/test_notification_channels.py +++ b/tests/test_notification_channels.py @@ -26,10 +26,10 @@ WebhookChannel, WebhookConfig, ) -from processes._email_internals import _HTMLEmailFormatter -from processes._logfile_formatting import _TaskLogfileFormatter -from processes._webhook_internals import _WebhookFormatter -from processes.notification_channels import _FileChannel +from processes.comms._email import _HTMLEmailFormatter +from processes.comms._logfile import _TaskLogfileFormatter +from processes.comms._webhook import _WebhookFormatter +from processes.comms.channels import _FileChannel from .base_test import BaseTest diff --git a/tests/test_report_send.py b/tests/test_report_send.py index 2ded832..8775f59 100644 --- a/tests/test_report_send.py +++ b/tests/test_report_send.py @@ -8,6 +8,7 @@ from processes import ( EmailChannel, + HTMLEmailStyle, ProcessExecutionReport, ReportContent, SMTPConfig, @@ -16,9 +17,8 @@ WebhookChannel, WebhookConfig, ) -from processes._email_internals import _build_report_html -from processes._error_data import ErrorData -from processes.email_config import HTMLEmailStyle +from processes.comms._email import _build_report_html +from processes.error_data import ErrorData # --------------------------------------------------------------------------- # Helpers diff --git a/tests/test_webhook_channel.py b/tests/test_webhook_channel.py index 4c259a4..a0bc5a4 100644 --- a/tests/test_webhook_channel.py +++ b/tests/test_webhook_channel.py @@ -20,7 +20,7 @@ from unittest.mock import MagicMock, patch from processes import WebhookChannel, WebhookConfig -from processes._webhook_internals import _build_task_webhook_handler, _WebhookFormatter +from processes.comms._webhook import _build_task_webhook_handler, _WebhookFormatter from .base_test import BaseTest @@ -91,7 +91,7 @@ def _config(self, **overrides: object) -> WebhookConfig: def test_emit_posts_json_payload(self) -> None: handler = _build_task_webhook_handler(self._config()) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -105,7 +105,7 @@ def test_emit_posts_json_payload(self) -> None: def test_default_content_type_header(self) -> None: handler = _build_task_webhook_handler(self._config()) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -116,7 +116,7 @@ def test_custom_headers_merge_with_default_content_type(self) -> None: config = self._config(headers={"Authorization": "Bearer token123"}, timeout=10) handler = _build_task_webhook_handler(config) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -129,7 +129,7 @@ def test_hmac_signature_added_when_secret_set(self) -> None: config = self._config(secret="shh") handler = _build_task_webhook_handler(config) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -140,7 +140,7 @@ def test_hmac_signature_added_when_secret_set(self) -> None: def test_no_signature_header_when_secret_none(self) -> None: handler = _build_task_webhook_handler(self._config()) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -151,7 +151,7 @@ def test_extra_payload_from_config_is_merged_into_body(self) -> None: config = self._config(extra_payload={"chat_id": "12345"}) handler = _build_task_webhook_handler(config) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record()) @@ -163,7 +163,7 @@ def test_extra_payload_from_config_is_merged_into_body(self) -> None: def test_emit_routes_request_errors_through_handle_error(self) -> None: handler = _build_task_webhook_handler(self._config()) - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.side_effect = OSError("connection refused") with patch.object(handler, "handleError") as mock_handle_error: handler.emit(_make_record()) @@ -176,7 +176,7 @@ def test_channel_builds_handler_using_webhook_internals(self) -> None: channel = WebhookChannel(WebhookConfig(url="https://example.test/hook")) handler = channel.build_handler("webhook_task") - with patch("processes._webhook_internals.urllib.request.urlopen") as mock_urlopen: + with patch("processes.comms._webhook.urllib.request.urlopen") as mock_urlopen: mock_urlopen.return_value = MagicMock() handler.emit(_make_record("webhook_task")) From 50b9808bfeebf701e4c627923b1c46651c8aa47c Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Thu, 18 Jun 2026 19:38:23 -0400 Subject: [PATCH 10/14] docs: record as-built deviations and Phase 3 skip in architecture plan --- arquitectura.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arquitectura.md b/arquitectura.md index 1e9a988..4ea114d 100644 --- a/arquitectura.md +++ b/arquitectura.md @@ -315,3 +315,25 @@ añade un adaptador. Esa es la escalabilidad que se busca. Recomendación: aprobar e implementar **Fase 0 y 1** primero (correctitud y deduplicación, riesgo mínimo, sin cambio de API). La **Fase 2** (paquete) es la más "churny" y conviene revisarla como PR aparte. + +--- + +## 8. Estado de implementación (as-built) + +Fases 0, 1 y 2 implementadas y verdes (134 tests, mypy, ruff; wheel verificado +con `comms/themes/` incluido). Desviaciones menores respecto al plan, todas +deliberadas: + +- **Configs no se fusionaron** en un único `config.py`: se mantienen + `comms/email_config.py` y `comms/webhook_config.py` (menos churn, sin + beneficio real en fusionar). +- **Nombres de módulo de delivery**: `comms/_email.py` y `comms/_webhook.py` + (en vez de `_smtp.py`/`_webhook.py`); contienen render + transporte + + handler de su medio. `_logfile_formatting.py` → `comms/_logfile.py`. +- **Sin shims** en rutas viejas: los tests acoplados a rutas internas se + repuntaron a la nueva ubicación (o al import público cuando el símbolo es + público). `html_logging.py` ya no existía, así que no hubo shim que mantener. +- **Fase 3 (separar render/transport en archivos distintos): no realizada.** + Las clases de transporte (`_SMTPTransport`, `_WebhookTransport`) ya quedan + nítidamente delimitadas dentro de sus archivos; separarlas más sería + fragmentación sin beneficio. Queda como refinamiento opcional futuro. From 1abcb2399239a6f6290ce188427a8e05c9fbe747 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Thu, 18 Jun 2026 20:12:12 -0400 Subject: [PATCH 11/14] test: send real email through in-process SMTP instead of mocking smtplib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add aiosmtpd as a dev dependency and a `smtp_server` fixture (conftest.py) that runs a real in-process SMTP server capturing delivered messages. Email-send tests now exercise the full path — smtplib conversation, MIME serialization, recipients — and assert on what is actually received. - test_report_send.py: TestEmailSendReport rewritten as integration tests (delivered count, From/To/Subject headers, decoded HTML body, content flags, errors_only excludes successes). Called via send_report directly so transport failures propagate. - test_email_themes.py: TestTaskEmailWiring streaming tests send real emails and assert on the received body/subject; added a negative guard that a successful run delivers zero alerts. Pure formatter render-matrix tests kept unchanged. - test_complex_dag_failures.py: OUTCOME #4 now asserts exactly one received email per failing task with the correct theme and Downstream Impact, replacing the smtplib instantiation-count mock. Runtime deps unchanged (aiosmtpd is dev-only; wheel still zero-dependency). --- pyproject.toml | 1 + tests/conftest.py | 75 +++++++++++++++++++ tests/test_complex_dag_failures.py | 78 ++++++++----------- tests/test_email_themes.py | 108 ++++++++++++--------------- tests/test_report_send.py | 115 +++++++++++++++++------------ uv.lock | 33 +++++++++ 6 files changed, 254 insertions(+), 156 deletions(-) create mode 100644 tests/conftest.py diff --git a/pyproject.toml b/pyproject.toml index 9e0d001..f7048f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ dependencies = [] [dependency-groups] dev = [ + "aiosmtpd>=1.4.6", "commitizen>=4.11.6", "mkdocs>=1.6.1", "mkdocs-material[imaging]>=9.7.1", diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..1f2a3ef --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,75 @@ +"""Shared pytest fixtures. + +``smtp_server`` runs a real in-process SMTP server (aiosmtpd) that captures every +delivered message in memory, so email tests exercise the full send path — +``smtplib`` conversation, MIME serialization, recipients — and assert on what is +actually *received*, instead of mocking ``smtplib.SMTP``. +""" + +from __future__ import annotations + +import socket +from collections.abc import Iterator +from email import message_from_bytes +from email.message import Message + +import pytest +from aiosmtpd.controller import Controller + + +class _CapturingHandler: + """aiosmtpd handler that records every received message in memory.""" + + def __init__(self) -> None: + self.messages: list[Message] = [] + + async def handle_DATA(self, server: object, session: object, envelope: object) -> str: + content = envelope.content # type: ignore[attr-defined] + self.messages.append(message_from_bytes(content)) + return "250 Message accepted for delivery" + + +class SMTPCapture: + """A running capture server: connection details plus the received messages.""" + + def __init__(self, host: str, port: int, handler: _CapturingHandler) -> None: + self.host = host + self.port = port + self._handler = handler + + @property + def messages(self) -> list[Message]: + """Every message received so far, in arrival order.""" + return self._handler.messages + + def last(self) -> Message: + """The most recently received message (asserts at least one arrived).""" + assert self._handler.messages, "no email was received by the capture server" + return self._handler.messages[-1] + + def last_html(self) -> str: + """The decoded text body of the most recently received message.""" + payload = self.last().get_payload(decode=True) + if isinstance(payload, bytes): + return payload.decode("utf-8", errors="replace") + return str(payload or "") + + +def _free_port() -> int: + """Reserve and return an ephemeral localhost port.""" + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return int(sock.getsockname()[1]) + + +@pytest.fixture +def smtp_server() -> Iterator[SMTPCapture]: + """Start a fresh in-process SMTP capture server for one test.""" + handler = _CapturingHandler() + host, port = "127.0.0.1", _free_port() + controller = Controller(handler, hostname=host, port=port) + controller.start() + try: + yield SMTPCapture(host, port, handler) + finally: + controller.stop() diff --git a/tests/test_complex_dag_failures.py b/tests/test_complex_dag_failures.py index fc14e35..fadd5dc 100644 --- a/tests/test_complex_dag_failures.py +++ b/tests/test_complex_dag_failures.py @@ -28,11 +28,11 @@ import re from collections import defaultdict from collections.abc import Iterable -from unittest.mock import patch from processes import EmailChannel, Process, SMTPConfig, Task, TaskDependency from .base_test import BaseTest +from .conftest import SMTPCapture class _RecordHandler(logging.Handler): @@ -61,7 +61,7 @@ def _ancestors_of(task_name: str, tasks: Iterable[Task]) -> set[str]: class TestComplexDagFailures(BaseTest): - def test_complex_dag_dual_independent_failures(self) -> None: + def test_complex_dag_dual_independent_failures(self, smtp_server: SMTPCapture) -> None: """Enterprise-pipeline integration test (14-task DAG, dual failures).""" call_counts: dict[str, int] = defaultdict(int) @@ -76,7 +76,7 @@ def _func(*args, **kwargs): return _func smtp_config = SMTPConfig( - mailhost=("smtp.enterprise.test", 25), + mailhost=(smtp_server.host, smtp_server.port), fromaddr="pipeline-alerts@enterprise.test", toaddrs=["sre-oncall@enterprise.test"], ) @@ -127,9 +127,8 @@ def make_task(name: str, deps, fail: bool = False) -> Task: t.logger.addHandler(rec) recorders[t.name] = rec - with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: - with Process(tasks) as process: - result = process.run(parallel=True, max_workers=4) + with Process(tasks) as process: + result = process.run(parallel=True, max_workers=4) # OUTCOME #1 — Independent Execution assert independent_task_names == {"A0", "A1", "A2", "A3", "B0", "B1", "B2", "C0", "C1"} @@ -218,78 +217,65 @@ def make_task(name: str, deps, fail: bool = False) -> Task: f"Task {name} task_context contains an HTML entity ({entity}): {serialized}" ) - # OUTCOME #4 — Mocked Alert Validation - assert mock_smtp_class.call_count == len(failing_task_names), ( - f"smtplib.SMTP should be instantiated {len(failing_task_names)} times, " - f"got {mock_smtp_class.call_count}" - ) - for c in mock_smtp_class.call_args_list: - assert isinstance(c.args[0], str), ( - f"smtplib.SMTP host arg must be a string, got {type(c.args[0]).__name__}: " - f"{c.args[0]!r}" - ) - assert c.args[0] == "smtp.enterprise.test" - assert c.args[1] == 25 - - smtp_instance = mock_smtp_class.return_value - sendmail_calls = smtp_instance.sendmail.call_args_list - assert len(sendmail_calls) == len(failing_task_names), ( - f"sendmail should fire {len(failing_task_names)} times, got {len(sendmail_calls)}" + # OUTCOME #4 — Real Alert Delivery: exactly one HTML email per failing + # task, each carrying the default theme and its accurate Downstream Impact. + assert len(smtp_server.messages) == len(failing_task_names), ( + f"exactly {len(failing_task_names)} failure emails should be delivered, " + f"got {len(smtp_server.messages)}" ) - for call in sendmail_calls: - fromaddr, toaddrs, msg = call.args + delivered_failing: set[str] = set() + for m in smtp_server.messages: + assert m["From"] == "pipeline-alerts@enterprise.test" + assert m["To"] == "sre-oncall@enterprise.test" + assert m.get_content_type() == "text/html" - assert fromaddr == "pipeline-alerts@enterprise.test" - assert toaddrs == ["sre-oncall@enterprise.test"] - assert "From: pipeline-alerts@enterprise.test" in msg - assert "To: sre-oncall@enterprise.test" in msg - assert "Subject: Error in task " in msg - assert "MIME-Version: 1.0" in msg - assert "Content-Type: text/html" in msg + body = (m.get_payload(decode=True) or b"").decode("utf-8", errors="replace") - match = re.search(r"Pipeline Failure: (\w+)", msg) + match = re.search(r"Pipeline Failure: (\w+)", body) assert match is not None, "Email body missing per-task failure heading" failing = match.group(1) assert failing in failing_task_names - assert f"Subject: Error in task {failing}" in msg, ( - f"Subject should be 'Error in task {failing}'" + delivered_failing.add(failing) + assert m["Subject"] == f"Error in task {failing}", ( + f"Subject should be 'Error in task {failing}', got {m['Subject']!r}" ) - assert "--accent: #2563eb" in msg, ( + assert "--accent: #2563eb" in body, ( "Default 'neutral' palette marker missing from email body — " "formatter did not load the bundled theme" ) - assert 'class="card"' in msg, ( + assert 'class="card"' in body, ( "Default 'modern' style marker missing from email body — " "formatter did not load the bundled theme" ) - assert 'class="header"' in msg, ( + assert 'class="header"' in body, ( "Email body missing the modern 'header' wrapper around the failure heading" ) - assert "Pipeline Failure:" in msg, "Email body missing the per-task failure heading" - assert "

    Downstream Impact

    " in msg, ( + assert "

    Downstream Impact

    " in body, ( "Email body missing 'Downstream Impact' heading" ) - assert "" in msg, "Email body missing
      list" + assert "" in body, "Email body missing
        list" expected_downstream = sorted( n for n in skipped_task_names if failing in _ancestors_of(n, tasks) ) for ds in expected_downstream: - assert f"
      • {ds}
        • " in msg, ( + assert f"
      • {ds}
        • " in body, ( f"Email for {failing} is missing downstream impact entry for {ds!r}" ) other_branch_downstream = sorted( n for n in skipped_task_names if failing not in _ancestors_of(n, tasks) ) for ds in other_branch_downstream: - assert f"
      • {ds}
        • " not in msg, ( + assert f"
      • {ds}
        • " not in body, ( f"Email for {failing} incorrectly lists downstream entry " f"for {ds!r} from the other failure branch" ) - assert f"func_{failing}" in msg, "Function name missing from email body" - assert "Planned enterprise failure" in msg, "Exception text missing from email body" + assert f"func_{failing}" in body, "Function name missing from email body" + assert "Planned enterprise failure" in body, "Exception text missing from email body" - assert smtp_instance.quit.call_count == len(failing_task_names) + assert delivered_failing == failing_task_names, ( + f"delivered alerts should cover exactly the failing tasks, got {delivered_failing}" + ) diff --git a/tests/test_email_themes.py b/tests/test_email_themes.py index b1cce4d..7609da6 100644 --- a/tests/test_email_themes.py +++ b/tests/test_email_themes.py @@ -19,8 +19,6 @@ import logging import logging.handlers -from email import message_from_string -from unittest.mock import patch import pytest @@ -33,14 +31,7 @@ from processes.comms._email import _HTMLEmailFormatter from .base_test import BaseTest - - -def _decode_mime_body(msg: str) -> str: - """Decode the base64 HTML body from a sendmail payload.""" - parsed = message_from_string(msg) - payload = parsed.get_payload(decode=True) or b"" - return payload.decode("utf-8", errors="replace") - +from .conftest import SMTPCapture _STYLE_MARKERS = { "classic": ["

        Pipeline Failure:", 'class="impact"', 'class="traceback"'], @@ -355,15 +346,17 @@ def _inner() -> None: class TestTaskEmailWiring(BaseTest): - def test_task_wiring_propagates_style_palette_language(self) -> None: - """An EmailChannel with a non-default style attached to a Task must produce - an email body that carries the chosen style, palette and language, and a - subject carrying the language prefix.""" - smtp_cfg = SMTPConfig( - mailhost=("smtp.enterprise.test", 25), + @staticmethod + def _cfg(server: SMTPCapture) -> SMTPConfig: + return SMTPConfig( + mailhost=(server.host, server.port), fromaddr="alerts@enterprise.test", toaddrs=["oncall@enterprise.test"], ) + + def test_task_wiring_propagates_style_palette_language(self, smtp_server: SMTPCapture) -> None: + """A failing Task with an EmailChannel must deliver one email whose body + carries the chosen style, palette and language.""" style_cfg = HTMLEmailStyle(style="modern", palette="catppuccin", language="es") def boom() -> None: @@ -373,19 +366,14 @@ def boom() -> None: name="wired", log_path=self._log("wired_task.log"), func=boom, - channels=[EmailChannel(smtp_cfg, style_cfg)], + channels=[EmailChannel(self._cfg(smtp_server), style_cfg)], ) - with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: - with Process([task]) as process: - process.run(parallel=False) + with Process([task]) as process: + process.run(parallel=False) - smtp_instance = mock_smtp_class.return_value - assert smtp_instance.sendmail.call_count == 1, ( - "Failing task should trigger exactly one sendmail call" - ) - _fromaddr, _toaddrs, msg = smtp_instance.sendmail.call_args.args - body = _decode_mime_body(msg) + assert len(smtp_server.messages) == 1, "Failing task should deliver exactly one email" + body = smtp_server.last_html() assert 'class="card"' in body, ( "Rendered email body is missing the 'modern' style marker class=\"card\"" @@ -399,14 +387,9 @@ def boom() -> None: assert "Función" in body, "Rendered email body is missing the Spanish 'lang_function_label'" assert "planned end-to-end failure" in body - def test_task_subject_carries_language_prefix(self) -> None: - """The subject set on the handler in Task.__init__ must be the - language-specific prefix followed by the task name.""" - smtp_cfg = SMTPConfig( - mailhost=("smtp.enterprise.test", 25), - fromaddr="alerts@enterprise.test", - toaddrs=["oncall@enterprise.test"], - ) + def test_task_subject_carries_language_prefix(self, smtp_server: SMTPCapture) -> None: + """The delivered email's Subject must be the language-specific prefix + followed by the task name.""" style_cfg = HTMLEmailStyle(language="de") def boom() -> None: @@ -416,31 +399,21 @@ def boom() -> None: name="subject_de", log_path=self._log("subject_task.log"), func=boom, - channels=[EmailChannel(smtp_cfg, style_cfg)], + channels=[EmailChannel(self._cfg(smtp_server), style_cfg)], ) - with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: - with Process([task]) as process: - process.run(parallel=False) - - smtp_instance = mock_smtp_class.return_value - assert smtp_instance.sendmail.call_count == 1 - _fromaddr, _toaddrs, msg = smtp_instance.sendmail.call_args.args + with Process([task]) as process: + process.run(parallel=False) - assert "Subject:" in msg - assert _SUBJECT_MARKERS["de"] + "subject_de" in msg, ( - f"Expected subject containing {_SUBJECT_MARKERS['de']!r} + task name, " - f"got message:\n{msg}" + assert len(smtp_server.messages) == 1 + subject = smtp_server.last()["Subject"] + assert subject == _SUBJECT_MARKERS["de"] + "subject_de", ( + f"Expected subject {_SUBJECT_MARKERS['de']!r} + task name, got {subject!r}" ) - def test_task_without_email_style_uses_defaults(self) -> None: - """When EmailChannel is constructed without a style, the handler uses - the HTMLEmailStyle defaults (modern/neutral/en).""" - smtp_cfg = SMTPConfig( - mailhost=("smtp.test", 25), - fromaddr="a@b.test", - toaddrs=["c@d.test"], - ) + def test_task_without_email_style_uses_defaults(self, smtp_server: SMTPCapture) -> None: + """When EmailChannel is constructed without a style, the delivered body + uses the HTMLEmailStyle defaults (modern/neutral/en).""" def boom() -> None: raise RuntimeError("boom") @@ -449,22 +422,33 @@ def boom() -> None: name="default_style", log_path=self._log("default_style_task.log"), func=boom, - channels=[EmailChannel(smtp_cfg)], + channels=[EmailChannel(self._cfg(smtp_server))], ) - with patch("processes.comms._email.smtplib.SMTP") as mock_smtp_class: - with Process([task]) as process: - process.run(parallel=False) + with Process([task]) as process: + process.run(parallel=False) - smtp_instance = mock_smtp_class.return_value - assert smtp_instance.sendmail.call_count == 1 - _fromaddr, _toaddrs, msg = smtp_instance.sendmail.call_args.args - body = _decode_mime_body(msg) + assert len(smtp_server.messages) == 1 + body = smtp_server.last_html() assert 'class="card"' in body, "Default 'modern' style marker missing" assert "--accent: #2563eb" in body, "Default 'neutral' palette marker missing" assert "Pipeline Failure:" in body, "Default English language marker missing" + def test_successful_run_sends_no_email(self, smtp_server: SMTPCapture) -> None: + """A task that succeeds must deliver zero failure alerts.""" + task = Task( + name="ok", + log_path=self._log("ok_task.log"), + func=lambda: "fine", + channels=[EmailChannel(self._cfg(smtp_server))], + ) + + with Process([task]) as process: + process.run(parallel=False) + + assert smtp_server.messages == [] + def test_two_tasks_sharing_smtp_config_get_independent_subjects(self) -> None: """Two tasks sharing one SMTPConfig must each get a handler with their own subject line — per-task isolation must be preserved.""" diff --git a/tests/test_report_send.py b/tests/test_report_send.py index 8775f59..1f82cc1 100644 --- a/tests/test_report_send.py +++ b/tests/test_report_send.py @@ -2,7 +2,6 @@ from __future__ import annotations -import email as _email_module import json from unittest.mock import MagicMock, patch @@ -20,6 +19,8 @@ from processes.comms._email import _build_report_html from processes.error_data import ErrorData +from .conftest import SMTPCapture + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -66,17 +67,13 @@ def _report(*entries: TaskReportEntry) -> ProcessExecutionReport: return ProcessExecutionReport({e.name: e for e in entries}) -def _smtp() -> SMTPConfig: - return SMTPConfig(mailhost=("localhost", 25), fromaddr="a@b.com", toaddrs=["c@d.com"]) - - -def _decode_mime_body(mime_string: str) -> str: - """Parse a MIME message string and return the decoded text body.""" - msg = _email_module.message_from_string(mime_string) - payload = msg.get_payload(decode=True) - if isinstance(payload, bytes): - return payload.decode("utf-8", errors="replace") - return str(payload or "") +def _cfg(server: SMTPCapture, *, toaddrs: list[str] | None = None) -> SMTPConfig: + """An SMTPConfig pointed at the in-process capture server.""" + return SMTPConfig( + mailhost=(server.host, server.port), + fromaddr="a@b.com", + toaddrs=toaddrs if toaddrs is not None else ["c@d.com"], + ) # --------------------------------------------------------------------------- @@ -271,52 +268,74 @@ def test_palette_css_injected(self) -> None: # --------------------------------------------------------------------------- class TestEmailSendReport: + """End-to-end: EmailChannel.send_report against a real in-process SMTP server. - @patch("smtplib.SMTP") - def test_sendmail_called(self, mock_smtp_cls: MagicMock) -> None: - mock_smtp = MagicMock() - mock_smtp_cls.return_value = mock_smtp + send_report is called directly (not via report.notify), so a transport + failure propagates and fails the test loudly. Each call must deliver exactly + one message; assertions inspect what the server actually received. + """ + def test_delivers_one_html_email(self, smtp_server: SMTPCapture) -> None: report = _report( _entry("a", TaskStatus.SUCCESS), _entry("b", TaskStatus.ERRORED, error=_error()), ) - EmailChannel(_smtp()).send_report(report, errors_only=False) - - mock_smtp.sendmail.assert_called_once() - mock_smtp.quit.assert_called_once() - - @patch("smtplib.SMTP") - def test_mime_type_is_html(self, mock_smtp_cls: MagicMock) -> None: - mock_smtp = MagicMock() - mock_smtp_cls.return_value = mock_smtp - - EmailChannel(_smtp()).send_report( - _report(_entry("a", TaskStatus.SUCCESS)), errors_only=False - ) + EmailChannel(_cfg(smtp_server)).send_report(report, errors_only=False) - _, _, msg_str = mock_smtp.sendmail.call_args[0] - assert 'Content-Type: text/html' in msg_str + assert len(smtp_server.messages) == 1 + msg = smtp_server.last() + assert msg["From"] == "a@b.com" + assert msg.get_content_type() == "text/html" + body = smtp_server.last_html() + assert "a" in body and "b" in body - @patch("smtplib.SMTP") - def test_html_body_contains_task_name(self, mock_smtp_cls: MagicMock) -> None: - mock_smtp = MagicMock() - mock_smtp_cls.return_value = mock_smtp + def test_to_header_lists_all_recipients(self, smtp_server: SMTPCapture) -> None: + cfg = _cfg(smtp_server, toaddrs=["c@d.com", "e@f.com"]) + EmailChannel(cfg).send_report(_report(_entry("a", TaskStatus.SUCCESS)), errors_only=False) - report = _report(_entry("my_task", TaskStatus.ERRORED, error=_error())) - EmailChannel(_smtp()).send_report(report, errors_only=False) + assert len(smtp_server.messages) == 1 + to_header = smtp_server.last()["To"] + assert "c@d.com" in to_header + assert "e@f.com" in to_header - _, _, msg_str = mock_smtp.sendmail.call_args[0] - body = _decode_mime_body(msg_str) - assert "my_task" in body + def test_errors_only_subject_and_excludes_success(self, smtp_server: SMTPCapture) -> None: + report = _report( + _entry("ok_task", TaskStatus.SUCCESS), + _entry("bad_task", TaskStatus.ERRORED, error=_error()), + ) + EmailChannel(_cfg(smtp_server)).send_report(report, errors_only=True) + + assert len(smtp_server.messages) == 1 + subject = smtp_server.last()["Subject"] + assert "Failed" in subject or "failed" in subject.lower() + body = smtp_server.last_html() + assert "bad_task" in body + assert "ok_task" not in body + + def test_show_traceback_false_omits_traceback_in_received_body( + self, smtp_server: SMTPCapture + ) -> None: + report = _report( + _entry("b", TaskStatus.ERRORED, error=_error(traceback_str="UNIQUE_TRACE_TEXT")) + ) + channel = EmailChannel( + _cfg(smtp_server), content=ReportContent(show_traceback=False, show_traced_vars=False) + ) + channel.send_report(report, errors_only=False) - @patch("smtplib.SMTP") - def test_errors_only_uses_errors_subject(self, mock_smtp_cls: MagicMock) -> None: - mock_smtp = MagicMock() - mock_smtp_cls.return_value = mock_smtp + assert len(smtp_server.messages) == 1 + assert "UNIQUE_TRACE_TEXT" not in smtp_server.last_html() - report = _report(_entry("b", TaskStatus.ERRORED, error=_error())) - EmailChannel(_smtp()).send_report(report, errors_only=True) + def test_show_traceback_true_includes_traceback_in_received_body( + self, smtp_server: SMTPCapture + ) -> None: + report = _report( + _entry("b", TaskStatus.ERRORED, error=_error(traceback_str="UNIQUE_TRACE_TEXT")) + ) + channel = EmailChannel( + _cfg(smtp_server), content=ReportContent(show_traceback=True, show_traced_vars=False) + ) + channel.send_report(report, errors_only=False) - _, _, msg_str = mock_smtp.sendmail.call_args[0] - assert "Failed" in msg_str or "failed" in msg_str.lower() + assert len(smtp_server.messages) == 1 + assert "UNIQUE_TRACE_TEXT" in smtp_server.last_html() diff --git a/uv.lock b/uv.lock index aa6a3dc..92fe5b3 100644 --- a/uv.lock +++ b/uv.lock @@ -2,6 +2,19 @@ version = 1 revision = 3 requires-python = ">=3.11" +[[package]] +name = "aiosmtpd" +version = "1.4.6" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "atpublic" }, + { name = "attrs" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c4/ca/b2b7cc880403ef24be77383edaadfcf0098f5d7b9ddbf3e2c17ef0a6af0d/aiosmtpd-1.4.6.tar.gz", hash = "sha256:5a811826e1a5a06c25ebc3e6c4a704613eb9a1bcf6b78428fbe865f4f6c9a4b8", size = 152775, upload-time = "2024-05-18T11:37:50.029Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ec/39/d401756df60a8344848477d54fdf4ce0f50531f6149f3b8eaae9c06ae3dc/aiosmtpd-1.4.6-py3-none-any.whl", hash = "sha256:72c99179ba5aa9ae0abbda6994668239b64a5ce054471955fe75f581d2592475", size = 154263, upload-time = "2024-05-18T11:37:47.877Z" }, +] + [[package]] name = "argcomplete" version = "3.6.3" @@ -11,6 +24,24 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/74/f5/9373290775639cb67a2fce7f629a1c240dce9f12fe927bc32b2736e16dfc/argcomplete-3.6.3-py3-none-any.whl", hash = "sha256:f5007b3a600ccac5d25bbce33089211dfd49eab4a7718da3f10e3082525a92ce", size = 43846, upload-time = "2025-10-20T03:33:33.021Z" }, ] +[[package]] +name = "atpublic" +version = "7.0.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/a9/05/e2e131a0debaf0f01b8a1b586f5f11713f6affc3e711b406f15f11eafc92/atpublic-7.0.0.tar.gz", hash = "sha256:466ef10d0c8bbd14fd02a5fbd5a8b6af6a846373d91106d3a07c16d72d96b63e", size = 17801, upload-time = "2025-11-29T05:56:45.45Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/96/c0/271f3e1e3502a8decb8ee5c680dbed2d8dc2cd504f5e20f7ed491d5f37e1/atpublic-7.0.0-py3-none-any.whl", hash = "sha256:6702bd9e7245eb4e8220a3e222afcef7f87412154732271ee7deee4433b72b4b", size = 6421, upload-time = "2025-11-29T05:56:44.604Z" }, +] + +[[package]] +name = "attrs" +version = "26.1.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/9a/8e/82a0fe20a541c03148528be8cac2408564a6c9a0cc7e9171802bc1d26985/attrs-26.1.0.tar.gz", hash = "sha256:d03ceb89cb322a8fd706d4fb91940737b6642aa36998fe130a9bc96c985eff32", size = 952055, upload-time = "2026-03-19T14:22:25.026Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/64/b4/17d4b0b2a2dc85a6df63d1157e028ed19f90d4cd97c36717afef2bc2f395/attrs-26.1.0-py3-none-any.whl", hash = "sha256:c647aa4a12dfbad9333ca4e71fe62ddc36f4e63b2d260a37a8b83d2f043ac309", size = 67548, upload-time = "2026-03-19T14:22:23.645Z" }, +] + [[package]] name = "babel" version = "2.17.0" @@ -818,6 +849,7 @@ source = { editable = "." } [package.dev-dependencies] dev = [ + { name = "aiosmtpd" }, { name = "commitizen" }, { name = "mkdocs" }, { name = "mkdocs-material", extra = ["imaging"] }, @@ -831,6 +863,7 @@ dev = [ [package.metadata.requires-dev] dev = [ + { name = "aiosmtpd", specifier = ">=1.4.6" }, { name = "commitizen", specifier = ">=4.11.6" }, { name = "mkdocs", specifier = ">=1.6.1" }, { name = "mkdocs-material", extras = ["imaging"], specifier = ">=9.7.1" }, From b7c5a0c5d766084cf8d2d1ac5f988b8744780c7b Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Fri, 19 Jun 2026 18:07:27 -0400 Subject: [PATCH 12/14] test: add manual report-notify SMTP inspection script A mixed 6-task DAG (independent + dependent tasks, 3 failures, 1 cascade-skip) whose ProcessExecutionReport is delivered by email to the same maildev server (127.0.0.1:1025) the other manual scripts use. Exercises, in one run: - with / without traced variables: fetch_orders fails from a frame rich in locals (default filter), noop_validate fails with no locals (empty section) - with / without a custom traceback frame filter: decode_payload uses traced_vars_frame_filter="json" so its traced vars come from the stdlib json frame, vs the default outermost-user-frame selection elsewhere - report delivery via SMTP: notify (full, show_traced_vars=True) and notify_errors (errors only, show_traced_vars=False) for side-by-side compare --- tests/manual_tests/manual_report_notify.py | 310 +++++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100644 tests/manual_tests/manual_report_notify.py diff --git a/tests/manual_tests/manual_report_notify.py b/tests/manual_tests/manual_report_notify.py new file mode 100644 index 0000000..3201f31 --- /dev/null +++ b/tests/manual_tests/manual_report_notify.py @@ -0,0 +1,310 @@ +"""Manual inspection: a mixed DAG whose ProcessExecutionReport is delivered by +email via SMTP, exercising every traced-variables configuration in one run. + +Unlike the other manual scripts (which only send per-task *failure alerts*), +this one is about the **report** path — ``ProcessExecutionReport.notify`` / +``notify_errors`` rendering the whole run as a single HTML email and sending it +to the same maildev server. + +Run by hand and eyeball the result in maildev: + + python tests/manual_tests/manual_report_notify.py + +What this exercises +------------------- +Tasks (independent and dependent): + +* ``load_config`` — independent, **succeeds** (produces config). +* ``transform`` — **dependent** on ``load_config`` (consumes its result), + **succeeds** — a downstream task that *does* run. +* ``fetch_orders`` — independent, **fails** from a frame holding several + rich local variables → **WITH traced variables**, using + the **default** frame filter. +* ``decode_payload`` — independent, **fails** inside the stdlib ``json`` + parser, with a **CUSTOM** ``traced_vars_frame_filter`` + (``"json"``) so the traced variables come from json's + internal frame instead of the task frame. +* ``noop_validate`` — independent, **fails** immediately with no locals bound + → **WITHOUT traced variables** (empty section). +* ``aggregate`` — **dependent** on ``fetch_orders``, therefore + **cascade-skipped** (never runs) → appears under + "Downstream Impact". + +Report delivery (the point of the script): + +* ``report.notify(full)`` — full report, ``show_traced_vars=True`` + → the traced-variables sections are present + (rich user locals for ``fetch_orders``, + json internals for ``decode_payload``, + empty for ``noop_validate``). +* ``report.notify_errors(brief)`` — errored entries only, ``show_traced_vars= + False`` → the **same** failures rendered + **WITHOUT** the traced-variables sections. + +So a single run demonstrates, side by side: with/without traced variables (both +per task and via the report content flag) and with/without a custom traceback +frame filter. + +Prerequisites +------------- +* maildev running on ``127.0.0.1:1025`` (web UI on 1080). +* The script connects to ``127.0.0.1`` (not ``localhost``) on purpose: on + Windows, ``localhost`` often resolves to IPv6 ``::1`` first while maildev + binds IPv4 only, producing ``WinError 10061``. + +Inspect +------- +* The console output for execution order and the per-task outcome table. +* The maildev web UI at http://localhost:1080. Expected messages: + - 3 per-task failure alerts (to ``task-alerts@inspect.test``) + - 1 full report (to ``report-full@inspect.test``) + - 1 errors-only report (to ``report-errors@inspect.test``) + Compare ``report-full`` vs ``report-errors`` to see the traced-variables + sections appear and disappear, and confirm ``decode_payload``'s traced + variables differ from ``fetch_orders``' (custom vs default frame filter). +""" + +from __future__ import annotations + +import json +import os +import sys +import traceback +from typing import Any + +# Make the in-tree package importable when the script is run directly. +_PROJECT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +if _PROJECT_ROOT not in sys.path: + sys.path.insert(0, _PROJECT_ROOT) + +from processes import ( # noqa: E402 + EmailChannel, + HTMLEmailStyle, + Process, + ProcessExecutionReport, + ReportContent, + SMTPConfig, + Task, + TaskDependency, +) + +# --------------------------------------------------------------------------- # +# Constants # +# --------------------------------------------------------------------------- # + +SMTP_HOST = "127.0.0.1" +SMTP_PORT = 1025 +WEB_PORT = 1080 +FROM_ADDR = "report-canary@enterprise.test" + +TASK_ALERTS_TO = "task-alerts@inspect.test" +REPORT_FULL_TO = "report-full@inspect.test" +REPORT_ERRORS_TO = "report-errors@inspect.test" + + +def _smtp(toaddr: str) -> SMTPConfig: + return SMTPConfig( + mailhost=(SMTP_HOST, SMTP_PORT), + fromaddr=FROM_ADDR, + toaddrs=[toaddr], + timeout=5, + ) + + +# --------------------------------------------------------------------------- # +# Task functions # +# --------------------------------------------------------------------------- # + + +def load_config() -> dict[str, Any]: + """Independent root task — succeeds and feeds ``transform``.""" + print(" [load_config] loading configuration ...") + return {"region": "us-east-1", "batch_size": 500} + + +def transform(config: dict[str, Any]) -> str: + """Dependent task — runs because its upstream succeeded.""" + print(f" [transform] transforming with config={config} ...") + return f"transformed::{config['region']}" + + +def fetch_orders(source: str, batch_id: int, *, region: str) -> list[int]: + """Fails from a frame rich in local variables. + + With the default frame filter, *these* user locals (``endpoint``, + ``attempt``, ``page_token``, ``collected``) are the ones traced. + """ + endpoint = f"https://api.internal/{region}/orders" + attempt = 3 + page_token = "p_98f3a2c" + collected: list[int] = [101, 102, 103] + print(f" [fetch_orders] source={source!r} batch_id={batch_id} region={region!r}") + raise ConnectionError( + f"upstream {endpoint!r} refused the connection after {attempt} attempts " + f"(page_token={page_token}, collected so far={collected})" + ) + + +def decode_payload(raw: str) -> Any: + """Fails inside the stdlib ``json`` parser. + + Paired with ``traced_vars_frame_filter='json'`` the traced variables are + captured from json's internal frame, not from this task frame. + """ + print(f" [decode_payload] decoding {raw!r} ...") + return json.loads(raw) # malformed input -> JSONDecodeError raised inside json/ + + +def noop_validate() -> None: + """Fails immediately with no local variables bound — the traced-variables + section is empty (the 'without traced variables' case).""" + print(" [noop_validate] validating ...") + raise ValueError("validation rule 'non_empty' violated") + + +def aggregate(*_args: Any, **_kwargs: Any) -> str: + """Downstream of ``fetch_orders`` — must be cascade-skipped, never run.""" + print(" [aggregate] this should never run — FAILED upstream") + return "aggregated" + + +# --------------------------------------------------------------------------- # +# Build the DAG # +# --------------------------------------------------------------------------- # + + +def _log_path(logs_dir: str, name: str) -> str: + return os.path.join(logs_dir, f"{name}.log") + + +def build_tasks(logs_dir: str) -> list[Task]: + """A 6-task DAG: 2 independent successes/deps, 3 independent failures with + distinct traced-vars configs, and 1 cascade-skipped dependent.""" + task_smtp = _smtp(TASK_ALERTS_TO) + default_style = HTMLEmailStyle() # no custom frame filter + json_filter_style = HTMLEmailStyle(traced_vars_frame_filter="json") # custom filter + dep = TaskDependency + + return [ + # Independent success → feeds a dependent task. + Task( + name="load_config", + log_path=_log_path(logs_dir, "load_config"), + func=load_config, + ), + # Dependent success — consumes load_config's result. + Task( + name="transform", + log_path=_log_path(logs_dir, "transform"), + func=transform, + dependencies=[dep("load_config", use_result_as_additional_args=True)], + ), + # Independent failure WITH rich traced variables, DEFAULT frame filter. + Task( + name="fetch_orders", + log_path=_log_path(logs_dir, "fetch_orders"), + func=fetch_orders, + args=("orders_feed", 4242), + kwargs={"region": "us-east-1"}, + channels=[EmailChannel(task_smtp, default_style)], + ), + # Independent failure with a CUSTOM frame filter ('json'). + Task( + name="decode_payload", + log_path=_log_path(logs_dir, "decode_payload"), + func=decode_payload, + args=('{"id": 1, "ok"',), # malformed JSON + channels=[EmailChannel(task_smtp, json_filter_style)], + ), + # Independent failure WITHOUT traced variables (no locals), default filter. + Task( + name="noop_validate", + log_path=_log_path(logs_dir, "noop_validate"), + func=noop_validate, + channels=[EmailChannel(task_smtp, default_style)], + ), + # Dependent on a failing task → cascade-skipped, never runs. + Task( + name="aggregate", + log_path=_log_path(logs_dir, "aggregate"), + func=aggregate, + dependencies=[dep("fetch_orders")], + ), + ] + + +# --------------------------------------------------------------------------- # +# Report delivery # +# --------------------------------------------------------------------------- # + + +def deliver_reports(report: ProcessExecutionReport) -> None: + """Send the report via SMTP twice: a full report with traced variables, and + an errors-only report without them.""" + full = EmailChannel( + _smtp(REPORT_FULL_TO), + HTMLEmailStyle(style="modern", palette="neutral", language="en"), + content=ReportContent(show_traceback=True, show_traced_vars=True), + ) + brief = EmailChannel( + _smtp(REPORT_ERRORS_TO), + HTMLEmailStyle(style="compact", palette="slate", language="en"), + content=ReportContent(show_traceback=True, show_traced_vars=False), + ) + + print("\ndelivering reports via SMTP ...") + print(f" notify -> {REPORT_FULL_TO} (full, with traced variables)") + report.notify(full) + print(f" notify_errors -> {REPORT_ERRORS_TO} (errors only, without traced variables)") + report.notify_errors(brief) + + +# --------------------------------------------------------------------------- # +# Entry point # +# --------------------------------------------------------------------------- # + + +def main() -> int: + here = os.path.dirname(os.path.abspath(__file__)) + logs_dir = os.path.join(here, "logs") + os.makedirs(logs_dir, exist_ok=True) + + cleared = 0 + for name in os.listdir(logs_dir): + if name.endswith(".log"): + os.remove(os.path.join(logs_dir, name)) + cleared += 1 + print(f"logs dir: {logs_dir} (cleared {cleared} stale .log file(s))") + print(f"from: {FROM_ADDR}") + print(f"smtp: {SMTP_HOST}:{SMTP_PORT} web: http://localhost:{WEB_PORT}") + print("=" * 72) + + tasks = build_tasks(logs_dir) + print(f"tasks: {len(tasks)} (2 independent + dependent successes, 3 failures, 1 skipped)") + + try: + with Process(tasks) as process: + report = process.run(parallel=False) + # Deliver while the process context is open so loggers/handlers + # are still alive for the per-task alerts already emitted. + print("\noutcome:") + for name, entry in report.entries.items(): + print(f" {entry.status.value:8s} {name}") + deliver_reports(report) + except Exception: + print("Process raised an unexpected exception:") + traceback.print_exc() + return 2 + + print("=" * 72) + print(f"Expected in maildev (http://localhost:{WEB_PORT}):") + print(f" 3 per-task failure alerts -> {TASK_ALERTS_TO}") + print(f" 1 full report -> {REPORT_FULL_TO}") + print(f" 1 errors-only report -> {REPORT_ERRORS_TO}") + print("Compare the two reports: traced-variables sections present vs absent,") + print("and decode_payload's traced vars (custom 'json' filter) vs fetch_orders'.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 033d2217bda8a7dc02a2b8e3fdff0daa1a7984d0 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Fri, 19 Jun 2026 19:41:06 -0400 Subject: [PATCH 13/14] feat: replace notify_errors with notify(only_errors=...) and add task filter - Remove ProcessExecutionReport.notify_errors; notify() gains only_errors:bool (errors-only payload, was notify_errors) so there is one entry point - Add notify(tasks=[...]): restrict the report to the named tasks, compared case-insensitively; None includes all, [] includes none; composes with only_errors. Implemented via a filtered sub-report (_for_tasks) - send_report's errors_only param is unchanged, so the errors-only subject / header behaviour is preserved - Update dispatch tests (notify_errors -> notify(only_errors=True)) and add coverage for the task filter; update the manual report-notify script --- src/processes/comms/base.py | 4 +- src/processes/execution_report.py | 51 +++++++--------- tests/manual_tests/manual_report_notify.py | 30 ++++----- tests/test_report_notify_dispatch.py | 71 ++++++++++++++++++++-- 4 files changed, 104 insertions(+), 52 deletions(-) diff --git a/src/processes/comms/base.py b/src/processes/comms/base.py index c5777c2..b3302d7 100644 --- a/src/processes/comms/base.py +++ b/src/processes/comms/base.py @@ -42,8 +42,8 @@ class ReportChannel(ABC): Unlike ``NotificationChannel`` (which builds a streaming ``logging.Handler`` for a single ``Task``), a report channel sends a complete report **once**, - after the run. ``ProcessExecutionReport.notify`` / ``notify_errors`` iterate - the channels they are given and call ``send_report`` on each. + after the run. ``ProcessExecutionReport.notify`` iterates the channels it is + given and calls ``send_report`` on each. """ @abstractmethod diff --git a/src/processes/execution_report.py b/src/processes/execution_report.py index 07a3ab0..14e2d62 100644 --- a/src/processes/execution_report.py +++ b/src/processes/execution_report.py @@ -169,9 +169,13 @@ def to_json(self, *, indent: int | None = None, **dumps_kwargs: Any) -> str: return json.dumps(self, default=_json_default, indent=indent, **dumps_kwargs) def notify( - self, *channels: ReportChannel, show_warnings: bool = True + self, + *channels: ReportChannel, + only_errors: bool = False, + tasks: list[str] | None = None, + show_warnings: bool = True, ) -> None: - """Deliver the full report through each channel, in order. + """Deliver the report through each channel, in order. Each channel renders and sends the report itself (email, webhook, ...). What detail is included is configured per channel (see ``ReportContent``). @@ -183,12 +187,20 @@ def notify( ---------- *channels : ReportChannel Channels to deliver the report to. No-op if none are given. + only_errors : bool + When ``True``, each channel restricts the payload to tasks whose + status is ``ERRORED`` (see :attr:`errored`). Defaults to ``False``. + tasks : list[str], optional + Restrict the report to these task names, compared case-insensitively. + ``None`` (default) includes every task; an empty list includes none. + Combines with ``only_errors`` — both filters apply. show_warnings : bool Emit a ``UserWarning`` when a channel fails. Defaults to ``True``. """ + report = self if tasks is None else self._for_tasks(tasks) for channel in channels: try: - channel.send_report(self, errors_only=False) + channel.send_report(report, errors_only=only_errors) except Exception as exc: if show_warnings: warnings.warn( @@ -196,30 +208,9 @@ def notify( stacklevel=2, ) - def notify_errors( - self, *channels: ReportChannel, show_warnings: bool = True - ) -> None: - """Deliver only the ``ERRORED`` entries through each channel, in order. - - Same as :meth:`notify`, but each channel restricts the payload to tasks - whose status is ``ERRORED`` (see :attr:`errored`). - If a channel raises, the exception is caught so the remaining channels - still receive the report; a ``UserWarning`` is emitted when - ``show_warnings`` is ``True``. - - Parameters - ---------- - *channels : ReportChannel - Channels to deliver the errored report to. No-op if none are given. - show_warnings : bool - Emit a ``UserWarning`` when a channel fails. Defaults to ``True``. - """ - for channel in channels: - try: - channel.send_report(self, errors_only=True) - except Exception as exc: - if show_warnings: - warnings.warn( - f"{type(channel).__name__} failed to send report: {exc}", - stacklevel=2, - ) + def _for_tasks(self, tasks: list[str]) -> ProcessExecutionReport: + """A copy of this report restricted to ``tasks`` (case-insensitive names).""" + wanted = {name.lower() for name in tasks} + return ProcessExecutionReport( + {name: entry for name, entry in self.entries.items() if name.lower() in wanted} + ) diff --git a/tests/manual_tests/manual_report_notify.py b/tests/manual_tests/manual_report_notify.py index 3201f31..1eb0340 100644 --- a/tests/manual_tests/manual_report_notify.py +++ b/tests/manual_tests/manual_report_notify.py @@ -2,9 +2,9 @@ email via SMTP, exercising every traced-variables configuration in one run. Unlike the other manual scripts (which only send per-task *failure alerts*), -this one is about the **report** path — ``ProcessExecutionReport.notify`` / -``notify_errors`` rendering the whole run as a single HTML email and sending it -to the same maildev server. +this one is about the **report** path — ``ProcessExecutionReport.notify`` +rendering the whole run as a single HTML email and sending it to the same +maildev server. Run by hand and eyeball the result in maildev: @@ -32,14 +32,16 @@ Report delivery (the point of the script): -* ``report.notify(full)`` — full report, ``show_traced_vars=True`` - → the traced-variables sections are present - (rich user locals for ``fetch_orders``, - json internals for ``decode_payload``, - empty for ``noop_validate``). -* ``report.notify_errors(brief)`` — errored entries only, ``show_traced_vars= - False`` → the **same** failures rendered - **WITHOUT** the traced-variables sections. +* ``report.notify(full)`` — full report, ``show_traced_vars= + True`` → the traced-variables sections are + present (rich user locals for + ``fetch_orders``, json internals for + ``decode_payload``, empty for + ``noop_validate``). +* ``report.notify(brief, only_errors=True)`` — errored entries only, + ``show_traced_vars=False`` → the **same** + failures rendered **WITHOUT** the + traced-variables sections. So a single run demonstrates, side by side: with/without traced variables (both per task and via the report content flag) and with/without a custom traceback @@ -253,10 +255,10 @@ def deliver_reports(report: ProcessExecutionReport) -> None: ) print("\ndelivering reports via SMTP ...") - print(f" notify -> {REPORT_FULL_TO} (full, with traced variables)") + print(f" notify -> {REPORT_FULL_TO} (full, with traced vars)") report.notify(full) - print(f" notify_errors -> {REPORT_ERRORS_TO} (errors only, without traced variables)") - report.notify_errors(brief) + print(f" notify(only_errors=True) -> {REPORT_ERRORS_TO} (errors only, no traced vars)") + report.notify(brief, only_errors=True) # --------------------------------------------------------------------------- # diff --git a/tests/test_report_notify_dispatch.py b/tests/test_report_notify_dispatch.py index 894b7c6..f1fd1fd 100644 --- a/tests/test_report_notify_dispatch.py +++ b/tests/test_report_notify_dispatch.py @@ -1,4 +1,4 @@ -"""Report notification dispatch: notify/notify_errors wiring and show_warnings behaviour.""" +"""Report notification dispatch: notify wiring, filtering and show_warnings behaviour.""" from __future__ import annotations @@ -11,11 +11,31 @@ ReportChannel, ReportContent, SMTPConfig, + TaskReportEntry, + TaskStatus, WebhookChannel, WebhookConfig, ) +def _report_with(*names: str) -> ProcessExecutionReport: + """A report carrying one SUCCESS entry per given task name.""" + return ProcessExecutionReport( + { + name: TaskReportEntry( + name=name, + function="f", + args=(), + kwargs={}, + status=TaskStatus.SUCCESS, + elapsed_seconds=0.0, + attempts=1, + ) + for name in names + } + ) + + class _SpyChannel(ReportChannel): def __init__(self) -> None: self.calls: list[tuple[Any, bool]] = [] @@ -41,16 +61,16 @@ def test_notify_dispatches_to_each_channel_in_order() -> None: assert b.calls == [(report, False)] -def test_notify_errors_sets_errors_only() -> None: +def test_notify_only_errors_sets_errors_only() -> None: report = ProcessExecutionReport() spy = _SpyChannel() - report.notify_errors(spy) + report.notify(spy, only_errors=True) assert spy.calls == [(report, True)] def test_notify_with_no_channels_is_noop() -> None: ProcessExecutionReport().notify() - ProcessExecutionReport().notify_errors() + ProcessExecutionReport().notify(only_errors=True) def test_notify_continues_after_channel_failure() -> None: @@ -78,16 +98,55 @@ def test_notify_silent_when_show_warnings_false() -> None: assert caught == [] -def test_notify_errors_continues_and_warns() -> None: +def test_notify_only_errors_continues_and_warns() -> None: report = ProcessExecutionReport() spy = _SpyChannel() with warnings.catch_warnings(record=True) as caught: warnings.simplefilter("always") - report.notify_errors(_BrokenChannel(), spy) + report.notify(_BrokenChannel(), spy, only_errors=True) assert len(caught) == 1 assert spy.calls == [(report, True)] +def test_notify_tasks_filters_by_name() -> None: + report = _report_with("alpha", "beta", "gamma") + spy = _SpyChannel() + report.notify(spy, tasks=["alpha", "gamma"]) + (sent_report, errors_only) = spy.calls[0] + assert set(sent_report.entries) == {"alpha", "gamma"} + assert errors_only is False + + +def test_notify_tasks_is_case_insensitive() -> None: + report = _report_with("Fetch_Orders", "Decode_Payload") + spy = _SpyChannel() + report.notify(spy, tasks=["fetch_orders"]) + assert set(spy.calls[0][0].entries) == {"Fetch_Orders"} + + +def test_notify_tasks_empty_list_sends_empty_report() -> None: + report = _report_with("alpha", "beta") + spy = _SpyChannel() + report.notify(spy, tasks=[]) + assert set(spy.calls[0][0].entries) == set() + + +def test_notify_tasks_none_includes_every_task() -> None: + report = _report_with("alpha", "beta") + spy = _SpyChannel() + report.notify(spy) + assert spy.calls[0][0] is report + + +def test_notify_tasks_combines_with_only_errors() -> None: + report = _report_with("alpha", "beta") + spy = _SpyChannel() + report.notify(spy, only_errors=True, tasks=["alpha"]) + (sent_report, errors_only) = spy.calls[0] + assert set(sent_report.entries) == {"alpha"} + assert errors_only is True + + def test_report_content_defaults_and_per_channel_override() -> None: assert ReportContent() == ReportContent(show_traceback=True, show_traced_vars=True) From f334964876814f0cf2a6f275da5bb4bd4affd980 Mon Sep 17 00:00:00 2001 From: Oliver Mohr Date: Fri, 19 Jun 2026 21:30:56 -0400 Subject: [PATCH 14/14] style: apply ruff format to comms/_email.py and test_report_send.py --- src/processes/comms/_email.py | 14 ++++---------- tests/test_report_send.py | 5 ++++- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/processes/comms/_email.py b/src/processes/comms/_email.py index 9608ebd..6b06c3e 100644 --- a/src/processes/comms/_email.py +++ b/src/processes/comms/_email.py @@ -329,7 +329,7 @@ def _build_task_section_html( return ( f"\n" - f' ' + f" " f'{html.escape(entry.name)}' f'{html.escape(status_label)}' f"\n" @@ -389,16 +389,12 @@ def _build_report_html( "lang_report_title_prefix": html.escape( lang.get("lang_report_title_prefix", "Process Report") ), - "lang_report_summary_title": html.escape( - lang.get("lang_report_summary_title", "Summary") - ), + "lang_report_summary_title": html.escape(lang.get("lang_report_summary_title", "Summary")), "lang_report_success_label": html.escape( lang.get("lang_report_success_label", "Successes") ), "lang_report_error_label": html.escape(lang.get("lang_report_error_label", "Errors")), - "lang_report_skipped_label": html.escape( - lang.get("lang_report_skipped_label", "Skipped") - ), + "lang_report_skipped_label": html.escape(lang.get("lang_report_skipped_label", "Skipped")), "summary_successes": str(len(report.successes)), "summary_errors": str(len(report.errored)), "summary_skipped": str(len(report.skipped)), @@ -435,9 +431,7 @@ def send_report_email( uses ``lang_report_email_subject_errors``. """ lang = _load_language_strings(style.language) - subject_key = ( - "lang_report_email_subject_errors" if errors_only else "lang_report_email_subject" - ) + subject_key = "lang_report_email_subject_errors" if errors_only else "lang_report_email_subject" subject = lang.get(subject_key, "Process Execution Report") html_body = _build_report_html(report, style, content, errors_only=errors_only) _SMTPTransport(smtp_config).send(subject, html_body) diff --git a/tests/test_report_send.py b/tests/test_report_send.py index 1f82cc1..193919e 100644 --- a/tests/test_report_send.py +++ b/tests/test_report_send.py @@ -25,6 +25,7 @@ # Helpers # --------------------------------------------------------------------------- + def _entry( name: str, status: TaskStatus, @@ -80,8 +81,8 @@ def _cfg(server: SMTPCapture, *, toaddrs: list[str] | None = None) -> SMTPConfig # Webhook tests # --------------------------------------------------------------------------- -class TestWebhookSendReport: +class TestWebhookSendReport: @patch("urllib.request.urlopen") def test_posts_json_with_all_entries(self, mock_urlopen: MagicMock) -> None: mock_urlopen.return_value.__enter__ = lambda s: s @@ -200,6 +201,7 @@ def test_hmac_signature_header_set(self, mock_urlopen: MagicMock) -> None: # Report HTML renderer tests (pure, no I/O mocks needed) # --------------------------------------------------------------------------- + class TestBuildReportHtml: def _style(self) -> HTMLEmailStyle: return HTMLEmailStyle() @@ -267,6 +269,7 @@ def test_palette_css_injected(self) -> None: # Email send tests (verify SMTP transport) # --------------------------------------------------------------------------- + class TestEmailSendReport: """End-to-end: EmailChannel.send_report against a real in-process SMTP server.