-
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
Upload/download fix live agent #1514
base: master
Are you sure you want to change the base?
Conversation
optional bot_response in callback action (log failure for no bot_response fix)
WalkthroughThe changes introduced in this pull request enhance the message dispatching logic in the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
kairon/shared/data/processor.py (2)
Line range hint
59-80
: Theapply_template
method looks good overall with one small suggestion.It handles applying a use-case template correctly, checking if the template directory exists before proceeding.
Using a default
sysadmin
user for templates seems reasonable.[suggestion] Consider providing a more specific error message than "Invalid template!" when the template directory does not exist, to help users diagnose the issue.
Line range hint
165-180
: Theapply_config
method looks good overall with one small suggestion.It handles applying a config template correctly, checking if the config file exists before proceeding.
[suggestion] Consider providing a more specific error message than "Invalid config!" when the config file does not exist, to help users diagnose the issue.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- kairon/async_callback/processor.py (2 hunks)
- kairon/shared/data/processor.py (4 hunks)
- tests/unit_test/data_processor/data_processor_test.py (3 hunks)
Additional comments not posted (16)
kairon/async_callback/processor.py (2)
85-86
: LGTM!The added conditional check is a good improvement. It prevents dispatching empty or None responses, thereby avoiding unnecessary calls to
dispatch_message
. This enhances the efficiency and robustness of the code.
155-156
: LGTM!The added conditional check is a good improvement. It prevents dispatching empty or None responses, thereby avoiding unnecessary calls to
dispatch_message
. This enhances the efficiency and robustness of the code, similar to the changes made in theasync_callback
function.kairon/shared/data/processor.py (11)
Line range hint
1-27
: Theupload_and_save
method looks good overall.It handles uploading and saving the training data files correctly, and cleans up the temporary directory after processing. No major issues observed.
Line range hint
29-57
: Thedownload_files
method looks good overall.It correctly fetches the various training data objects and packages them into a zip file for download. Using a zip file is an appropriate way to deliver multiple files. No major issues observed.
Line range hint
82-128
: Thesave_from_path
method looks good overall.It handles loading and saving Rasa training data from a directory correctly. Using the
RasaFileImporter
is the right way to load the various Rasa file formats.Validating the custom actions using
TrainingDataValidator
is a good practice.The loaded data is saved appropriately using
self.save_training_data()
.No major issues observed.
Line range hint
130-163
: Thesave_training_data
method looks good overall.It provides a flexible way to save different parts of the training data based on the
what
argument. Theoverwrite
argument is useful to control whether existing data should be replaced.Delegating the actual saving of individual pieces of data to separate methods is good for modularity and keeping this method concise.
No major issues observed.
Line range hint
182-191
: Theload_multiflow_stories_yaml
method looks good.It queries the
MultiflowStories
collection to fetch multiflow stories for a bot and returns the result as a YAML string.Excluding metadata fields like
id
,bot
,user
,timestamp
,status
from the YAML output is reasonable.No issues observed.
Line range hint
193-203
: Theload_bot_content
method looks good.It handles loading bot content correctly, checking the
enable_faq
flag in bot settings to determine if content should be loaded.Delegating to
self.__prepare_cognition_data_for_bot()
to actually fetch the content is good for modularity and keeping this method concise.No issues observed.
Line range hint
205-255
: The__prepare_cognition_data_for_bot
method looks good.It handles fetching cognition data for a bot correctly. Querying the
CognitionSchema
collection first to get the schema for each collection, and then fetching the corresponding data fromCognitionData
is a reasonable approach.The aggregated result format as a list of dictionaries seems appropriate and easy to work with.
No issues observed.
Line range hint
257-268
: Theget_config_templates
method looks good.It handles fetching the list of available config templates correctly. Using
Utility.list_files()
is a reasonable way to get the list of config files.Reading each config file using
read_config_file()
and returning the config data along with the template name is appropriate.No issues observed.
Line range hint
270-289
: Thedelete_bot_data
method looks good.It provides a way to delete different parts of bot data based on the
what
argument, which is useful.Delegating the actual deletion to separate methods for each type of data (domain, stories, NLU, config, rules, actions, multiflow stories, bot content) is good for modularity and keeping this method concise.
No issues observed.
Line range hint
291-300
: Thesave_nlu
method looks good.It handles saving NLU training data correctly by calling separate methods to save training examples, entity synonyms, lookup tables and regex features.
Delegating the saving of each type of NLU data to separate methods is good for modularity and keeping this method concise.
No issues observed.
Line range hint
302-311
: Thedelete_nlu
method looks good.It handles deleting NLU training data correctly by calling
Utility.hard_delete_document()
to delete the relevant collections (TrainingExamples
,EntitySynonyms
,LookupTables
,RegexFeatures
).No issues observed.
tests/unit_test/data_processor/data_processor_test.py (3)
1139-1158
: LGTM!The new test method
test_list_live_agent_actions
is well-structured and thoroughly tests the functionality of loading and listing live agent actions using theMongoProcessor
. The assertions cover the expected structure and content of the loaded and listed data.This addition aligns with the PR objective of enhancing the live agent feature and improves the test coverage.
3588-3588
: LGTM!The modification to the expected output in the
test_download_data_files_with_actions
method correctly includeslive_agent_action
in the set of actions being tested. This change ensures that the test method verifies the inclusion oflive_agent_action
in the downloaded data files.This update aligns with the PR objective of enhancing the live agent feature and maintains the accuracy of the test.
3625-3625
: LGTM!The modification to the expected output in the
test_load_action_configurations
method correctly includeslive_agent_action
in the action configurations being tested. This change ensures that the test method verifies the inclusion oflive_agent_action
in the loaded action configurations.This update aligns with the PR objective of enhancing the live agent feature and maintains the accuracy of the test.
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.
approved
optional bot_response in callback action (log failure for no bot_response fix)
added live agent in upload/download
Summary by CodeRabbit
New Features
Bug Fixes
Tests