-
Notifications
You must be signed in to change notification settings - Fork 49
Update Scylla version in CI #509
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
Conversation
"test_materialized_view_metadata_alter" checks which strategy is used on a view. The test did not set the strategy explicitly, instead relying on defaults. This default has changed in Scylla, causing the test to fail.
Scylla 2025.1 deprecated compact storage, and disabled in by default, which caused tests to fail. We should start expecting them to fail on version 2025.1 and later.
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.
Pull Request Overview
This PR updates the ScyllaDB version used in CI from 6.2 to 2025.2 and addresses test failures caused by breaking changes in ScyllaDB 2025.1/2025.2. The main changes accommodate ScyllaDB's deprecation of compact storage and changes to the default compaction strategy.
- Introduces a new
requirescompactstorage
decorator to mark tests that need compact storage support - Updates materialized view creation to explicitly specify compaction strategy
- Upgrades CI to use ScyllaDB 2025.2
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/integration/init.py | Adds new decorator and utility function for handling ScyllaDB version-specific test skipping |
tests/integration/standard/test_metadata.py | Applies compact storage decorator to affected tests and updates materialized view with explicit compaction strategy |
.github/workflows/integration-tests.yml | Updates ScyllaDB version from 6.2 to 2025.2 in CI configuration |
@@ -297,7 +305,7 @@ def _id_and_mark(f): | |||
requiresmallclockgranularity = unittest.skipIf("Windows" in platform.system() or "asyncore" in EVENT_LOOP_MANAGER, | |||
"This test is not suitible for environments with large clock granularity") | |||
requiressimulacron = unittest.skipIf(SIMULACRON_JAR is None or CASSANDRA_VERSION < Version("2.1"), "Simulacron jar hasn't been specified or C* version is 2.0") | |||
|
|||
requirescompactstorage = xfail_scylla_version(lambda v: v >= Version('2025.1.0'), reason="ScyllaDB deprecated compact storage", raises=InvalidRequest) |
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.
The InvalidRequest
exception class is used but not imported. This will cause a NameError at runtime when the decorator is applied.
Copilot uses AI. Check for mistakes.
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.
This bot is dump, this exception is definitely imported:
from cassandra import OperationTimedOut, ReadTimeout, ReadFailure, WriteTimeout, WriteFailure, AlreadyExists,\
InvalidRequest
"WITH compaction = {{ 'class' : 'SizeTieredCompactionStrategy' }}".format( | ||
self.keyspace_name, self.function_table_name) |
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.
The string formatting is incorrect. The .format()
call has no placeholders to substitute, and the double braces will result in literal braces in the SQL. This should likely be a simple string concatenation or the .format()
call should be removed.
"WITH compaction = {{ 'class' : 'SizeTieredCompactionStrategy' }}".format( | |
self.keyspace_name, self.function_table_name) | |
"WITH compaction = {{ 'class' : 'SizeTieredCompactionStrategy' }}" |
Copilot uses AI. Check for mistakes.
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.
And again the bot is dumb. Double braces are a mean to escape the braces so they are put literally. There are 2 format args, and 2 placeholders - 0 and 1. I don't see a bug here.
This PR fixes the tests failures that started to appear in 2025.1 / 2025.2.
There were 2 breaking Scylla changes that needed fixing:
Pre-review checklist
I added relevant tests for new features and bug fixes.I have provided docstrings for the public items that I want to introduce.I have adjusted the documentation in./docs/source/
.I added appropriateFixes:
annotations to PR description.