-
Notifications
You must be signed in to change notification settings - Fork 487
chore(deps): Upgrade Neo4j Python driver to 6.0.0 #2340
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BREAKING CHANGE: Neo4j Python driver 6.0.0 removes legacy transaction methods. This upgrade migrates to the new API: - read_transaction → execute_read - write_transaction → execute_write Neo4j database 4.x may still work but is not guaranteed. We recommend using Neo4j 5.x or higher. Closes #1979 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Jeremy Chapeau <jeremy@subimage.io>
Contributor
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.
3 issues found across 19 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="tests/unit/cartography/client/test_core.py">
<violation number="1" location="tests/unit/cartography/client/test_core.py:19">
P1: Rule violated: **Tests and documentation quality**
This test still asserts a mocked session call count (`execute_write.assert_not_called()`), which violates the rule to test outcomes in the graph rather than mock interactions. Refactor the test to validate graph state (e.g., using check_nodes/check_rels) or remove the mock-based assertion per the rule’s 'Test outcomes, not implementation' clause.</violation>
</file>
<file name="tests/unit/driftdetect/test_detector.py">
<violation number="1" location="tests/unit/driftdetect/test_detector.py:41">
P1: Rule violated: **Tests and documentation quality**
Tests should verify outcomes (graph/state data) rather than asserting mock call parameters. The added `execute_read.assert_called_with(...)` lines are mock-parameter assertions, which this rule explicitly forbids. Replace these with assertions on the resulting state/graph data instead.</violation>
</file>
<file name="tests/unit/cartography/intel/aws/test_resourcegroupstaggingapi.py">
<violation number="1" location="tests/unit/cartography/intel/aws/test_resourcegroupstaggingapi.py:93">
P1: Rule violated: **Tests and documentation quality**
Test asserts a mock call count instead of verifying graph outcomes. The rule "Test outcomes, not implementation" requires tests to validate graph data (e.g., via check_nodes/check_rels) rather than mock call counts.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
kunaals
approved these changes
Feb 4, 2026
Collaborator
kunaals
left a comment
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.
lgtm
6 tasks
10 tasks
jychp
pushed a commit
that referenced
this pull request
Feb 6, 2026
### Type of change <!-- Mark the relevant option with an "x" --> - [x] Bug fix (non-breaking change that fixes an issue) ### Summary Use `session.execute_read(...)` for AWS EC2 images/snapshots read helpers so reads go through managed transactions and Cartography's existing retry logic. This avoids transient `SessionExpired` failures during long Aura syncs and aligns with the driver API migration in PR #2340. Redacted error observed: ``` SessionExpired: Failed to read from defunct connection <aura-host>:7687 Connection reset by peer ``` Relevant docs: - https://neo4j.com/docs/python-manual/current/transactions/ (managed transactions retry transient errors) - https://neo4j.com/docs/python-manual/current/query-advanced/ (implicit transactions do not retry) - https://graphacademy.neo4j.com/courses/drivers-python/3-in-production/1-transaction-management/ (managed transaction semantics) ### Related issues or links <!-- Include links to relevant issues or other pages. Use "Fixes #123" or "Closes #123" to auto-close issues. --> - #2340 ### Breaking changes <!-- If this PR introduces breaking changes, describe the impact and migration path. Otherwise, delete this section. --> None. ### How was this tested? <!-- Describe how you tested your changes. Include relevant details such as test configuration, commands run, or manual testing steps. --> - Standard integration tests ### Checklist #### General - [x] I have read the [contributing guidelines](https://cartography-cncf.github.io/cartography/dev/developer-guide.html). - [x] The linter passes locally (`make lint`). - [x] I have added/updated tests that prove my fix is effective or my feature works. #### Proof of functionality <!-- Provide at least one of the following to help reviewers verify your changes: --> - [ ] Screenshot showing the graph before and after changes. - [ ] New or updated unit/integration tests. #### If you are adding or modifying a synced entity - [ ] Included Cartography sync logs from a real environment demonstrating successful synchronization of the new/modified entity. Logs should show: - The sync job starting and completing without errors - The number of nodes/relationships created or updated - Example: ``` INFO:cartography.intel.aws.ec2:Loading 42 EC2 instances for region us-east-1 INFO:cartography.intel.aws.ec2:Synced EC2 instances in 3.21 seconds ``` #### If you are changing a node or relationship - [ ] Updated the [schema documentation](https://github.com/cartography-cncf/cartography/tree/master/docs/root/modules). - [ ] Updated the [schema README](https://github.com/cartography-cncf/cartography/blob/master/docs/schema/README.md). #### If you are implementing a new intel module - [ ] Used the NodeSchema [data model](https://cartography-cncf.github.io/cartography/dev/writing-intel-modules.html#defining-a-node). ### Notes for reviewers This completes the managed-transaction API migration for two AWS EC2 read helpers that still invoked `read_list_of_values_tx` directly with a `Session`, which bypassed the retry path. PR #2340 updated the driver APIs overall but did not touch these call sites. Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Summary
Upgrade the Neo4j Python driver from 5.x to 6.0.0. The driver 6.0.0 removes legacy transaction methods, requiring migration to the new API:
read_transaction→execute_readwrite_transaction→execute_writeThis change updates all usages across the codebase, including source files, tests, and documentation.
Related issues or links
Breaking changes
Neo4j database compatibility:
How was this tested?
Checklist
General
make lint).Proof of functionality
Notes for reviewers
This is a straightforward API migration. The new methods (
execute_read/execute_write) have the same signatures as the old ones, so the changes are purely mechanical replacements.