feat: add user manager factory pattern and product API enhancements #166
Conversation
- Add user manager factory pattern with SQLite and MySQL backends - Add user manager configuration to MOSConfig - Add product API router and configuration - Add DingDing notification integration - Add notification service utilities - Update OpenAPI documentation
There was a problem hiding this comment.
Pull Request Overview
Adds user manager factory pattern with multiple database backends, product API enhancements for DingDing notifications, and improved streaming functionality. The PR introduces a flexible user management system supporting both SQLite and MySQL backends through factory patterns.
- Implementation of user manager factory pattern with SQLite and MySQL backends
- Integration of DingDing notification service with chat reporting
- Addition of user manager configuration to MOSConfig system
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/memos/mem_user/persistent_factory.py | Factory for creating persistent user managers with SQLite/MySQL backends |
| src/memos/mem_user/mysql_user_manager.py | Complete MySQL user management implementation with SQLAlchemy models |
| src/memos/mem_user/mysql_persistent_user_manager.py | MySQL persistent user manager with configuration storage |
| src/memos/mem_user/factory.py | Basic user manager factory without persistence features |
| src/memos/mem_os/utils/notification_utils.py | Utility functions for DingDing bot notifications |
| src/memos/mem_os/product.py | Enhanced product class with notification integration |
| src/memos/configs/mem_user.py | Configuration models for user manager backends |
| src/memos/configs/mem_os.py | Added user manager configuration to MOSConfig |
| src/memos/api/tools/notification_service.py | Service for loading DingDing bot functions |
| src/memos/api/tools/dinding_report_bot.py | Complete DingDing bot implementation with OSS integration |
| src/memos/api/routers/product_router.py | Updated router with notification integration and streaming fixes |
| src/memos/api/config.py | Added DingDing bot configuration support |
| examples/mem_user/user_manager_factory_example.py | Usage examples for user manager factories |
| docs/openapi.json | Updated OpenAPI schema with new user manager configuration |
Comments suppressed due to low confidence (1)
src/memos/api/config.py:459
- This code block appears to be commented out or removed, but the context suggests it might be accidentally deleted. The internet retriever configuration logic seems incomplete.
)
| config = config_factory.config | ||
|
|
||
| if backend == "sqlite": | ||
| return user_manager_class(db_path=config.db_path, user_id=config.user_id) | ||
| elif backend == "mysql": | ||
| return user_manager_class( | ||
| user_id=config.user_id, | ||
| host=config.host, | ||
| port=config.port, | ||
| username=config.username, | ||
| password=config.password, | ||
| database=config.database, | ||
| charset=config.charset, | ||
| ) | ||
| else: | ||
| raise ValueError(f"Unsupported backend: {backend}") |
There was a problem hiding this comment.
The conditional logic checking backend twice (lines 32 and 38) creates redundant validation. Consider removing the elif/else block at lines 40-51 since the initial validation at line 32-33 already ensures the backend is valid.
| config = config_factory.config | |
| if backend == "sqlite": | |
| return user_manager_class(db_path=config.db_path, user_id=config.user_id) | |
| elif backend == "mysql": | |
| return user_manager_class( | |
| user_id=config.user_id, | |
| host=config.host, | |
| port=config.port, | |
| username=config.username, | |
| password=config.password, | |
| database=config.database, | |
| charset=config.charset, | |
| ) | |
| else: | |
| raise ValueError(f"Unsupported backend: {backend}") | |
| config = config_factory.config.__dict__ | |
| return user_manager_class(**config) |
Copilot uses AI. Check for mistakes.
| return user.user_id | ||
| except IntegrityError: | ||
| session.rollback() | ||
| logger.info(f"failed to create user with name '{user_name}' already exists") |
There was a problem hiding this comment.
The log message has grammatical errors. It should read 'Failed to create user with name '{user_name}' - user already exists' for clarity.
| logger.info(f"failed to create user with name '{user_name}' already exists") | |
| logger.info(f"Failed to create user with name '{user_name}' - user already exists") |
Copilot uses AI. Check for mistakes.
| from datetime import datetime | ||
|
|
There was a problem hiding this comment.
The datetime import is already at the top of the file (line 9). This redundant import inside the function should be removed.
| from datetime import datetime |
Copilot uses AI. Check for mistakes.
src/memos/mem_user/factory.py
Outdated
| if backend == "sqlite": | ||
| return user_manager_class(db_path=config.db_path, user_id=config.user_id) | ||
| elif backend == "mysql": | ||
| return user_manager_class( | ||
| user_id=config.user_id, | ||
| host=config.host, | ||
| port=config.port, | ||
| username=config.username, | ||
| password=config.password, | ||
| database=config.database, | ||
| charset=config.charset, | ||
| ) | ||
| else: | ||
| raise ValueError(f"Unsupported backend: {backend}") |
There was a problem hiding this comment.
Same issue as in persistent_factory.py - the conditional logic checking backend twice creates redundant validation. The elif/else block at lines 40-51 should be removed.
| if backend == "sqlite": | |
| return user_manager_class(db_path=config.db_path, user_id=config.user_id) | |
| elif backend == "mysql": | |
| return user_manager_class( | |
| user_id=config.user_id, | |
| host=config.host, | |
| port=config.port, | |
| username=config.username, | |
| password=config.password, | |
| database=config.database, | |
| charset=config.charset, | |
| ) | |
| else: | |
| raise ValueError(f"Unsupported backend: {backend}") | |
| return user_manager_class(**config) |
Copilot uses AI. Check for mistakes.
| @@ -0,0 +1,422 @@ | |||
| """dingding_report_bot.py""" | |||
There was a problem hiding this comment.
The filename in the docstring has a typo. It should be 'dingding_report_bot.py' instead of 'dinding_report_bot.py'.
| """dingding_report_bot.py""" | |
| """dinding_report_bot.py""" |
Copilot uses AI. Check for mistakes.
Done, I have fix it by coplit |
…emTensor#166) * feat: add user manager factory pattern and product API enhancements - Add user manager factory pattern with SQLite and MySQL backends - Add user manager configuration to MOSConfig - Add product API router and configuration - Add DingDing notification integration - Add notification service utilities - Update OpenAPI documentation * fix: change py to meos_tools * fix: code reorganize
2 participants
Summary: (summary)
Fix: #(issue)
Docs Issue/PR: (docs-issue-or-pr-link)
Reviewer: @(reviewer)
Checklist: