feat: report notifications (email + webhook) and comms package refactor - #5
Merged
Merged
Conversation
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.
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).
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.
- 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(): <details open> 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
- 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
- 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
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.
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).
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
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the ability for a finished
ProcessExecutionReportto notify itselfthrough one or more channels (HTML email over SMTP, or JSON webhook), and
reorganizes all communication code into a dedicated
comms/package along theway. Runtime dependencies stay at zero (
aiosmtpdis a test-only devdependency).
What's new (public API)
ProcessExecutionReport.notify(*channels, only_errors=False, tasks=None, show_warnings=True)only_errors=Truerestricts the payload toERROREDtasks.tasks=[...]restricts to the named tasks, compared case-insensitively(
None= all,[]= none); composes withonly_errors.UserWarningunlessshow_warnings=False.ReportChannel(abstract) — delivers a complete report once, after the run.EmailChannel/WebhookChannelare now dual-capability: aNotificationChannel(streaming per-task failure alert) and aReportChannel(one-shot report). The same instance can serve both roles.ReportContent(show_traceback=True, show_traced_vars=True)— per-channel,shareable selection of how much detail a report notification includes.
ProcessExecutionReport.to_json()— lossless JSON of the whole report.Architecture refactor (behavior-preserving)
The communication layer was split out of the flat top-level namespace into a
comms/package, with the domain depending only on abstract ports:comms/base.py—NotificationChannel,ReportChannel,ReportContent(ports).comms/channels.py—EmailChannel,WebhookChannel,_FileChannel(adapters).comms/_email.py/comms/_webhook.py— renderers + transports + handlers.comms/_logfile.py,comms/_error_context.py,comms/email_config.py,comms/webhook_config.py,comms/themes/.Supporting changes:
TaskStatus/TaskResult/TaskDependencymoved to a leaftask_types.py, andErrorDatato a leaferror_data.py, so the comms renderers import them directly instead of theprevious status-
.valuestring workaround._SMTPTransportand one_WebhookTransportare shared by the streaming handler and the one-shot report sender.
Task.close_handlers();Processno longerreaches into
task.logger.handlersin two places.Full rationale and the staged plan are in
arquitectura.md(incl. an"as-built" section noting deliberate deviations; Phase 3 — splitting
render/transport into separate files — was intentionally skipped).
Testing
(
aiosmtpd,tests/conftest.py::smtp_server) and assert on what is actuallyreceived — headers, MIME, decoded HTML body — instead of mocking
smtplib.Streaming tests assert an exact received count (the streaming
emitswallows errors, so a loose check would pass on a silent no-send).
tests of the template engine.
only_errors,tasks), webhook reportpayload, content flags, and a manual inspection script
(
tests/manual_tests/manual_report_notify.py).Verification
uv run pytest→ 141 passeduv run mypy→ clean (21 source files)uv run ruff check .→ cleanuv buildwheel verified to shipcomms/themes/assets.Notes
aiosmtpdis added under[dependency-groups] devonly; it is not part of thepublished wheel's
Requires-Dist.report-notifications-design.md/report-notifications-implementation.mdarekept as point-in-time design records; the authoritative API is the docstrings
and tests.
Type of change
feat— new featurefix— bug fixrefactor— no behavior changedocs— documentation onlytest— tests onlychore/ci/build