Extract build mode from relocatable packages for proper timeout configuration #705
+117
−4
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
- Modified ScyllaCluster.__init__ to extract build mode from install_dir when both install_dir and cassandra_version are provided - Moved __set_default_timeouts() call after parent init to ensure scylla_mode is properly set - Added comprehensive tests for build mode extraction with different modes (debug, dev, release) - All existing tests pass Co-authored-by: fruch <[email protected]>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Copilot
AI
changed the title
[WIP] Add support for different build modes in relocatable packages
Extract build mode from relocatable packages for proper timeout configuration
Dec 24, 2025
fruch
reviewed
Dec 25, 2025
tests/test_build_mode_extraction.py
Outdated
| def test_scylla_cluster_init_extracts_mode_from_install_dir(): | ||
| """Test that ScyllaCluster.__init__ extracts build mode from install_dir when cassandra_version is also provided.""" | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: |
Contributor
There was a problem hiding this comment.
there a pytest fixture for this, please use that
Contributor
Author
There was a problem hiding this comment.
Updated to use the tmp_path pytest fixture in commit 38a90c9.
fruch
reviewed
Dec 25, 2025
tests/test_build_mode_extraction.py
Outdated
| """Test that timeouts are correctly set based on build mode.""" | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| test_cases = [ |
Contributor
Author
There was a problem hiding this comment.
Updated to use @pytest.mark.parametrize in commit 3d35f1b. The test now runs as 3 separate test instances (one for each mode: debug, release, dev).
Replaced manual tempfile usage with the tmp_path pytest fixture as suggested in code review. This is the standard pytest approach for temporary directories and is more consistent with other tests in the repository. Co-authored-by: fruch <[email protected]>
| """ | ||
| Test that build modes are correctly extracted from relocatable packages. | ||
| """ | ||
| from pathlib import Path |
Refactored test_scylla_cluster_timeout_settings to use @pytest.mark.parametrize instead of a for loop. This creates 3 separate test instances (one for each mode) which provides better test isolation and clearer output when tests fail. Co-authored-by: fruch <[email protected]>
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.
Plan to add build mode support for relocatable packages
Summary of Changes
Problem: Relocatable packages had
scylla_modehardcoded toNone, which prevented proper support for different build types (debug/dev/release).Solution:
Modified
ScyllaCluster.__init__(ccmlib/scylla_cluster.py):install_dirandcassandra_versionare provided (e.g., when loading a cluster from disk), the build mode is now extracted from the install directory usingscylla_extract_install_dir_and_mode()load_from_repository(), but wasn't being used when loading from diskFixed timeout calculation:
__set_default_timeouts()call to afterCluster.__init__()to ensurescylla_modeis properly set before calculating timeoutsAdded comprehensive tests (tests/test_build_mode_extraction.py):
tmp_pathpytest fixture per code review@pytest.mark.parametrizefor better test isolationImpact: Relocatable packages now properly support different build modes (debug/dev/release), with appropriate timeout values for each mode.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.