Skip to content

Better timeout options handling in wait started #451

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.

Sign up for GitHub

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

Merged
aalexfvk merged 4 commits into from
Mar 17, 2026
Merged

Better timeout options handling in wait started #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54ffc25
Better timeout options handling in wait started
kirillgarbar Mar 2, 2026
c887047
Fix mypy
kirillgarbar Mar 2, 2026
0e6233d
Fix constraints
kirillgarbar Mar 2, 2026
06fd4ec
Merge remote-tracking branch 'origin' into wait-started
kirillgarbar Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions ch_tools/chadmin/cli/wait_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,12 @@ def wait_replication_sync_command(
help="Time to wait, in seconds. If not set, the timeout is determined dynamically based on chosen timeout strategy.",
)
@constraint(
If(Equal("timeout_strategy", "parts"), then=AcceptAtMost(1), else_=accept_none),
["wait"],
If(Equal("timeout_strategy", "parts"), then=accept_none),
["file_processing_speed", "min_timeout", "max_timeout"],
)
@constraint(
If(Equal("timeout_strategy", "files"), then=AcceptAtMost(3), else_=accept_none),
["file_processing_speed", "min_timeout", "max_timeout"],
If(Equal("timeout_strategy", "files"), then=AcceptAtMost(4)),
["file_processing_speed", "min_timeout", "max_timeout", "wait"],
)
@constraint(If("track_restart", then=require_all), ["restart_start_time"])
@pass_context
Expand Down Expand Up @@ -410,8 +410,17 @@ def get_timeout_by_files(
Calculate and return timeout by files.
"""
file_processing_speed = file_processing_speed or DEFAULT_FILE_PROCESSING_SPEED
min_timeout = min_timeout or DEFAULT_MIN_TIMEOUT
max_timeout = max_timeout or DEFAULT_MAX_TIMEOUT

if min_timeout is None and max_timeout is None:
min_timeout = DEFAULT_MIN_TIMEOUT
max_timeout = DEFAULT_MAX_TIMEOUT
elif max_timeout and not min_timeout:
min_timeout = min(DEFAULT_MIN_TIMEOUT, max_timeout)
elif min_timeout and not max_timeout:
max_timeout = max(DEFAULT_MAX_TIMEOUT, min_timeout)

# For mypy
assert min_timeout and max_timeout

file_processing_timeout = get_file_count() // file_processing_speed
return max(min_timeout, min(file_processing_timeout, max_timeout))
Expand Down
11 changes: 0 additions & 11 deletions tests/unit/chadmin/test_wait_started.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,6 @@ def test_cli_parts_strategy_rejects_file_options(
assert result.exit_code != 0


def test_cli_files_strategy_rejects_wait_option(
cli_runner: CliRunner, cli_context: dict
) -> None:
result = cli_runner.invoke(
wait_group,
["started", "--timeout-strategy", "files", "--wait", "100"],
obj=cli_context,
)
assert result.exit_code != 0


@patch("ch_tools.chadmin.cli.chadmin_group.logging")
@patch("ch_tools.chadmin.cli.wait_group.is_clickhouse_alive")
@patch("ch_tools.chadmin.cli.wait_group.warmup_system_users")
Expand Down
Loading