-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Set the path where all memories are saved #542
base: main
Are you sure you want to change the base?
Conversation
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.
Currently we have default values in storage classes, like LTMSQLiteStorage
. We should probably adjust the pipeline to move defaults up then, otherwise we'll have multiple places with default values.
I agree with you. Do you want me to do this as part of this PR ? |
@arnaudgelas that would be awesome! Happy to streamline the review process. |
042d3a7
to
39ecfbb
Compare
This PR is stale because it has been open for 45 days with no activity. |
Hey @arnaudgelas, thanks for the Pull Request. Can you take a moment to check if this is still relevant, I'm saying because there were a lot of changes in regards to the Memory part of the project. Also, there are a lot of conflicts across the PR so maybe would be good to start a new branch from the main or resolve them. Let us know if you need any help and we’ll sort it out together. Thanks! |
Disclaimer: This review was made by a crew of AI Agents. Code Review Comment for PR #542OverviewThis pull request introduces significant improvements to the crewAI project by addressing code quality issues and updating memory storage path configurations. The changes enhance maintainability and promote best practices. Code Quality Findings1. Pre-commit Error Fixes
2. Memory Storage Path Configuration
Historical Context from Related PRsIt would be beneficial to refer to related PRs that may have previously addressed memory management or module structure since these changes build upon those foundations. Referencing those decisions can provide insights into the design choices made. However, due to technical constraints, I couldn't fetch that data directly. Implications for Related FilesA notable impact of these changes will likely be felt in files directly interacting with the
Specific Improvement Suggestions
ConclusionThe modifications proposed in this pull request significantly improve both code quality and memory management functionality. The suggested improvements will further enhance robustness and clarity. Overall, excellent work on navigating these critical areas of the codebase! Let's continue to build on this momentum toward cleaner and more modular code. |
I will work on it next week |
39ecfbb
to
2b25822
Compare
@gvieira @joaomdmoura I am looking forward to reading your reviews/comments |
- Introduced `DatabaseStorage` class in `utilities/paths.py` to encapsulate logic for managing database storage paths. - Supports app-specific storage directories with default or custom configurations. - Ensures consistent handling and directory creation. - Updated `knowledge_storage.py`: - Replaced direct calls to `db_storage_path()` with `DatabaseStorage` usage. - Adjusted initialization to accept `DatabaseStorage` as a parameter. - Updated `kickoff_task_outputs_storage.py`: - Migrated to `DatabaseStorage` for path management. - Simplified constructor by removing hardcoded paths. - Updated `ltm_sqlite_storage.py`: - Integrated `DatabaseStorage` for database path handling. - Enhanced consistency with other storage modules. - Updated `rag_storage.py`: - Refactored to use `DatabaseStorage` for managing storage paths. - Improved maintainability by consolidating path logic. - Removed outdated `db_storage_path()` function and related utilities in `utilities/paths.py`. - Adjusted import paths and parameter handling in all affected modules. - Reduced redundant code and improved modularity of storage path management.
6145b6e
to
b418cb5
Compare
No description provided.