Follow-up to #105171, which added `ruff` with a deliberately narrow,
bug-class-only rule set and left the unused-name and star-import rules
off pending a dedicated cleanup pass. This enables that second batch in
`pyproject.toml` and fixes every violation across the tree (the bulk in
integration tests):
- `F401` unused-import and `F841` unused-variable: removed, preserving
any side-effecting right-hand side (e.g. `result = node.query(...)`
becomes `node.query(...)`). Two `try: import ...` feature probes are
kept with `# noqa: F401`.
- `F541` f-string-missing-placeholders: dropped the redundant `f` prefix.
- `F403`/`F405` star imports: every `from module import *` is expanded
into explicit imports of the names actually used, taken from each
name's real origin, so stdlib/third-party names import from their real
source rather than through a test "convenience bundle". The now-empty
`tests/integration/helpers/kafka/common_direct.py` bundle is removed and
its three remaining consumers import `_VarintBytes`/`KafkaAdminClient`
directly.
Eliminating the star imports surfaced a real bug the wildcard had masked
from `F821`: `generate_presets.py` in `test_prometheus_protocols` called
an undefined `execute_instant_query_with_http_api`; fixed to the existing
`execute_query_via_http_api`.
No `black`/`isort` reformatting was applied (neither is enforced in the
style check), so the diff is limited to the lint fixes themselves.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Configures ruff via [tool.ruff] in pyproject.toml and wires it into
the existing style-test job in ci/jobs/check_style.py. The initial
rule set is narrow on purpose - only rules whose every hit is a real
bug or unreachable reference:
- F402 import-shadowed-by-loop-var
- F522/F523 unused .format args
- F601 duplicate dict-literal keys
- F632 is comparison with a literal
- F811 redefinition of an unused name (silently shadowed tests,
duplicate route handlers, etc.)
- F821 undefined name (the PR #105159 class - undefined variable
inside an f-string evaluated only on an error/diagnostic branch,
masking the original failure with NameError)
Stylistic rules (F401, F541, F841, F403, F405) are intentionally
left off for follow-up cleanups so the initial signal is bug-class
only.
Bugs found and fixed by the initial run:
F821 (same pattern as PR #105159):
- ci/jobs/scripts/check_ci.py - commit_status_data referenced in
an exception branch but never assigned
- tests/fuzz/runner.py - PIPE referenced without
`from subprocess import PIPE`
- tests/integration/test_backup_restore_on_cluster_with_checksum_data_file_name/test.py
- os.path.join with os not imported
- tests/integration/test_database_iceberg/test.py - BASE_URL_DOCKER
typo for BASE_URL
- tests/integration/test_keeper_remove_acl/test.py - node_zk
referenced in a finally cleanup that was unreachable; the bare
except silently swallowed the NameError on every run
- tests/integration/test_storage_delta_disks/test.py - missing
`from helpers.config_cluster import minio_secret_key`
F811 silently-shadowed tests (later def overwrites earlier):
- test_disk_types::test_different_types
- test_storage_s3::test_empty_file
- test_executable_table_function - one of the two same-named
functions renamed to test_executable_storage_*
- test_storage_delta::test_partition_columns_3 - second def
renamed to test_partition_columns_jumbled
F811 dead route handler:
- test_merge_tree_s3_failover endpoint had two `def fail_request`;
the throttle one was unreachable, renamed to throttle_request
F601 duplicate dict-literal keys with divergent values in
test_storage_s3_queue/test_5.py - same setting specified twice
with different values; cleaned up to make the intended value
explicit
F522/F523 .format-string drift - templates updated without
dropping the corresponding args (test_jbod_ha,
test_manipulate_statistics, test_mysql_protocol,
test_storage_kafka, helpers/dictionary)
F632 `assert error is ""` -> `==` in test_allow_feature_tier
F402 loop variable named `field` shadowing
`from dataclasses import field` in praktika/issue.py
Plus auto-fixed duplicate imports across CI scripts and
integration tests.
Two cases use `# noqa: F821` for legitimate self-referential type
annotations (TypeVar bound and nested dataclass forward reference)
in praktika/utils.py and praktika/gh.py.
Reference: https://github.com/ClickHouse/ClickHouse/pull/105159
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Otherwise you need to manually allow 25 version:
$ black tests/integration/test_merge_tree_s3/test.py
Oh no! 💥💔💥 The required version `24` does not match the running version `25.1.0`!
But, maybe someone will dislike this?
