-
Notifications
You must be signed in to change notification settings - Fork 79
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
Mail channel initial #1615
Changes from 18 commits
57eda3a
42c839e
9c7cee8
f550de4
3c7aac8
a46955d
d33567c
ef5ae5d
c81ec96
20291d1
198c8ff
4d4da37
7cc9c6b
16477a3
36905f1
e28f9f0
6a84142
497bbdb
c35b0a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -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!"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
@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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🧰 Tools🪛 Ruff (0.8.0)1696-1696: Do not perform function call (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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🧰 Tools🪛 Ruff (0.8.0)1710-1710: Do not perform function call (B008) |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
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
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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
|
||
|
||
|
||
@staticmethod | ||
def reload(bot: Text, user: Text): | ||
exc = None | ||
|
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance input validation and error handling The current JSON parsing lacks:
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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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')
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
🔗 Analysis chainLGTM! 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 executedThe 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 | ||
|
@@ -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 | ||
|
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🧰 Tools🪛 Ruff (0.8.0)39-39: Within an (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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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
Suggested change
🧰 Tools🪛 Ruff (0.8.0)51-51: Within an (B904) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||
yield | ||||||||||||||||||
disconnect() | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -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!"} |
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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
"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 | ||
|
There was a problem hiding this comment.
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.
📝 Committable suggestion
🧰 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)