Skip to content

Conversation

@dkropachev
Copy link
Collaborator

This regression was cased by 78f5542 And missed during review.
compression=None is the same as compression=False, but it is valid now.

Fixes: #612

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.
  • 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.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@avikivity
Copy link
Member

Test case?

@dkropachev dkropachev force-pushed the dk/enable-none-compression branch from 5239b6b to 787e07e Compare December 7, 2025 10:02
@dkropachev
Copy link
Collaborator Author

Test case?

Added

@dkropachev dkropachev force-pushed the dk/enable-none-compression branch from 787e07e to 0ff7104 Compare December 7, 2025 10:17
Copy link

Copilot AI left a 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 fixes a regression where compression=None was incorrectly rejected. The fix treats compression=None the same as compression=False by converting it using bool(compression), which evaluates to False when compression is None.

Key Changes:

  • Updated type annotations to accept None as a valid compression value
  • Added logic to convert None to False using bool(compression)
  • Added comprehensive test coverage for the compression=None scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cassandra/cluster.py Updated type annotations for the compression parameter to include None, and added handling logic to convert None to False
tests/unit/test_cluster.py Added comprehensive test cases for compression behavior, including a scenario testing compression=None in test_connection_factory_passes_compression_kwarg

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This regression was cased by 78f5542
And missed during review.
compression=None is the same as compression=False, but it is valid now.
@dkropachev dkropachev force-pushed the dk/enable-none-compression branch from 0ff7104 to c6847d8 Compare December 8, 2025 12:41
@dkropachev dkropachev merged commit 592ebf7 into scylladb:master Dec 8, 2025
20 checks passed
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.

Regression in the compression configuration

3 participants