fix: Resolve profiler issue with partial and recursive logger imports #420
Conversation
Resolve issue for profiler with partial and recursive logger imports
WalkthroughThe pull request introduces a standardized logging approach across multiple modules in the Changes
Poem
Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cognee/tasks/repo_processor/get_local_dependencies.py (1)
Line range hint
78-78: Remove commented-out code.The commented-out logging statement is no longer needed since it's been replaced with the new logging approach.
- # logging.warning(f"Failed to analyze code entity {code_entity['name']}: {e}")cognee/tasks/repo_processor/get_source_code_chunks.py (2)
Line range hint
47-53: Consider enhancing error context for better debugging.While the error handling is good, consider including the source code length or first few characters (sanitized) in the warning message. This would help distinguish between truly empty files and parsing failures.
- logger.error(f"Error parsing source code: {e}") + logger.error(f"Error parsing source code (length={len(source_code)}): {e}") - logger.warning("Parsed module has no children (empty or invalid source code).") + logger.warning(f"Parsed module has no children. First 50 chars: {source_code[:50]!r}")
Line range hint
142-157: Consider structuring error handling more specifically.The current error handling could be enhanced in several ways:
- Use specific exception types instead of catching all exceptions
- Add structured logging for better error tracking
- Consider adding debug logs for successful processing
Example improvement:
try: yield data_point if not isinstance(data_point, CodeFile): continue if not data_point.contains: - logger.warning(f"CodeFile {data_point.id} contains no code parts") + logger.warning("Empty CodeFile", extra={ + "file_id": data_point.id, + "file_type": type(data_point).__name__ + }) continue for code_part in data_point.contains: try: yield code_part for source_code_chunk in get_source_code_chunks_from_code_part( code_part, model_name=embedding_model ): yield source_code_chunk - except Exception as e: - logger.error(f"Error processing code part: {e}") + except (ValueError, parso.ParserSyntaxError) as e: + logger.error("Failed to process code part", extra={ + "error": str(e), + "code_part_id": code_part.id + }) - except Exception as e: - logger.error(f"Error processing data point: {e}") + except (TypeError, AttributeError) as e: + logger.error("Failed to process data point", extra={ + "error": str(e), + "data_point_type": type(data_point).__name__ + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/profiling.yaml(1 hunks)cognee/tasks/repo_processor/__init__.py(0 hunks)cognee/tasks/repo_processor/expand_dependency_graph.py(1 hunks)cognee/tasks/repo_processor/extract_code_parts.py(1 hunks)cognee/tasks/repo_processor/get_local_dependencies.py(1 hunks)cognee/tasks/repo_processor/get_source_code_chunks.py(1 hunks)cognee/tasks/repo_processor/top_down_repo_parse.py(1 hunks)
💤 Files with no reviewable changes (1)
- cognee/tasks/repo_processor/init.py
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/profiling.yaml
🧰 Additional context used
🪛 GitHub Actions: test | profiling
cognee/tasks/repo_processor/extract_code_parts.py
[error] Circular import detected: Cannot import 'logger' from 'cognee.tasks.repo_processor' due to circular dependency
🔇 Additional comments (5)
cognee/tasks/repo_processor/extract_code_parts.py (1)
4-6: LGTM! Logging setup looks good.The change from custom logger to Python's built-in logging module resolves the circular dependency issue.
🧰 Tools
🪛 GitHub Actions: test | profiling
[error] Circular import detected: Cannot import 'logger' from 'cognee.tasks.repo_processor' due to circular dependency
cognee/tasks/repo_processor/expand_dependency_graph.py (1)
8-10: LGTM! Logging setup and usage are appropriate.The implementation uses proper log levels:
- Debug for non-critical information
- Warning for potential issues
- Error for exceptions
cognee/tasks/repo_processor/get_local_dependencies.py (1)
13-15: LGTM! Logging setup looks good.The change from custom logger to Python's built-in logging module is implemented correctly.
cognee/tasks/repo_processor/top_down_repo_parse.py (1)
7-9: LGTM! Comprehensive logging implementation.The implementation uses all appropriate log levels:
- Info for general information
- Debug for detailed information
- Warning for potential issues
- Error for exceptions
cognee/tasks/repo_processor/get_source_code_chunks.py (1)
11-11: Excellent improvement to logger initialization!Using
logging.getLogger(__name__)instead of a hardcoded string is the recommended approach. This change:
- Ensures unique logger names based on the module's location in the package hierarchy
- Prevents potential circular dependencies during profiling
- Allows for more granular log configuration
Resolve issue for profiler with partial and recursive logger imports
Summary by CodeRabbit
Chores
logging.getLogger(__name__)Documentation