Add Session.wait_for_schema_agreement to replace ControlConnection.wait_for_schema_agreement#877
Merged
dkropachev merged 2 commits intoscylladb:masterfrom May 8, 2026
Conversation
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
60de2cb to
7669675
Compare
dkropachev
changed the title
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
837644c to
30fd542
Compare
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
30fd542 to
25e148d
Compare
There was a problem hiding this comment.
Pull request overview
Adds a session-scoped schema agreement API (Session.wait_for_schema_agreement) and transitions existing call sites away from the control-connection implementation, addressing failures when the control connection is unhealthy/closed and improving correctness when schema changes are executed on non-control-connection hosts.
Changes:
- Introduces
Session.wait_for_schema_agreement(wait_time=None, scope=...)that checkssystem.local.schema_versionvia host-targeted session queries across connected hosts. - Deprecates
ControlConnection.wait_for_schema_agreement()into a compatibility wrapper and moves logic to_wait_for_schema_agreement()for internal metadata-refresh flows. - Updates docs and unit/integration tests to validate the new session behavior and migrate callers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cassandra/cluster.py |
Adds Session.wait_for_schema_agreement with scoped host selection; deprecates ControlConnection.wait_for_schema_agreement in favor of _wait_for_schema_agreement. |
docs/api/cassandra/cluster.rst |
Documents Session.wait_for_schema_agreement in the public API docs. |
tests/unit/test_session_schema_agreement.py |
Adds focused unit tests for session-scoped schema agreement behavior. |
tests/unit/test_control_connection.py |
Updates tests to use _wait_for_schema_agreement internally and asserts deprecation warning behavior on the public wrapper. |
tests/unit/test_cluster.py |
Adds unit coverage for host selection scopes and retry/error behavior in Session.wait_for_schema_agreement. |
tests/integration/standard/test_udts.py |
Migrates integration usage to Session.wait_for_schema_agreement. |
tests/integration/long/test_schema.py |
Migrates integration usage to Session.wait_for_schema_agreement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lorak-mmk
requested changes
May 7, 2026
Lorak-mmk
left a comment
Lorak-mmk
left a comment
There was a problem hiding this comment.
You added type hints for some arguments, but not all. Please make sure all arguments in cluster.py are typed.
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
3 times, most recently
from
12e789c to
539c714
Compare
Collaborator
|
I believe last two commits should be squashed into previous commits to make the history clean |
Collaborator
Author
fixed |
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
539c714 to
0140afc
Compare
Lorak-mmk
requested changes
May 7, 2026
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
0140afc to
13bd713
Compare
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
13bd713 to
0d8250e
Compare
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
0d8250e to
c2c3c1c
Compare
Lorak-mmk
requested changes
May 8, 2026
Lorak-mmk
left a comment
Lorak-mmk
left a comment
There was a problem hiding this comment.
Looks good, just 2 small comments.
Add Session.wait_for_schema_agreement() as a session-scoped schema agreement check. The new API queries schema_version from system.local on the connected hosts selected by the requested rack, dc, or cluster scope, respects Cluster.max_schema_agreement_wait and the control-connection metadata timeouts, and bounds the fan-out with configurable parallelism. Update the public Session docs and switch the integration callers that were explicitly waiting on schema agreement to use the session API. Add unit coverage for agreement, retries, busy connections, missing pools, batching, scope filtering, and invalid scope handling.
…ment Keep ControlConnection.wait_for_schema_agreement() as a compatibility wrapper, but move the existing implementation to _wait_for_schema_agreement() and deprecate the public method in favor of Session.wait_for_schema_agreement(). This lets the control-connection refresh path continue using the old logic internally without emitting warnings. The control-connection wait path was designed for internal metadata refresh use, not as a user-facing schema agreement API. It observes schema agreement from one single node, assuming that schema change statement have been ran on that host. Using it by users will lead to false positives, if user ran statement on a host different from host of control connection. Update the unit tests to call the internal helper everywhere a warning is not expected, add explicit deprecation coverage for the public wrapper, and set stacklevel=2 so the warning points at the caller instead of inside the driver.
dkropachev
force-pushed
the
dk/session-wait-for-schema-agreement-api
branch
from
c2c3c1c to
d050c34
Compare
Lorak-mmk
approved these changes
May 8, 2026
sylwiaszunejko
approved these changes
May 8, 2026
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
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.
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.Summary
Fixes: #604
This change adds a session-scoped schema agreement API and moves callers over to it.
Session.wait_for_schema_agreement()queriesschema_versionfromsystem.localon the connected hosts selected by rack, dc, or cluster scope. It uses regular host-targeted session queries, respectsCluster.max_schema_agreement_waitand the control-connection timeout, and retries until the outer schema-agreement deadline expires.ControlConnection.wait_for_schema_agreement()is kept as a compatibility wrapper, but the implementation now lives under_wait_for_schema_agreement()and the public method is deprecated in favor ofSession.wait_for_schema_agreement(). The control-connection wait path is meant for internal metadata refresh use, not as a user-facing schema agreement API. It observes schema agreement from a single node, assuming the schema-changing statement was run on that host. Using it directly from user code can therefore produce false positives when the statement was run on a different host than the control connection.The integration callers that explicitly waited on schema agreement now use the session API.
Testing
Focused unit coverage passed for:
tests/unit/test_control_connection.pytests/unit/test_session_schema_agreement.pytests/unit/test_cluster.pyThat focused run currently reports
49 passed, 28 skipped.I did not rerun the full repo-ci workflow after the latest simplification.
Integration scenarios
The integration call sites updated in this series are:
tests/integration/long/test_schema.pytests/integration/standard/test_udts.pyThose scenarios were not run locally in this branch.