Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mail channel initial #1615

Closed
wants to merge 19 commits into from
4 changes: 3 additions & 1 deletion kairon/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@


def create_argument_parser():
from kairon.cli import importer, training, testing, conversations_deletion, translator, delete_logs, message_broadcast,content_importer
from kairon.cli import importer, training, testing, conversations_deletion, translator, delete_logs,\
message_broadcast,content_importer, mail_channel

parser = ArgumentParser(
prog="kairon",
Expand All @@ -62,6 +63,7 @@ def create_argument_parser():
delete_logs.add_subparser(subparsers, parents=parent_parsers)
message_broadcast.add_subparser(subparsers, parents=parent_parsers)
content_importer.add_subparser(subparsers, parents=parent_parsers)
mail_channel.add_subparser(subparsers, parents=parent_parsers)
return parser


Expand Down
59 changes: 59 additions & 0 deletions kairon/api/app/routers/bot/bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from kairon.shared.account.activity_log import UserActivityLogger
from kairon.shared.actions.data_objects import ActionServerLogs
from kairon.shared.auth import Authentication
from kairon.shared.channels.mail.data_objects import MailClassificationConfig
from kairon.shared.constants import TESTER_ACCESS, DESIGNER_ACCESS, CHAT_ACCESS, UserActivityType, ADMIN_ACCESS, \
EventClass, AGENT_ACCESS
from kairon.shared.content_importer.content_processor import ContentImporterLogProcessor
Expand All @@ -33,6 +34,7 @@
from kairon.shared.data.audit.processor import AuditDataProcessor
from kairon.shared.data.constant import ENDPOINT_TYPE, ModelTestType, \
AuditlogActions
from kairon.shared.data.data_models import MailConfigRequest
from kairon.shared.data.data_objects import TrainingExamples, ModelTraining, Rules
from kairon.shared.data.model_processor import ModelProcessor
from kairon.shared.data.processor import MongoProcessor
Expand Down Expand Up @@ -1658,3 +1660,60 @@ async def get_slot_actions(
llm_models = MongoProcessor.get_slot_mapped_actions(current_user.get_bot(), slot_name)
return Response(data=llm_models)


@router.get("/mail/config", response_model=Response)
async def get_all_mail_configs(
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=TESTER_ACCESS)):
"""
Fetches mail config
"""
data = MailClassificationConfig.objects(bot=current_user.get_bot(), status=True)
formatted_data = [
{key: value for key, value in item.to_mongo().items() if key not in {"_id", "user"}}
for item in data
]

return {"data": formatted_data}



@router.post("/mail/config", response_model=Response)
async def set_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
Applies the mail config
"""
request_dict = request_data.dict()
MailClassificationConfig.create_doc(**request_dict, bot=current_user.get_bot(), user=current_user.get_user())
return {"message": "Config applied!"}

Comment on lines +1680 to +1691
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for duplicate configurations

The endpoint should handle potential duplicate configurations gracefully.

     request_dict = request_data.dict()
+    try:
         MailClassificationConfig.create_doc(**request_dict, bot=current_user.get_bot(), user=current_user.get_user())
-    return {"message": "Config applied!"}
+        return {"message": "Config applied!"}
+    except Exception as e:
+        raise AppException(f"Failed to create mail configuration: {str(e)}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@router.post("/mail/config", response_model=Response)
async def set_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
Applies the mail config
"""
request_dict = request_data.dict()
MailClassificationConfig.create_doc(**request_dict, bot=current_user.get_bot(), user=current_user.get_user())
return {"message": "Config applied!"}
@router.post("/mail/config", response_model=Response)
async def set_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
Applies the mail config
"""
request_dict = request_data.dict()
try:
MailClassificationConfig.create_doc(**request_dict, bot=current_user.get_bot(), user=current_user.get_user())
return {"message": "Config applied!"}
except Exception as e:
raise AppException(f"Failed to create mail configuration: {str(e)}")
🧰 Tools
🪛 Ruff (0.8.0)

1683-1683: Do not perform function call Security in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


@router.put("/mail/config", response_model=Response)
async def update_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
update the mail config
"""
request_dict = request_data.dict()
MailClassificationConfig.update_doc(**request_dict, bot=current_user.get_bot())
return {"message": "Config updated!"}

Comment on lines +1693 to +1704
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for existing configuration

The endpoint should verify that the configuration exists before updating.

     request_dict = request_data.dict()
+    existing = MailClassificationConfig.objects(bot=current_user.get_bot(), intent=request_dict['intent'], status=True).first()
+    if not existing:
+        raise AppException(f"Mail configuration for intent '{request_dict['intent']}' not found")
     MailClassificationConfig.update_doc(**request_dict, bot=current_user.get_bot())
     return {"message": "Config updated!"}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@router.put("/mail/config", response_model=Response)
async def update_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
update the mail config
"""
request_dict = request_data.dict()
MailClassificationConfig.update_doc(**request_dict, bot=current_user.get_bot())
return {"message": "Config updated!"}
@router.put("/mail/config", response_model=Response)
async def update_mail_config(
request_data: MailConfigRequest,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
update the mail config
"""
request_dict = request_data.dict()
existing = MailClassificationConfig.objects(bot=current_user.get_bot(), intent=request_dict['intent'], status=True).first()
if not existing:
raise AppException(f"Mail configuration for intent '{request_dict['intent']}' not found")
MailClassificationConfig.update_doc(**request_dict, bot=current_user.get_bot())
return {"message": "Config updated!"}
🧰 Tools
🪛 Ruff (0.8.0)

1696-1696: Do not perform function call Security in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)



@router.delete("/mail/config/{intent}", response_model=Response)
async def del_soft_mail_config(
intent: str,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
delete the mail config
"""
MailClassificationConfig.soft_delete_doc(current_user.get_bot(), intent)
return {"message": "Config deleted!"}
Comment on lines +1707 to +1716
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for existing configuration

The endpoint should verify that the configuration exists before deletion.

+    existing = MailClassificationConfig.objects(bot=current_user.get_bot(), intent=intent, status=True).first()
+    if not existing:
+        raise AppException(f"Mail configuration for intent '{intent}' not found")
     MailClassificationConfig.soft_delete_doc(current_user.get_bot(), intent)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@router.delete("/mail/config/{intent}", response_model=Response)
async def del_soft_mail_config(
intent: str,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
delete the mail config
"""
MailClassificationConfig.soft_delete_doc(current_user.get_bot(), intent)
return {"message": "Config deleted!"}
@router.delete("/mail/config/{intent}", response_model=Response)
async def del_soft_mail_config(
intent: str,
current_user: User = Security(Authentication.get_current_user_and_bot, scopes=DESIGNER_ACCESS)
):
"""
delete the mail config
"""
existing = MailClassificationConfig.objects(bot=current_user.get_bot(), intent=intent, status=True).first()
if not existing:
raise AppException(f"Mail configuration for intent '{intent}' not found")
MailClassificationConfig.soft_delete_doc(current_user.get_bot(), intent)
return {"message": "Config deleted!"}
🧰 Tools
🪛 Ruff (0.8.0)

1710-1710: Do not perform function call Security in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)




19 changes: 19 additions & 0 deletions kairon/chat/agent_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,22 @@ async def handle_channel_message(bot: Text, userdata: UserMessage):
if not is_live_agent_enabled:
return await AgentProcessor.get_agent(bot).handle_message(userdata)
return await LiveAgentHandler.process_live_agent(bot, userdata)

@staticmethod
def get_agent_without_cache(bot: str, use_store: bool = True) -> Agent:
endpoint = AgentProcessor.mongo_processor.get_endpoints(
bot, raise_exception=False
)
action_endpoint = Utility.get_action_url(endpoint)
model_path = Utility.get_latest_model(bot)
domain = AgentProcessor.mongo_processor.load_domain(bot)
if use_store:
mongo_store = Utility.get_local_mongo_store(bot, domain)
lock_store_endpoint = Utility.get_lock_store(bot)
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint, tracker_store=mongo_store,
lock_store=lock_store_endpoint)
else:
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint)

agent.model_ver = model_path.split("/")[-1]
return agent
Comment on lines +82 to +99
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring to reduce code duplication and add error handling

The implementation duplicates logic from the reload method and lacks proper error handling. Consider the following improvements:

  1. Add comprehensive error handling similar to the reload method
  2. Add docstring with parameter descriptions
  3. Consider extracting common logic into a private method shared with reload

Here's a suggested implementation:

    @staticmethod
    def get_agent_without_cache(bot: str, use_store: bool = True) -> Agent:
+        """
+        Load bot agent directly from model without caching
+
+        Args:
+            bot: bot identifier
+            use_store: whether to use MongoDB store and lock store
+
+        Returns:
+            Agent: loaded bot agent
+
+        Raises:
+            AppException: if bot has not been trained
+        """
+        try:
            endpoint = AgentProcessor.mongo_processor.get_endpoints(
                bot, raise_exception=False
            )
            action_endpoint = Utility.get_action_url(endpoint)
            model_path = Utility.get_latest_model(bot)
            domain = AgentProcessor.mongo_processor.load_domain(bot)
            if use_store:
                mongo_store = Utility.get_local_mongo_store(bot, domain)
                lock_store_endpoint = Utility.get_lock_store(bot)
                agent = KaironAgent.load(model_path, action_endpoint=action_endpoint, tracker_store=mongo_store,
                                         lock_store=lock_store_endpoint)
            else:
                agent = KaironAgent.load(model_path, action_endpoint=action_endpoint)

            agent.model_ver = model_path.split("/")[-1]
            return agent
+        except Exception as e:
+            logging.exception(e)
+            raise AppException("Bot has not been trained yet!")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@staticmethod
def get_agent_without_cache(bot: str, use_store: bool = True) -> Agent:
endpoint = AgentProcessor.mongo_processor.get_endpoints(
bot, raise_exception=False
)
action_endpoint = Utility.get_action_url(endpoint)
model_path = Utility.get_latest_model(bot)
domain = AgentProcessor.mongo_processor.load_domain(bot)
if use_store:
mongo_store = Utility.get_local_mongo_store(bot, domain)
lock_store_endpoint = Utility.get_lock_store(bot)
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint, tracker_store=mongo_store,
lock_store=lock_store_endpoint)
else:
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint)
agent.model_ver = model_path.split("/")[-1]
return agent
@staticmethod
def get_agent_without_cache(bot: str, use_store: bool = True) -> Agent:
"""
Load bot agent directly from model without caching
Args:
bot: bot identifier
use_store: whether to use MongoDB store and lock store
Returns:
Agent: loaded bot agent
Raises:
AppException: if bot has not been trained
"""
try:
endpoint = AgentProcessor.mongo_processor.get_endpoints(
bot, raise_exception=False
)
action_endpoint = Utility.get_action_url(endpoint)
model_path = Utility.get_latest_model(bot)
domain = AgentProcessor.mongo_processor.load_domain(bot)
if use_store:
mongo_store = Utility.get_local_mongo_store(bot, domain)
lock_store_endpoint = Utility.get_lock_store(bot)
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint, tracker_store=mongo_store,
lock_store=lock_store_endpoint)
else:
agent = KaironAgent.load(model_path, action_endpoint=action_endpoint)
agent.model_ver = model_path.split("/")[-1]
return agent
except Exception as e:
logging.exception(e)
raise AppException("Bot has not been trained yet!")

31 changes: 31 additions & 0 deletions kairon/chat/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,37 @@ async def chat(
)
return chat_response

@staticmethod
async def process_messages_via_bot(
messages: [str],
account: int,
bot: str,
user: str,
is_integration_user: bool = False,
metadata: Dict = None,
):
"""Process a list of messages through the bot.
Args:
messages: List of messages to process
account: Account ID
bot: Bot ID
user: User ID
is_integration_user: Flag indicating if user is integration user
metadata: Additional metadata

Returns:
List of responses from the bot
"""
responses = []
uncached_model = AgentProcessor.get_agent_without_cache(bot, False)
metadata = ChatUtils.get_metadata(account, bot, is_integration_user, metadata)
for message in messages:
msg = UserMessage(message, sender_id=user, metadata=metadata)
chat_response = await uncached_model.handle_message(msg)
responses.append(chat_response)
return responses
Comment on lines +46 to +74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and consider caching implications

While the implementation is good, consider these improvements:

  1. Add error handling for failed responses
  2. Consider the performance impact of using uncached model for each batch

Consider this implementation:

 @staticmethod
 async def process_messages_via_bot(
         messages: [str],
         account: int,
         bot: str,
         user: str,
         is_integration_user: bool = False,
         metadata: Dict = None,
 ):
     responses = []
-    uncached_model = AgentProcessor.get_agent_without_cache(bot, False)
+    model = AgentProcessor.get_agent(bot)  # Use cached model
     metadata = ChatUtils.get_metadata(account, bot, is_integration_user, metadata)
     for message in messages:
         try:
             msg = UserMessage(message, sender_id=user, metadata=metadata)
-            chat_response = await uncached_model.handle_message(msg)
+            chat_response = await model.handle_message(msg)
             responses.append(chat_response)
+        except Exception as e:
+            logger.error(f"Error processing message: {message}, error: {str(e)}")
+            responses.append({"error": str(e)})
     return responses

Committable suggestion skipped: line range outside the PR's diff.



@staticmethod
def reload(bot: Text, user: Text):
exc = None
Expand Down
36 changes: 36 additions & 0 deletions kairon/cli/mail_channel.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import json
from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
from typing import List
from rasa.cli import SubParsersAction

from kairon.events.definitions.mail_channel_schedule import MailChannelScheduleEvent


def process_channel_mails(args):
mails = json.loads(args.mails)
if not isinstance(mails, list):
raise ValueError("Mails should be a list")
MailChannelScheduleEvent(args.bot, args.user).execute(mails=mails)
Comment on lines +9 to +13
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance input validation and error handling

The current JSON parsing lacks:

  1. Size limits for the input
  2. Proper error handling for malformed JSON
  3. Validation of individual mail objects in the list
 def process_channel_mails(args):
+    MAX_SIZE = 1024 * 1024  # 1MB limit
+    if len(args.mails) > MAX_SIZE:
+        raise ValueError("Input size exceeds maximum allowed size")
+    
+    try:
         mails = json.loads(args.mails)
+    except json.JSONDecodeError as e:
+        raise ValueError(f"Invalid JSON format: {str(e)}")
+
     if not isinstance(mails, list):
         raise ValueError("Mails should be a list")
+        
+    if not mails:
+        raise ValueError("Mail list cannot be empty")
+        
+    for mail in mails:
+        if not isinstance(mail, dict) or 'subject' not in mail or 'body' not in mail:
+            raise ValueError("Each mail must be a dictionary with 'subject' and 'body' fields")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def process_channel_mails(args):
mails = json.loads(args.mails)
if not isinstance(mails, list):
raise ValueError("Mails should be a list")
MailChannelScheduleEvent(args.bot, args.user).execute(mails=mails)
def process_channel_mails(args):
MAX_SIZE = 1024 * 1024 # 1MB limit
if len(args.mails) > MAX_SIZE:
raise ValueError("Input size exceeds maximum allowed size")
try:
mails = json.loads(args.mails)
except json.JSONDecodeError as e:
raise ValueError(f"Invalid JSON format: {str(e)}")
if not isinstance(mails, list):
raise ValueError("Mails should be a list")
if not mails:
raise ValueError("Mail list cannot be empty")
for mail in mails:
if not isinstance(mail, dict) or 'subject' not in mail or 'body' not in mail:
raise ValueError("Each mail must be a dictionary with 'subject' and 'body' fields")
MailChannelScheduleEvent(args.bot, args.user).execute(mails=mails)



def add_subparser(subparsers: SubParsersAction, parents: List[ArgumentParser]):
mail_parser = subparsers.add_parser(
"mail-channel",
conflict_handler="resolve",
formatter_class=ArgumentDefaultsHelpFormatter,
parents=parents,
help="Mail channel"
)
mail_parser.add_argument('bot',
type=str,
help="Bot id for which command is executed", action='store')

mail_parser.add_argument('user',
type=str,
help="Kairon user who is initiating the command", action='store')
Comment on lines +24 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for bot and user IDs

The CLI accepts bot and user IDs without validation. Consider adding format checks or existence validation.

     mail_parser.add_argument('bot',
                             type=str,
+                            pattern=r'^[a-zA-Z0-9_-]+$',
                             help="Bot id for which command is executed", action='store')

     mail_parser.add_argument('user',
                            type=str,
+                           pattern=r'^[a-zA-Z0-9_-]+$',
                            help="Kairon user who is initiating the command", action='store')

Committable suggestion skipped: line range outside the PR's diff.


mail_parser.add_argument('mails',
type=str,
help="json representing List of mails to be processed", action='store')

mail_parser.set_defaults(func=process_channel_mails)
4 changes: 3 additions & 1 deletion kairon/events/definitions/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from kairon.events.definitions.data_importer import TrainingDataImporterEvent
from kairon.events.definitions.faq_importer import FaqDataImporterEvent
from kairon.events.definitions.history_delete import DeleteHistoryEvent
from kairon.events.definitions.mail_channel_schedule import MailChannelScheduleEvent
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

EventClass enum is missing the required email_channel_scheduler value

The EventClass enum in kairon/shared/constants.py needs to be updated to include the email_channel_scheduler value to match the new MailChannelScheduleEvent integration.

  • Add email_channel_scheduler to the EventClass enum in kairon/shared/constants.py
🔗 Analysis chain

LGTM! Clean integration of mail channel scheduler.

The changes properly integrate the mail channel scheduler into the event system while maintaining the existing structure.

Let's verify the EventClass enum definition:

Also applies to: 24-25

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify EventClass enum includes email_channel_scheduler
rg -A 1 "class EventClass" | rg -A 15 "email_channel_scheduler"

Length of output: 63


Script:

#!/bin/bash
# Let's first find the file containing EventClass definition
fd "EventClass" --type f

# Then examine the enum definition
rg "class EventClass" -A 10

Length of output: 754

from kairon.events.definitions.message_broadcast import MessageBroadcastEvent
from kairon.events.definitions.model_testing import ModelTestingEvent
from kairon.events.definitions.model_training import ModelTrainingEvent
Expand All @@ -20,7 +21,8 @@ class EventFactory:
EventClass.multilingual: MultilingualEvent,
EventClass.faq_importer: FaqDataImporterEvent,
EventClass.message_broadcast: MessageBroadcastEvent,
EventClass.content_importer: DocContentImporterEvent
EventClass.content_importer: DocContentImporterEvent,
EventClass.email_channel_scheduler: MailChannelScheduleEvent
}

@staticmethod
Expand Down
51 changes: 51 additions & 0 deletions kairon/events/definitions/mail_channel_schedule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
from typing import Text
from loguru import logger
from kairon import Utility
from kairon.events.definitions.base import EventsBase
from kairon.exceptions import AppException
from kairon.shared.channels.mail.processor import MailProcessor
from kairon.shared.constants import EventClass


class MailChannelScheduleEvent(EventsBase):
"""
Event to start mail channel scheduler if not already running.
"""

def __init__(self, bot: Text, user: Text, **kwargs):
"""
Initialise event.
"""
self.bot = bot
self.user = user

def validate(self):
"""
validate mail channel exists
"""
return MailProcessor.validate_smpt_connection(self.bot)

Comment on lines +22 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance SMTP connection validation

The validate method could be more comprehensive:

  1. Add timeout for SMTP connection
  2. Validate required mail configuration
  3. Check for required permissions
     def validate(self):
         """
-        validate mail channel exists
+        Validate mail channel configuration and connectivity.
+
+        Returns:
+            bool: True if validation succeeds
+
+        Raises:
+            AppException: If validation fails
         """
-        return MailProcessor.validate_smpt_connection(self.bot)
+        try:
+            if not MailProcessor.validate_mail_config(self.bot):
+                raise AppException("Invalid mail configuration")
+            return MailProcessor.validate_smtp_connection(self.bot, timeout=10)
+        except Exception as e:
+            logger.error(f"Mail channel validation failed for bot {self.bot}: {str(e)}")
+            raise AppException("Mail channel validation failed") from e

Committable suggestion skipped: line range outside the PR's diff.


def enqueue(self, **kwargs):
"""
Send event to event server.
"""
try:
mails: list = kwargs.get('mails', [])
payload = {'bot': self.bot, 'user': self.user, 'mails': mails}
Utility.request_event_server(EventClass.email_channel_scheduler, payload)
except Exception as e:
logger.error(str(e))
raise AppException(e)
Comment on lines +37 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve exception handling in enqueue method

The exception handling needs improvement to preserve the traceback.

         except Exception as e:
             logger.error(str(e))
-            raise AppException(e)
+            raise AppException("Failed to enqueue mail channel event") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
except Exception as e:
logger.error(str(e))
raise AppException(e)
except Exception as e:
logger.error(str(e))
raise AppException("Failed to enqueue mail channel event") from e
🧰 Tools
🪛 Ruff (0.8.0)

39-39: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


def execute(self, **kwargs):
"""
Execute the event.
"""
try:
mails = kwargs.get('mails')
if mails:
MailProcessor.process_message_task(self.bot, mails)
except Exception as e:
logger.error(str(e))
raise AppException(e)
Comment on lines +41 to +51
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add retry mechanism and improve error handling in execute method

The execute method could benefit from:

  1. A retry mechanism for transient failures
  2. Better error handling with preserved traceback
  3. More detailed logging
     def execute(self, **kwargs):
         """
         Execute the event.
+
+        Args:
+            **kwargs: Must contain 'mails' list of mail objects to process
+
+        Raises:
+            AppException: If mail processing fails
         """
         try:
             mails = kwargs.get('mails')
+            if not mails:
+                logger.warning(f"No mails to process for bot {self.bot}")
+                return
+                
             if mails:
-                MailProcessor.process_message_task(self.bot, mails)
+                max_retries = 3
+                for attempt in range(max_retries):
+                    try:
+                        MailProcessor.process_message_task(self.bot, mails)
+                        break
+                    except Exception as retry_error:
+                        if attempt == max_retries - 1:
+                            raise retry_error
+                        logger.warning(f"Retry {attempt + 1}/{max_retries} failed: {str(retry_error)}")
         except Exception as e:
-            logger.error(str(e))
-            raise AppException(e)
+            logger.error(f"Failed to process mails for bot {self.bot}: {str(e)}")
+            raise AppException("Mail processing failed") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def execute(self, **kwargs):
"""
Execute the event.
"""
try:
mails = kwargs.get('mails')
if mails:
MailProcessor.process_message_task(self.bot, mails)
except Exception as e:
logger.error(str(e))
raise AppException(e)
def execute(self, **kwargs):
"""
Execute the event.
Args:
**kwargs: Must contain 'mails' list of mail objects to process
Raises:
AppException: If mail processing fails
"""
try:
mails = kwargs.get('mails')
if not mails:
logger.warning(f"No mails to process for bot {self.bot}")
return
if mails:
max_retries = 3
for attempt in range(max_retries):
try:
MailProcessor.process_message_task(self.bot, mails)
break
except Exception as retry_error:
if attempt == max_retries - 1:
raise retry_error
logger.warning(f"Retry {attempt + 1}/{max_retries} failed: {str(retry_error)}")
except Exception as e:
logger.error(f"Failed to process mails for bot {self.bot}: {str(e)}")
raise AppException("Mail processing failed") from e
🧰 Tools
🪛 Ruff (0.8.0)

51-51: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

9 changes: 9 additions & 0 deletions kairon/events/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ async def lifespan(app: FastAPI):
""" MongoDB is connected on the bot trainer startup """
config: dict = Utility.mongoengine_connection(Utility.environment['database']["url"])
connect(**config)
from kairon.shared.channels.mail.scheduler import MailScheduler
MailScheduler.epoch()
Comment on lines +59 to +60
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for MailScheduler.epoch()

The MailScheduler.epoch() call during startup should include error handling to prevent silent failures and ensure proper logging.

 from kairon.shared.channels.mail.scheduler import MailScheduler
-MailScheduler.epoch()
+try:
+    MailScheduler.epoch()
+except Exception as e:
+    logger.exception(f"Failed to initialize mail scheduler: {str(e)}")
+    raise
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from kairon.shared.channels.mail.scheduler import MailScheduler
MailScheduler.epoch()
from kairon.shared.channels.mail.scheduler import MailScheduler
try:
MailScheduler.epoch()
except Exception as e:
logger.exception(f"Failed to initialize mail scheduler: {str(e)}")
raise

yield
disconnect()

Expand Down Expand Up @@ -144,3 +146,10 @@ def delete_scheduled_event(event_id: Text = Path(description="Event id")):
def dispatch_scheduled_event(event_id: Text = Path(description="Event id")):
KScheduler().dispatch_event(event_id)
return {"data": None, "message": "Scheduled event dispatch!"}


@app.get('/api/mail/request_epoch', response_model=Response)
def request_epoch():
from kairon.shared.channels.mail.scheduler import MailScheduler
MailScheduler.epoch()
return {"data": None, "message": "Mail scheduler epoch request!"}
Empty file.
25 changes: 25 additions & 0 deletions kairon/shared/channels/mail/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@


class MailConstants:
DEFAULT_SMTP_SERVER = 'smtp.gmail.com'
DEFAULT_IMAP_SERVER = 'imap.gmail.com'
DEFAULT_SMTP_PORT = 587
DEFAULT_LLM_TYPE = "openai"
DEFAULT_HYPERPARAMETERS = {
"frequency_penalty": 0,
"logit_bias": {},
"max_tokens": 300,
"model": "gpt-4o-mini",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in model name

The model name "gpt-4o-mini" appears to be incorrect. Common OpenAI model names are "gpt-4", "gpt-3.5-turbo", etc.

-        "model": "gpt-4o-mini",
+        "model": "gpt-4"  # or appropriate model name

Committable suggestion skipped: line range outside the PR's diff.

"n": 1,
"presence_penalty": 0,
"stop": None,
"stream": False,
"temperature": 0,
"top_p": 0
}
DEFAULT_TEMPLATE = "<p>Dear {name},</p> <p>{bot_response}</p> <br/><br/><span style='color:#999;'> Generated by kAIron AI.</span>\n"
DEFAULT_SYSTEM_PROMPT = 'Classify into one of the intents and extract entities as given in the context.' \
'If the mail does not belong to any of the intents, classify intent as null.'

PROCESS_MESSAGE_BATCH_SIZE = 4

Loading
Loading