Skip to content

graph: remove DSE Graph support - #1128

Open
roydahan wants to merge 1 commit into
masterfrom
remove/dse-graph
Open

roydahan wants to merge 1 commit into
masterfrom
remove/dse-graph

Conversation

@roydahan

@roydahan roydahan commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Removes DataStax Enterprise (DSE) Graph support from the driver, with no deprecation period. This supersedes the closed #1110, where review asked to "just drop this logic".

Why

  • ScyllaDB has no graph engine, so execute_graph and the graph-language payload options cannot work against it. Graph on ScyllaDB goes through JanusGraph's own stack, not this API.
  • The fluent API (cassandra.datastax.graph.fluent) could not be imported with any gremlinpython the graph extra allowed (>=3.7.4), because it imports TraversalSideEffects, which newer gremlinpython removed.
  • The DSE Graph integration tests were already deleted (e022378, "Remove DSE tests"), and coverage already excluded this code (coverage: exclude DSE- and Astra-only code from measurement #1113).

What's removed (BREAKING CHANGE)

Public API:

  • The cassandra.datastax.graph packages (graphson, query, types, fluent) and cassandra.graph (dse-driver compat shims)
  • cassandra.cluster.GraphExecutionProfile and cassandra.cluster.GraphAnalyticsExecutionProfile
  • cassandra.cluster.EXEC_PROFILE_GRAPH_DEFAULT, EXEC_PROFILE_GRAPH_SYSTEM_DEFAULT and EXEC_PROFILE_GRAPH_ANALYTICS_DEFAULT; Cluster no longer registers default graph profiles under them
  • Session.execute_graph() and Session.execute_graph_async()
  • The graph extra (gremlinpython)

Internals and housekeeping:

  • The Session helpers _maybe_set_graph_paging, _resolve_execution_profile_options, _transform_params, _target_analytics_master, _on_analytics_master_result and _graph_paging_available
  • The GraphStatement branches in Session._create_response_future() and ResponseFuture._is_keyspace_change_query()
  • Imports in cluster.py that only graph code used (json, int64_pack, HostTargetingStatement, DefaultLoadBalancingPolicy, NeverRetryPolicy)
  • The graph packages in the setuptools package list, the "graph" keyword, and the graph entries in the coverage omit/exclude_also lists
  • gremlinpython and its dependency chain (aiohttp etc.) in the root uv.lock, re-locked with a plain uv lock that only removed packages. The docs build dependency was already dropped in docs: drop unused gremlinpython dependency #1139.
  • The graph unit tests and the graph entries in docs/api/cassandra/cluster.rst; a "Removed" entry under Unreleased in CHANGELOG.rst

One side effect: each default graph profile wrapped the default profile's load-balancing policy in a DefaultLoadBalancingPolicy, so populate/on_up/etc. reached that inner policy several times. With those profiles gone they reach it once. Host distance results don't change.

Left for a follow-up

The DSE schema parsers and graph metadata in cassandra/metadata.py (SchemaParserDSE60/67/68, TableMetadataDSE68, VertexMetadata, EdgeMetadata, KeyspaceMetadata.graph_engine, the if dse_version: block), along with the 'graph' workload check in tests/integration/__init__.py, are removed in #1129. cassandra/datastax/cloud, continuous paging, and the geo/DateRange types are not touched.

Coverage (unit only, scripts/coverage.sh, no SCYLLA_VERSION, same environment)

master (168e479) this PR
TOTAL stmts / miss 18075 / 5194 18056 / 5194
TOTAL cover 67.48% 67.45%
cassandra/cluster.py stmts / miss 4022 / 802 (77%) 4003 / 800 (77%)

The graph methods were already excluded from coverage, so the numbers barely move. The 19 statements removed from cluster.py are imports, constants and default-profile registration that used to run. A second branch run differed by one or two lines.

Verification

  • python -c "import cassandra.cluster, cassandra.metadata, cassandra.query" works. Importing cassandra.graph or cassandra.datastax.graph now raises ModuleNotFoundError.
  • Grepping for the removed names across cassandra, tests, docs and pyproject.toml finds only the CHANGELOG text. pyflakes on the new cluster.py reports nothing, and every from cassandra... import in cassandra/, tests/, benchmarks/ and examples/ still resolves.
  • Unit suite, Python 3.13, Cython build with compress-lz4:
    • CASS_DRIVER_NO_SKIP=1 pytest tests/unit --ignore=tests/unit/column_encryption --ignore=tests/unit/io/test_asyncioreactor.py --ignore=tests/unit/io/test_asyncorereactor.py -k 'not test_deserialize_date_range_month': 1559 passed, 1 deselected.
    • EVENT_LOOP_MANAGER=asyncio CASS_DRIVER_NO_SKIP=1 pytest tests/unit/io/test_asyncioreactor.py: 41 passed.
  • After rebasing onto 2d02599: uv lock --check passes for both lockfiles, and the unit suite (CASS_DRIVER_NO_CYTHON=1 CASS_DRIVER_NO_SKIP=1, Cython-only tests excluded) has the same failure/error set as master in that environment.
  • make -C docs test (Sphinx with -W --keep-going): build succeeded with no warnings (before the rebase).
  • Not run locally: integration tests (no cluster available). This PR's CI covers them. The free-threaded test_execute_query_timeout failure is not from this PR; see the comments below.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes. (N/A: removal only)
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce. (N/A: no new public items)
  • I have adjusted the documentation in ./docs/source/. (The API docs live in ./docs/api/; updated there.)
  • I added appropriate Fixes: annotations to PR description. (No tracking issue)

🤖 Generated with Claude Code

https://claude.ai/code/session_01QYcPjKDEQaQTfxpdfsFjZA

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: QUIET
  • Plan: Essentials
  • Run ID: 1a2d75ea-de76-4cb5-a051-16406a52bedb
📥 Commits

Reviewing files that changed from the base of the PR and between f5a6de9 and 4f6ca93.

⛔ Files ignored due to path filters: 1
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing: 2
  • cassandra/cluster.py
  • tests/unit/test_response_future.py
💤 Files with no reviewable changes: 1
  • tests/unit/test_response_future.py

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

Walkthrough

The change removes DSE Graph support, including graph query and fluent APIs, GraphSON handling, execution profiles, session methods, compatibility modules, and the gremlinpython extra. It also removes related documentation and tests and updates package and coverage configuration.

Priority: Low

Change: Other

Merge Risk: Minimal · up to 4f6ca

Graph support is intentionally removed, and no additional merge-blocking issue is established by the available evidence. Normal checks remain appropriate.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the primary change: removal of DSE Graph support.
Linked Issues check Passed Issue #1110 is closed and supplies historical context only. No active directly linked issue remains, so no linked-issue coding requirements apply.
Out of Scope Changes check Passed The changes remove DSE Graph APIs, implementation modules, tests, documentation, dependency configuration, and coverage configuration. These changes match the stated PR scope. The PR leaves the declar…
Description check Passed The pull request description is complete. It explains the purpose, rationale, breaking changes, scope, follow-up work, coverage, verification, test results, documentation updates, and checklist status…
Full details: Out of Scope Changes check

Explanation

The changes remove DSE Graph APIs, implementation modules, tests, documentation, dependency configuration, and coverage configuration. These changes match the stated PR scope. The PR leaves the declared DSE metadata follow-up and unrelated geo and cloud work unchanged.

Full details: Description check

Explanation

The pull request description is complete. It explains the purpose, rationale, breaking changes, scope, follow-up work, coverage, verification, test results, documentation updates, and checklist status.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

The security/snyk (dkropachev) status ("1 test has failed") is not caused by this PR. Snyk runs its test only when a PR changes the manifest it tracks, which appears to be docs/pyproject.toml. The previous PRs that changed that file, #804 (gremlinpython bump) and #978 (Python version bump), got the identical "1 test has failed" status, and both were merged. PRs that don't touch the manifest report "No manifest changes detected" instead.

This PR only removes a docs dependency (gremlinpython) and re-locks without upgrading anything, so it can't introduce a new finding. Whatever Snyk flags is already present in the docs environment on master. I can't open the Snyk report from here, and I don't have the means to re-run it.


Generated by Claude Code

@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.61%. Comparing base (2d02599) to head (a72f566).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1128      +/-   ##
==========================================
- Coverage   86.77%   86.61%   -0.16%     
==========================================
  Files          44       44              
  Lines       19005    18986      -19     
  Branches     2965     2963       -2     
==========================================
- Hits        16491    16445      -46     
- Misses       1848     1872      +24     
- Partials      666      669       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roydahan

Copy link
Copy Markdown
Collaborator Author

@dkropachev please note it's a breaking change for a functionality we don't know if anyone uses with our driver (even for a Cassandra cluster).

@mykaul

mykaul commented Oct 22, 2026

Copy link
Copy Markdown

This drops gremlinpython, but geomet is still a mandatory runtime dependency (dependencies = ['geomet>=1.1', ...]). With graph gone, nothing in the driver calls it anymore.

geomet is used only by Point/LineString/Polygon.from_wkt() in cassandra/util.py, which is already behind a try/except (_HAS_GEOMET). The only caller of from_wkt inside the driver was cassandra/datastax/graph/graphson.py, which this PR removes. The wire (de)serialization of PointType/LineStringType/PolygonType in cqltypes.py uses struct and doesn't need geomet. The only other users of from_wkt are tests/unit/advanced/test_geometry.py, and those tests already skip when geomet is missing.

Every install currently pulls in geomet, plus click and colorama through it, for a public helper that's rarely used. Could geomet move to an optional extra (e.g. geo = ['geomet>=1.1']), either here or in the follow-up that handles the geo types? If it goes in an extra, geomet should also be added to the dev group so the geometry tests keep running.

Related: #1143 removes the unused pyyaml runtime dependency.

@qodo-scylladb

qodo-scylladb Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Code Review by Qodo


Resolved findings
1. Loader loses its requested graph extra ⊘ Outdated
Description
Removing the graph optional dependency leaves scylla-driver[graph] without a matching extra. The
loader role in scylla-ansible-roles still requests it, so provisioning no longer installs
gremlinpython through that requirement, although pip may only warn rather than fail.
Code

pyproject.toml[35]

-graph = ['gremlinpython>=3.7.4,<4']
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The loader role requests `scylla-driver[graph]`, but the driver no longer publishes that extra.

## Fix Focus Areas
- pyproject.toml[34-39]
- /cross_repos/scylla-ansible-roles/ansible-scylla-loader/tasks/main.yml[48-57]

## Recommended Fix
Coordinate an update to `scylla-ansible-roles`: remove the `scylla-driver[graph]` requirement, and add an explicit `gremlinpython` requirement only if the loader independently needs it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗


Powered by Qodo

Copy link
Copy Markdown
Collaborator Author

CI note: test libev (3.14t) failed on 4f6ca93 in tests/integration/standard/test_cluster.py::ClusterTests::test_execute_query_timeout ("session.execute didn't time out in 10 tries").

This failure isn't caused by this PR:

I'll re-run the failed job once after the current run finishes.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Follow-up on the test_execute_query_timeout failure. I re-ran the failed jobs once: test libev (3.15t) passed, but test libev (3.14t) failed again in the same way, with all 10 tries reporting "DID NOT RAISE OperationTimedOut". So this is a real failure on free-threaded builds, not a one-off. It is not caused by this PR: master has had no driver change since the last green run (77af867). The passing 3.14t job (run 39438057224, Oct 20) and the failing one differ only in the interpreter, CPython 3.14.8 vs 3.14.9 free-threaded. The CI action pin, Scylla release:2026.1.13 and ccm==2.0.5 are identical.

Likely mechanism (from reading the code; I haven't reproduced it locally because I have no cluster):

  • ResponseFuture.__init__ calls _start_timer() at cassandra/cluster.py:7030. That happens before _query() assigns self._connection at cassandra/cluster.py:7450.
  • With request_timeout = sys.float_info.min, the timer has already expired when it is created.
  • With the GIL, the libev thread usually can't service it until the caller has set _connection. Without the GIL, the libev thread can run _on_timeout right away, while _connection is still None.
  • The PYTHON-853 guard (cluster.py:7062) then reschedules the timeout 10 ms later, up to 3 times (about 30 ms).
  • A SELECT against a local node finishes well within that window, so the query succeeds and the test never sees OperationTimedOut.

Proposed fix, as a separate PR (out of this PR's scope): stop letting an already-expired timer be pushed back while the request is still being dispatched. One option is for _on_timeout, when _connection is None, to record that the deadline has passed. _query() would then fail with OperationTimedOut instead of sending once it sees that flag (or sees _time_remaining <= 0). Another option is to start the timeout timer only after _connection is assigned. Either way, the test needs no change.

Nothing in this PR changes that path, so I'm not changing this PR. #603 tracks the test; the fix should go there or in its own PR.


Generated by Claude Code

@dkropachev dkropachev left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkropachev

Copy link
Copy Markdown

@roydahan , please rebase

DataStax Enterprise Graph queries cannot work against ScyllaDB, which has
no graph engine (graph workloads on ScyllaDB go through JanusGraph's own
stack, not this API). The code was already dead weight:

- the fluent API (cassandra.datastax.graph.fluent) could not be imported
  with any gremlinpython the `graph` extra allows (>=3.7.4), because it
  imports TraversalSideEffects, which newer gremlinpython removed;
- the DSE Graph integration tests were already deleted ("Remove DSE
  tests"), and coverage already excluded this code.

Remove it outright, without a deprecation period. This is a breaking
change for anyone importing these names.

Removed:
- the cassandra.datastax.graph package (graphson, query, types, fluent)
  and the cassandra.graph dse-driver compatibility package;
- cassandra.cluster.GraphExecutionProfile and
  GraphAnalyticsExecutionProfile;
- the EXEC_PROFILE_GRAPH_DEFAULT, EXEC_PROFILE_GRAPH_SYSTEM_DEFAULT and
  EXEC_PROFILE_GRAPH_ANALYTICS_DEFAULT constants and the default graph
  profiles Cluster registered under them;
- Session.execute_graph() and execute_graph_async(), plus their private
  helpers (_maybe_set_graph_paging, _resolve_execution_profile_options,
  _transform_params, _target_analytics_master,
  _on_analytics_master_result, _graph_paging_available);
- the GraphStatement branches in Session._create_response_future() and
  ResponseFuture._is_keyspace_change_query();
- imports in cluster.py that only graph code used (json, int64_pack,
  HostTargetingStatement, DefaultLoadBalancingPolicy, NeverRetryPolicy);
- the `graph` extra (gremlinpython), the "graph" keyword, the graph
  packages from the setuptools package list, and the graph coverage
  omit/exclude entries;
- gremlinpython and its dependency chain from uv.lock (the docs build
  dependency was already dropped in #1139);
- graph unit tests (tests/unit/advanced/test_graph.py,
  tests/unit/advanced/test_execution_profile.py and the graph case in
  tests/unit/test_response_future.py) and the API doc entries.

DSE schema parsing and graph metadata in cassandra/metadata.py
(SchemaParserDSE6x, VertexMetadata, EdgeMetadata,
KeyspaceMetadata.graph_engine) are left for a follow-up change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYcPjKDEQaQTfxpdfsFjZA

Copy link
Copy Markdown
Collaborator Author

Rebased onto current master (2d02599) as a single commit, a72f566. The change itself is the same. The only adjustment is in the commit message: the docs gremlinpython removal already landed in #1139, so this commit now touches only the root uv.lock. The unit suite gives the same results as master. uv lock --check passes for both lockfiles, and nothing references the removed names any more.


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants