From 0627b9003094179a48f3801ce07ea9178d8bbcfc Mon Sep 17 00:00:00 2001 From: Aryan Neogi Date: Wed, 10 Jun 2026 00:13:30 +0530 Subject: [PATCH] fix/Turn of successful githab CI notifications for Discord --- fenn/notification/notifier.py | 7 +++-- fenn/notification/service.py | 14 ++++++++++ fenn/notification/services/discord.py | 24 +++++++++++++++-- fenn/remote/credentials.py | 3 +-- .../unit/notification/test_discord_service.py | 27 +++++++++++++++++++ 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/fenn/notification/notifier.py b/fenn/notification/notifier.py index 84678d6..ecb021d 100644 --- a/fenn/notification/notifier.py +++ b/fenn/notification/notifier.py @@ -49,11 +49,13 @@ def remove_service(self, service: Type[Service]) -> None: ) raise - def notify(self, message: str) -> None: + def notify(self, message: str, is_success: bool = True) -> None: """Send notification to all registered services. Args: message: The message to send. + is_success: Whether the operation succeeded (default: True). + Services can use this to filter notifications. """ if not self._services: return @@ -63,7 +65,8 @@ def notify(self, message: str) -> None: for service in self._services: try: - service.send_notification(message) + if service.should_notify(is_success): + service.send_notification(message) successful_services.append(service.__class__.__name__) # logger.info(f"Successfully sent notification via {service.__class__.__name__}") except Exception as e: diff --git a/fenn/notification/service.py b/fenn/notification/service.py index 99d5e0b..2db04ae 100644 --- a/fenn/notification/service.py +++ b/fenn/notification/service.py @@ -15,3 +15,17 @@ def send_notification(self, message: str) -> None: Exception: If the notification fails to send. """ pass + + def should_notify(self, is_success: bool) -> bool: + """Determine if a notification should be sent based on status. + + This method can be overridden by subclasses to filter notifications + based on success/failure status. + + Args: + is_success: True if the operation succeeded, False if it failed. + + Returns: + True if the notification should be sent, False otherwise. + """ + return True diff --git a/fenn/notification/services/discord.py b/fenn/notification/services/discord.py index 076c8d3..ea6ae54 100644 --- a/fenn/notification/services/discord.py +++ b/fenn/notification/services/discord.py @@ -6,11 +6,18 @@ class Discord(Service): """Discord notification service using webhooks.""" - def __init__(self, webhook_url: str): - """Initialize Discord service.""" + def __init__(self, webhook_url: str, notify_on_success: bool = False): + """Initialize Discord service. + + Args: + webhook_url: Discord webhook URL. + notify_on_success: If False (default), only send notifications on failure. + If True, send notifications for both success and failure. + """ super().__init__() self._discord_webhook_url = webhook_url + self._notify_on_success = notify_on_success def send_notification(self, message: str) -> None: """Send notification to Discord channel. @@ -30,3 +37,16 @@ def send_notification(self, message: str) -> None: raise requests.exceptions.RequestException( f"Failed to send Discord notification: {err}" ) + + def should_notify(self, is_success: bool) -> bool: + """Determine if a notification should be sent based on status. + + Args: + is_success: True if the operation succeeded, False if it failed. + + Returns: + True if the notification should be sent, False otherwise. + """ + if is_success: + return self._notify_on_success + return True diff --git a/fenn/remote/credentials.py b/fenn/remote/credentials.py index f6466b7..96b5743 100644 --- a/fenn/remote/credentials.py +++ b/fenn/remote/credentials.py @@ -24,12 +24,11 @@ import os import sys +import tomllib from dataclasses import dataclass from pathlib import Path from typing import Dict, Optional -import tomllib - from fenn.remote.exceptions import CredentialsError DEFAULT_PROFILE = "default" diff --git a/tests/unit/notification/test_discord_service.py b/tests/unit/notification/test_discord_service.py index e21254c..041a3eb 100644 --- a/tests/unit/notification/test_discord_service.py +++ b/tests/unit/notification/test_discord_service.py @@ -60,3 +60,30 @@ def test_send_notification_error(self, send_message_to_discord, message): ) assert "400" in str(exc_info.value) + + def test_notify_on_success_false_blocks_success(self): + """Test that notify_on_success=False blocks success notifications.""" + discord = Discord( + "https://discord.com/api/webhooks/123/abc", notify_on_success=False + ) + assert discord.should_notify(is_success=True) is False + + def test_notify_on_success_false_allows_failure(self): + """Test that notify_on_success=False allows failure notifications.""" + discord = Discord( + "https://discord.com/api/webhooks/123/abc", notify_on_success=False + ) + assert discord.should_notify(is_success=False) is True + + def test_notify_on_success_true_allows_both(self): + """Test that notify_on_success=True allows both success and failure notifications.""" + discord = Discord( + "https://discord.com/api/webhooks/123/abc", notify_on_success=True + ) + assert discord.should_notify(is_success=True) is True + assert discord.should_notify(is_success=False) is True + + def test_default_notify_on_success_is_false(self): + """Test that the default value of notify_on_success is False.""" + discord = Discord("https://discord.com/api/webhooks/123/abc") + assert discord.should_notify(is_success=True) is False