SMTP Email Alerter integration + make generic Alerter steps #3379
Conversation
This commit adds a new SMTP Email Alerter integration, allowing users to send email notifications from within ZenML pipelines and steps. Features: - Support for both plain text and HTML-formatted emails - Documentation with detailed examples and setup instructions - Support for customizable email subjects and recipients - Predefined step for easy integration into pipelines 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
strickvl
added
enhancement
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces SMTP email alerting capabilities into the ZenML framework. Documentation is updated to explain the new SMTP Email Alerter, its configuration, usage, and security advice. In the codebase, exception handling in failure hooks has been streamlined using standard traceback formatting and a markdown-to-HTML conversion helper was added for improved email message formatting. Additionally, a new constant for SMTP email integration is defined and several new components—including integrations, alerters, flavors, hooks, and post steps—were implemented to support the new email notification functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Step as Pipeline Step
participant Hook as SMTP Email Alerter Failure Hook
participant Alerter as SMTPEmailAlerter
participant SMTP as SMTP Server
Step->>Hook: Trigger failure event
Hook->>Alerter: Build email payload & parameters
Alerter->>SMTP: Connect (with TLS/auth) and send email
SMTP-->>Alerter: Acknowledgement of sent email
Alerter-->>Hook: Return success/failure status
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py
Outdated
Show resolved
Hide resolved
- Replace warning log with a raised NotImplementedError - Update method docstring to reflect the new error handling approach - Clarify that interactive approvals are not supported for email alerters
- Create dedicated hooks for SMTP email alerters to provide better email formatting - Implement proper HTML formatting for both success and failure emails - Add markdown-style formatting support (backticks, asterisks, etc.) - Improve traceback display with monospaced formatting - Update documentation to explain the specialized hooks - Fix circular import issues in the integration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Use standard Python traceback instead of Rich for more concise error reporting - Update docs to include more details about email-specific hooks' benefits - Fix consistency in traceback handling throughout the code 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove all Rich traceback formatting code from hooks - Replace with Python's native traceback module for cleaner, more readable error messages - Update both standard and email-specific hooks to use the same approach - Remove unnecessary imports 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Clean pipeline and step names in email subjects to remove Markdown symbols - Ensure email subjects display cleanly without formatting artifacts - Fix issue where * or ` characters could appear in email subjects 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add proper type annotations for process_markdown_line function - Add List type annotation for formatted_parts - Clean up imports with explicit typing imports 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
strickvl
changed the title
- Add detection of SMTP email alerter type - Replace Rich traceback with standard Python traceback - Create dedicated HTML email format with proper styling - Add comprehensive markdown-to-HTML converter for ChatGPT responses - Handle code blocks with language annotations - Convert numbered lists to bullet lists for better email display - Add proper type annotations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
src/zenml/integrations/smtp_email/steps/smtp_email_alerter_post_step.py (1)
26-71: Well-structured email alerter step implementation.The step function is properly implemented with clear parameters, good error handling, and comprehensive documentation. It correctly integrates with the alerter system and provides flexibility for customizing email content.
I have a few suggestions for improvement:
Consider adding basic validation for the message parameter to handle empty strings:
@step def smtp_email_alerter_post_step( message: str, recipient_email: Optional[str] = None, subject: Optional[str] = None, include_pipeline_info: bool = True, ) -> None: """Step that sends an email alert using the active alerter. Args: message: Message to be included in the email. recipient_email: Optional recipient email address. If not provided, the email will be sent to the default recipient configured in the alerter. subject: Optional custom subject for the email. include_pipeline_info: If True, includes pipeline and step information in the email. Raises: RuntimeError: If no alerter is configured in the active stack. + ValueError: If message is empty. """ + if not message or not message.strip(): + raise ValueError("Message cannot be empty") + alerter = Client().active_stack.alerter if not alerter: raise RuntimeError( "No alerter configured in the active stack. " "Please configure an alerter in your stack." )Consider adding more helpful information in the docstring about how to format messages, particularly if the alerter supports HTML or Markdown formatting:
"""Step that sends an email alert using the active alerter. Args: message: Message to be included in the email. + HTML or Markdown formatting can be used for rich content. recipient_email: Optional recipient email address. If not provided, the email will be sent to the default recipient configured in the alerter. subject: Optional custom subject for the email. include_pipeline_info: If True, includes pipeline and step information in the email.src/zenml/integrations/smtp_email/flavors/smtp_email_alerter_flavor.py (1)
62-80: Consider handling possible timeouts and sensitive error handling inis_valid.While this method is a solid approach for validating SMTP connectivity, you might consider:
- Setting a timeout on the
smtplib.SMTPconstructor in case of unresponsive servers.- Ensuring any sensitive exception details don’t leak into logs in production environments.
docs/book/component-guide/alerters/smtp_email.md (2)
186-186: Add a comma after “alerter”.In the sentence, “These work with any alerter but may not provide optimal formatting for emails,” a comma after “alerter” can improve readability:
-These work with any alerter but may not provide optimal formatting for emails: +These work with any alerter, but may not provide optimal formatting for emails:🧰 Tools
🪛 LanguageTool
[uncategorized] ~186-~186: Possible missing comma found.
Context: ...erter Hooks These work with any alerter but may not provide optimal formatting for ...(AI_HYDRA_LEO_MISSING_COMMA)
236-242: Use consistent list style to satisfy linting rules.Markdownlint is flagging the dash-based list style here. Switching from dashes to asterisks will comply with the MD004 rule:
- - Format messages with proper HTML for email clients - - Include relevant pipeline and step information - - Set descriptive email subjects - - Use structured layout for better readability - - Format error tracebacks for better legibility in email clients - - Support Markdown-style formatting + * Format messages with proper HTML for email clients + * Include relevant pipeline and step information + * Set descriptive email subjects + * Use structured layout for better readability + * Format error tracebacks for better legibility in email clients + * Support Markdown-style formatting🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
236-236: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
237-237: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
238-238: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
239-239: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
240-240: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
241-241: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
242-242: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
src/zenml/integrations/smtp_email/hooks/email_alerter_hooks.py (2)
28-77: Avoid duplicated fallback handling logic in failure hook.While falling back to generic
alerter.post()is useful if the alerter is not an SMTPEmailAlerter, consider extracting the fallback logic into a shared helper method to reduce duplication and keep the hook more concise.
175-288: Refactor HTML templates for better maintainability.The success hook’s HTML generation mirrors much of the logic used in the failure hook, differing mainly by displayed content. You might centralize or parametricize the HTML template to reduce code duplication and improve long-term maintainability.
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py (4)
28-186: Consider using a standard Markdown library
The custom_format_markdown_for_htmlfunction uses regex-based transformations and manually escapes HTML. While this works, it can be error-prone for more complex Markdown features and poses potential XSS risks if any untrusted input slips through. A well-tested library, such as Python’smarkdownwith controlled extensions, could simplify maintenance and reduce vulnerabilities.
254-255: Ensure sensitive info is not exposed in prompts
Using{exception}and the full traceback in the prompt to the OpenAI model might unintentionally expose private data or credentials. Consider redacting sensitive information before sending it to external services.Would you like me to propose a secure redaction process for these fields?
273-273: Validate GPT suggestions for safety
Although the suggestion is appended as plain text here, confirm that any HTML or formatting from GPT is sanitized before use or display, especially if rendering beyond plain text contexts.
277-379: Separate SMTP email-building logic for clarity
Building the message, subject, and HTML body inline makes the code lengthy and less modular. Extracting HTML creation and subject assembly into helper methods could improve maintainability and readability.src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py (2)
181-314: Avoid duplicating markdown/HTML parsing logic
Certain markdown processing here overlaps with_format_markdown_for_htmlin the OpenAI hooks. Consider consolidating or reusing a shared helper function to reduce code duplication and potential drift.
315-378: Use a context manager for SMTP
Inpost, manually callingserver.quit()works but a context manager (e.g.,with smtplib.SMTP(...) as server:) is often safer and ensures proper cleanup even upon exceptions. Additionally, consider capturing and re-raising critical SMTP errors to alert the system if the email could not be sent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/book/component-guide/alerters/alerters.md(1 hunks)docs/book/component-guide/alerters/smtp_email.md(1 hunks)src/zenml/hooks/alerter_hooks.py(2 hunks)src/zenml/integrations/constants.py(1 hunks)src/zenml/integrations/openai/hooks/open_ai_failure_hook.py(7 hunks)src/zenml/integrations/smtp_email/__init__.py(1 hunks)src/zenml/integrations/smtp_email/alerters/__init__.py(1 hunks)src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py(1 hunks)src/zenml/integrations/smtp_email/flavors/__init__.py(1 hunks)src/zenml/integrations/smtp_email/flavors/smtp_email_alerter_flavor.py(1 hunks)src/zenml/integrations/smtp_email/hooks/__init__.py(1 hunks)src/zenml/integrations/smtp_email/hooks/email_alerter_hooks.py(1 hunks)src/zenml/integrations/smtp_email/steps/__init__.py(1 hunks)src/zenml/integrations/smtp_email/steps/smtp_email_alerter_post_step.py(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/zenml/integrations/smtp_email/steps/init.py
- src/zenml/integrations/smtp_email/alerters/init.py
- src/zenml/integrations/smtp_email/flavors/init.py
- src/zenml/integrations/smtp_email/hooks/init.py
🧰 Additional context used
📓 Path-based instructions (2)
`src/zenml/**/*.py`: Review the Python code for conformity w...
src/zenml/**/*.py: Review the Python code for conformity with Python best practices.
src/zenml/integrations/constants.pysrc/zenml/hooks/alerter_hooks.pysrc/zenml/integrations/smtp_email/flavors/smtp_email_alerter_flavor.pysrc/zenml/integrations/smtp_email/steps/smtp_email_alerter_post_step.pysrc/zenml/integrations/smtp_email/__init__.pysrc/zenml/integrations/openai/hooks/open_ai_failure_hook.pysrc/zenml/integrations/smtp_email/hooks/email_alerter_hooks.pysrc/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py
`docs/**/*.md`: Review the documentation for readability and...
docs/**/*.md: Review the documentation for readability and clarity.
docs/book/component-guide/alerters/alerters.mddocs/book/component-guide/alerters/smtp_email.md
🪛 LanguageTool
docs/book/component-guide/alerters/smtp_email.md
[uncategorized] ~23-~23: Possible missing comma found.
Context: ..., no additional integrations need to be installed as it uses the standard library's `smtp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...at the parameters mean: * smtp_server: Your email provider's SMTP server (e.g....
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Possible missing comma found.
Context: ...erter Hooks These work with any alerter but may not provide optimal formatting for ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/book/component-guide/alerters/smtp_email.md
236-236: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
237-237: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
238-238: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
239-239: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
240-240: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
241-241: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
242-242: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
🔇 Additional comments (15)
src/zenml/integrations/constants.py (1)
70-70: New SMTP Email integration constant added.This constant will be used to identify the new SMTP Email Alerter integration throughout the codebase.
docs/book/component-guide/alerters/alerters.md (4)
2-2: Documentation updated to include email as a notification method.The description now correctly reflects the addition of email alerts alongside chat services.
8-10: Description expanded to include email notifications.The introduction now clearly explains that Alerters can send messages to both chat services and email, improving user understanding of the component's capabilities.
14-14: SMTP Email Alerter added to available integrations list.The text now properly references the new alerter and its documentation page.
16-21: Table updated to include SMTP Email Alerter.The table now includes the new SMTP Email alerter with appropriate flavor and integration information. The formatting and structure of the table are maintained consistently.
src/zenml/hooks/alerter_hooks.py (2)
16-16: Improved exception handling for better email compatibility.The switch from Rich console to standard Python traceback formatting is a good improvement for email compatibility. This change ensures that exception information will display properly in email clients.
Also applies to: 36-41
48-51: Enhanced exception message formatting.The updated format for exception messages now includes the exception type name along with the exception message, providing more valuable information to users receiving the alerts.
src/zenml/integrations/smtp_email/flavors/smtp_email_alerter_flavor.py (1)
82-140: Well-structured flavor class.All the property methods (e.g.,
name,docs_url,sdk_docs_url,config_class,implementation_class) cleanly encapsulate flavor details. The usage of a dedicatedconfig_classhelps maintain clarity and consistency.src/zenml/integrations/smtp_email/__init__.py (1)
48-66: Cleanly handling potential circular imports.Your approach to registering the integration first, then conditionally importing hooks, is well designed to avoid import cycles. This structure is both practical and transparent.
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py (3)
16-17: Imports look appropriate
No issues identified with addingtracebackand typing (List,Match,Optional).
234-237: Redact or sanitize traceback details
Forwarding complete tracebacks via email may leak secrets or sensitive information. Consider sanitizing or redacting certain data, especially in production.
397-398: Verify custom model availability
Models “gpt-4o” and “gpt-4” are referenced here. Make sure “gpt-4o” is valid and accessible via the OpenAI API.Also applies to: 408-409
src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py (3)
16-19: Imports are valid
No issues spotted with the SMTP and MIME-related imports; these are standard for sending emails in Python.
82-129: Recipient email retrieval is well-structured
The logic for retrieving and validating the recipient email from parameters, settings, or the config is clear. Raising a ValueError if not found is appropriate.
379-398: Interactive approvals not supported
Theaskmethod correctly raisesNotImplementedError, reflecting that email-based alerters cannot handle interactive approvals. This is consistent with the alerter’s design.
src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py
Outdated
Show resolved
Hide resolved
| RuntimeError: If no alerter is configured in the active stack. | ||
| """ | ||
| alerter = Client().active_stack.alerter | ||
| if not alerter: |
There was a problem hiding this comment.
Shouldn't we also check the alerter type here?
In general it feels kinda weird to me what is happening with alerters. Shouldn't there be one alerter post step, which works independently of the alerter type? This is the whole reason for stack switching.
I would suggest that we think of a generic format that get's passed to alerter.post(...), and we can then get rid of all these specific steps. We can also avoid checks in the open AI hooks for example that handle the email alerter separately. The alerter itself should be responsible for the formatting, not some outside logic IMO.
There was a problem hiding this comment.
I've heavily updated it based on this comment. Hope I haven't gone too far. There's no breaking changes, but it's def a fairly large change.
src/zenml/integrations/smtp_email/alerters/smtp_email_alerter.py
Outdated
Show resolved
Hide resolved
…necessary installation check This commit simplifies the `SMTPEmailIntegration` class by removing the installation check and the circular import handling for hooks. The hooks are now expected to be imported directly from the hooks module in user code, streamlining the integration process.
This commit modifies the type hint of the `ask` method in the `SMTPEmailAlerter` class from `bool` to `Never`, indicating that this method is not intended to return a value. This change enhances code clarity and aligns with the method's purpose of not supporting interactive approvals for email alerters.
This commit simplifies the recipient email validation logic in the `SMTPEmailAlerter` class by removing unnecessary type checks. The condition now directly checks if `params` is truthy and if `recipient_email` is not `None`, enhancing code readability and maintainability.
- Test inline code, code blocks, bold, and italic formatting - Test HTML escaping and security - Test edge cases and special characters - Test nested and consecutive formatting
- Add try/except to catch NotImplementedError from alerters - Provide clear error message explaining which alerters support ask() - Update docstring to mention NotImplementedError
- Update Slack ask() method to handle AlerterMessage - Fix duplicate Args sections in Discord alerter - Add __all__ exports to smtp_email/__init__.py
- Replace type: ignore with proper try/except for Discord imports - Add helpful error message if discord.py is not installed - Verified SMTP email logo URL is accessible (returns 200 OK)
Properly decode smtp_error bytes to string in SMTPSenderRefused exception handling to avoid "b'...'" representation in error messages. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove unused type: ignore comment for discord import - Remove redundant cast and fix type-var errors by using @client.event directly - Fix union-attr errors by checking if channel isinstance of Messageable - Fix arg-type error by converting AlerterMessage to string before sending 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Improved the start_client method to properly clean up Discord client resources: - Ensure client is closed before loop cleanup - Add small delay for proper cleanup - Cancel all pending tasks explicitly - Handle CancelledError gracefully - Wait for task cancellation to complete This prevents "Unclosed connector" and "Task was destroyed but it is pending" warnings. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Adds a new SMTP Email Alerter integration, refactors existing Slack and Discord alerters to use a unified AlerterMessage model, and introduces generic alerter_post_step/alerter_ask_step steps while deprecating the specialized ones.
- New SMTP Email alerter with HTML/plain text, templating, and Markdown-to-HTML formatting
- Updated Slack and Discord alerters to accept
AlerterMessageand backward-compatibility hooks - Introduced generic alerter steps and centralized email templates; updated docs accordingly
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/zenml/integrations/slack/steps/slack_alerter_post_step.py | Added deprecation warning and logger import |
| src/zenml/integrations/slack/alerters/slack_alerter.py | Extended post/ask to accept AlerterMessage |
| src/zenml/integrations/openai/hooks/open_ai_failure_hook.py | Switched to plain traceback, added email-specific formatting |
| src/zenml/integrations/discord/alerters/discord_alerter.py | Extended post/ask, added cleanup in event loop |
| src/zenml/alerter/steps/alerter_post_step.py | New generic alerter_post_step |
| src/zenml/alerter/steps/alerter_ask_step.py | New generic alerter_ask_step |
| src/zenml/hooks/templates.py | Added HTML email templates |
| docs/book/component-guide/alerters/*.md | Updated docs for generic steps and SMTP Email |
Update link from ./alerters.md to ./README.md to point to the correct file containing the "How to Use Alerters with ZenML" section. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new SMTP Email Alerter integration (with HTML and plain-text support) and unifies the alerter interface via a standardized AlerterMessage model and generic steps. It also deprecates the specialized Slack/Discord alerter steps and updates the OpenAI failure hook to route email alerts through the new template system.
- Added SMTP Email Alerter integration with professional HTML templates and proper traceback formatting
- Implemented generic
alerter_post_stepandalerter_ask_stepleveragingAlerterMessage - Deprecated Slack- and Discord-specific steps and updated existing alerter implementations to accept
AlerterMessage
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/zenml/integrations/slack/steps/slack_alerter_post_step.py | Added deprecation warning for specialized Slack post step |
| src/zenml/integrations/slack/steps/slack_alerter_ask_step.py | Added deprecation warning for specialized Slack ask step |
| src/zenml/integrations/slack/alerters/slack_alerter.py | Extended to accept AlerterMessage and parse title/body |
| src/zenml/integrations/openai/hooks/open_ai_failure_hook.py | Updated failure hook to support SMTP Email alerter and HTML |
| src/zenml/integrations/discord/steps/discord_alerter_*.py | Added deprecation warnings for specialized Discord steps |
| src/zenml/integrations/discord/alerters/discord_alerter.py | Extended to accept AlerterMessage and parse title/body (ask bug) |
| src/zenml/alerter/steps/alerter_post_step.py | Added generic alerter_post_step using AlerterMessage |
| src/zenml/alerter/steps/alerter_ask_step.py | Added generic alerter_ask_step with improved error context |
| src/zenml/integrations/constants.py | Added SMTP_EMAIL constant |
| src/zenml/hooks/templates.py | Introduced shared HTML email templates |
| docs/book/component-guide/alerters/*.md | Updated docs for SMTP Email integration and generic steps |
Comments suppressed due to low confidence (4)
src/zenml/integrations/discord/alerters/discord_alerter.py:365
- In the
askmethod,embed_blocksis referenced insideon_readybut never computed—this will raise aNameError. You should callself._create_blocks(message_str, params)(or similar) before usingembed_blocks.
if embed_blocks:
src/zenml/integrations/slack/alerters/slack_alerter.py:271
- [nitpick] The docstring under
postlists themessageparameter twice—please remove the duplicate entry and ensure eachArgssection only appears once.
def post(
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py:29
- The new
_format_markdown_for_htmlfunction contains complex transformation logic but lacks dedicated unit tests. Consider adding tests covering code blocks, inline code, bold/italic, and list conversions to ensure correctness.
def _format_markdown_for_html(markdown_text: str) -> str:
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py:365
- The
openai_chatgpt_alerter_failure_hookwas changed to usegpt-4oinstead of the originalgpt-3.5-turbo. This may not be intentional and could alter behavior—please verify the intended model name.
openai_alerter_failure_hook_helper(exception, "gpt-4o")
Successfully resolved conflicts by combining: - New unified alerter approach (alerter_post_step/alerter_ask_step) - Comprehensive ask step documentation from develop - AlerterMessage support for structured messages - Deprecated specialized steps warnings - Custom approval/disapproval keywords documentation
- Extract 9 focused helper functions for each markdown transformation - Simplify main function to orchestrate transformations - Add comprehensive unit tests for each helper function (23 new tests) - Improve code readability and maintainability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Implement a unified approach for alerter hooks using the AlerterMessage model, providing better structured alerts across all alerter types (Slack, Discord, Email). - Update alerter_failure_hook and alerter_success_hook to use AlerterMessage with title, body, and metadata fields for better organization - Add graceful fallback to legacy string format for backwards compatibility - Add deprecation warnings in BaseAlerter.post() when strings are passed - Create comprehensive unit tests for both AlerterMessage and string formats - Update documentation to showcase AlerterMessage as the primary approach - Add hook usage examples to each alerter flavor documentation - Fix SMTP alerter docstring to resolve darglint validation errors This change improves consistency across alerter implementations while maintaining full backwards compatibility for existing users. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Adds a new SMTP Email Alerter integration and refactors existing alerting steps to use a unified AlerterMessage model across all alerters.
- Introduces
SMTPEmailAlerter, HTML/template support, and markdown-to-HTML formatting in failure hooks - Converts Slack and Discord steps and alerters to accept
AlerterMessagepayloads - Adds generic
alerter_post_stepandalerter_ask_stepsteps for all alerter flavors and updates docs
Reviewed Changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/zenml/integrations/slack/steps/slack_alerter_ask_step.py |
Deprecated Slack-specific ask step with warning and redirect |
src/zenml/integrations/slack/alerters/slack_alerter.py |
Support for AlerterMessage in post and ask methods |
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py |
Enhanced OpenAI failure hook with HTML/email formatting |
src/zenml/integrations/discord/steps/discord_alerter_post_step.py |
Deprecated Discord-specific post step with warning |
src/zenml/integrations/discord/alerters/discord_alerter.py |
Support for AlerterMessage and improved client cleanup logic |
src/zenml/alerter/steps/alerter_post_step.py |
New generic alerter_post_step |
src/zenml/alerter/steps/alerter_ask_step.py |
New generic alerter_ask_step |
src/zenml/alerter/base_alerter.py |
Base alerter updated to accept AlerterMessage |
src/zenml/hooks/templates.py |
New email template helpers |
docs/book/... |
Documentation updates for generic steps and SMTP integration |
Comments suppressed due to low confidence (4)
src/zenml/alerter/base_alerter.py:54
- The stub implementations of
postandaskinBaseAlerteralways returnTrue, which may hide missing overrides in subclasses. Consider making these methods abstract or raisingNotImplementedErrorto enforce that each alerter provides its own logic.
def post(
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py:460
- The
openai_chatgpt_alerter_failure_hooknow uses model namegpt-4o, which may be confusing or inconsistent with other hook names. Verify that this is the intended model identifier and align naming with users' expectations (e.g.,gpt-3.5-turboorgpt-4).
openai_alerter_failure_hook_helper(exception, "gpt-4o")
docs/book/how-to/steps-pipelines/advanced_features.md:463
- [nitpick] The sentence explains the new format but could be strengthened by linking to the
AlerterMessagemodel definition or example usage, improving discoverability for readers.
These hooks use the new `AlerterMessage` format which provides better structured alerts with title, body, and metadata fields.
src/zenml/integrations/openai/hooks/open_ai_failure_hook.py:376
- [nitpick] The multi-line f-string for
plain_messageincludes leading indentation and blank lines. Usetextwrap.dedentor build the string without indentation to avoid unintended whitespace in the email body.
plain_message = f"""OpenAI Failure Hook Notification - Step failed!
Never type was added to typing module in Python 3.11. For compatibility with earlier Python versions, import it from typing_extensions instead. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The PR adds a new SMTP Email Alerter integration to ZenML, enabling users to send email notifications on pipeline steps events. It also implements a unified alerter approach with a standardized AlerterMessage model across all alerter types.
Key Features
New SMTP Email Alerter Integration
Unified Alerter Architecture
AlerterMessagemodel for consistent alerter interfacesalerter_post_stepandalerter_ask_stepfor all alerter typesEnhanced Alerter Hooks (New)
alerter_failure_hookandalerter_success_hookto use structuredAlerterMessageformatalerter.post()Bug Fixes
default_subjectvariable in SMTP Email Alerter componentScreenshot
Tests run
Completed TODOs
Manual Testing with Example Pipelines
All three alerters have been successfully tested with example pipelines:
Discord Alerter: Pipeline Run
alerter_post_stepworkingalerter_ask_stepworkingSlack Alerter: Pipeline Run
alerter_post_stepworkingalerter_ask_stepworking (requires bot to be invited to channel)SMTP Email Alerter: Pipeline Run
Test code for all three alerters can be found in this Gist.