Feat: Add query param support and Made Neo4jOps singleton! #9
Conversation
running queries more efficiently as it is now accepting query parameters
|
Warning Rate Limit Exceeded@amindadgar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 27 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent update ushers in significant improvements to the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- docker-compose.test.yml (1 hunks)
- setup.py (1 hunks)
- tc_neo4j_lib/init.py (1 hunks)
- tc_neo4j_lib/credentials.py (1 hunks)
- tc_neo4j_lib/neo4j_ops.py (4 hunks)
- tc_neo4j_lib/schema.py (1 hunks)
- tests/integration/test_run_queries.py (1 hunks)
- tests/unit/test_credentials.py (1 hunks)
Files skipped from review due to trivial changes (2)
- docker-compose.test.yml
- setup.py
Additional Context Used
Ruff (1)
tc_neo4j_lib/neo4j_ops.py (1)
4-4:
neo4j.Driverimported but unused
Additional comments not posted (5)
tc_neo4j_lib/__init__.py (1)
3-3: The import ofQueryis correctly placed and enhances the usability of the library by exposing theQueryclass directly.tests/unit/test_credentials.py (1)
6-19: The unit tests for credential loading are comprehensive, checking both the presence of necessary keys and the non-null values, which is crucial for the robustness of the credentials management.tc_neo4j_lib/credentials.py (1)
6-45: Theload_neo4j_credentialsfunction is well-implemented with robust error handling for missing environment variables. However, there's a small typo in the documentation: "Parameters" should be corrected to "Parameters".- Parameters + ParametersLikely invalid or redundant comment.
tests/integration/test_run_queries.py (1)
6-56: The integration tests are comprehensive, covering the connection to Neo4j and the execution of single and multiple queries. This ensures that theNeo4jOpsclass functions as expected in a real-world scenario.tc_neo4j_lib/neo4j_ops.py (1)
Line range hint
11-112: The refactoring of theNeo4jOpsclass, including the implementation of the singleton pattern and the enhancements to the_run_queryandrun_queries_in_batchmethods, are well-executed. The error handling in_run_queryis robust, appropriately logging different types of Neo4j errors.
tc_neo4j_lib/schema.py
Outdated
| class Query: | ||
| def __init__(self, query: str, parameters: dict[str, Any]) -> None: | ||
| """ | ||
| A schema for passing neo4j query | ||
|
|
||
| Paramters | ||
| ------------ | ||
| query : str | ||
| the actual query to pass to neo4j driver | ||
| parameters : dict[str, Any] | ||
| the neo4j parameters for the query itself | ||
| """ | ||
| self.query = query | ||
| self.parameters = parameters |
There was a problem hiding this comment.
The Query class is well-defined with clear documentation. However, there's a small typo in the documentation: "Paramters" should be corrected to "Parameters".
- Paramters
+ ParametersCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| class Query: | |
| def __init__(self, query: str, parameters: dict[str, Any]) -> None: | |
| """ | |
| A schema for passing neo4j query | |
| Paramters | |
| ------------ | |
| query : str | |
| the actual query to pass to neo4j driver | |
| parameters : dict[str, Any] | |
| the neo4j parameters for the query itself | |
| """ | |
| self.query = query | |
| self.parameters = parameters | |
| class Query: | |
| def __init__(self, query: str, parameters: dict[str, Any]) -> None: | |
| """ | |
| A schema for passing neo4j query | |
| Parameters | |
| ------------ | |
| query : str | |
| the actual query to pass to neo4j driver | |
| parameters : dict[str, Any] | |
| the neo4j parameters for the query itself | |
| """ | |
| self.query = query | |
| self.parameters = parameters |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
tc_neo4j_lib/neo4j_ops.py (2)
Line range hint
47-61: Enhance error handling in_run_query.The error handling in
_run_querycould be enhanced by re-raising exceptions after logging them, allowing higher-level methods to handle or log them as necessary. This would provide more flexibility in how errors are managed throughout the application.- except TransientError as err: - logging.error("Neo4j transient error!") - logging.error(f"Code: {err.code}, message: {err.message}") - except DatabaseError as err: - logging.error("Neo4J database error") - logging.error(f"Code: {err.code}, message: {err.message}") - except ClientError as err: - logging.error("Neo4j Client Error!") - logging.error(f"Code: {err.code}, message: {err.message}") + except (TransientError, DatabaseError, ClientError) as err: + logging.error(f"Neo4j error: {err.code}, message: {err.message}") + raise
Line range hint
72-113: Optimize batch processing inrun_queries_in_batch.The method
run_queries_in_batchmodifies thequerieslist directly within the loop, which can lead to unexpected behavior and bugs. Consider using a different variable to store the slice for clarity and to avoid modifying the input list.- queries = queries[index : index + session_batch] + batch_queries = queries[index : index + session_batch] + # Use batch_queries in the subsequent code
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tc_neo4j_lib/credentials.py (1 hunks)
- tc_neo4j_lib/neo4j_ops.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- tc_neo4j_lib/credentials.py
Additional comments not posted (1)
tc_neo4j_lib/neo4j_ops.py (1)
18-22: Ensure proper singleton usage by raising an exception in the constructor.The use of an exception to prevent multiple instantiations directly through the constructor is a good practice in implementing the singleton pattern. This ensures that the instance is controlled through the
get_instancemethod.
mypy cannot identify the previous condition results, so it is rasing error that the variable could be `None`.
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tc_neo4j_lib/credentials.py (1 hunks)
- tc_neo4j_lib/neo4j_ops.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- tc_neo4j_lib/credentials.py
Additional comments not posted (3)
tc_neo4j_lib/neo4j_ops.py (3)
14-24: Constructor logic correctly enforces singleton behavior and initializes database connection.
33-46: Database connection method correctly handles credential loading, driver creation, and connectivity verification.
Line range hint
48-72: Query execution method correctly handles parameters and various Neo4j errors.
we were assigning the variable to itself which data loss could happen.
Summary by CodeRabbit
New Features
Queryclass for defining Neo4j query schemas.Bug Fixes
Enhancements
Neo4jOpsclass to enforce a singleton pattern and refactored methods for better query handling.Queryobjects instead of strings.Tests
Versioning