Skip to content

Commit

Permalink
feature(pre-commit): replace pylint with ruff scylladb#5799
Browse files Browse the repository at this point in the history
Ruff is rust base linter for python that work incredibly fast
since pylint speed is causing use issue on SCT CI
we should start using ruff,
for now it uses the pylint set of rules it has implemeted
and we can slow extand it's configuration to use more
of the available rules set it has

Ref: https://github.com/charliermarsh/ruff
Ref: https://www.youtube.com/watch?v=jeoL4qsSLbE
  • Loading branch information
fruch authored and dimakr committed Jun 18, 2024
1 parent e8ad1ef commit e55ca39
Show file tree
Hide file tree
Showing 75 changed files with 476 additions and 487 deletions.
7 changes: 3 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ repos:
types: [python]
exclude: '\.sh$'

- id: pylint
name: pylint
entry: pylint -j 2 -d consider-using-f-string
- id: ruff
name: ruff
entry: ruff check --fix
language: system
exclude: ^docker/alternator-dns/.*$
types: [python]

- repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
Expand Down
4 changes: 2 additions & 2 deletions docker/alternator-dns/dns_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@


def livenodes_update():
global alternator_port
global livenodes
global alternator_port # noqa: PLW0602
global livenodes # noqa: PLW0603
while True:
# Contact one of the already known nodes by random, to fetch a new
# list of known nodes.
Expand Down
2 changes: 1 addition & 1 deletion docker/env/version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.69-update-scylla-driver-3.26.8
1.70-introduce-ruff
2 changes: 1 addition & 1 deletion functional_tests/scylla_operator/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def publish_test_result():

@pytest.fixture(autouse=True, scope='package', name="tester")
def fixture_tester() -> ScyllaOperatorFunctionalClusterTester:
global TESTER # pylint: disable=global-statement
global TESTER # pylint: disable=global-statement # noqa: PLW0603
os.chdir(sct_abs_path())
tester_inst = ScyllaOperatorFunctionalClusterTester()
TESTER = tester_inst # putting tester global, so we can report skipped test (one with mark.skip)
Expand Down
2 changes: 1 addition & 1 deletion longevity_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def chunks(_list, chunk_size):
self._pre_create_templated_user_schema(batch_start=extra_tables_idx,
batch_end=extra_tables_idx+num_of_newly_created_tables)
for i in range(num_of_newly_created_tables):
batch += self.create_templated_user_stress_params(extra_tables_idx + i, cs_profile=cs_profile)
batch.append(self.create_templated_user_stress_params(extra_tables_idx + i, cs_profile=cs_profile))

nodes_ips = self.all_node_ips_for_stress_command
for params in batch:
Expand Down
1 change: 1 addition & 0 deletions mgmt_cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,7 @@ def test_repair_multiple_keyspace_types(self): # pylint: disable=invalid-name
keyspace_repair_percentage = per_keyspace_progress.get(keyspace_name, None)
assert keyspace_repair_percentage is not None, \
"The keyspace {} was not included in the repair!".format(keyspace_name)

assert keyspace_repair_percentage == 100, \
"The repair of the keyspace {} stopped at {}%".format(
keyspace_name, keyspace_repair_percentage)
Expand Down
5 changes: 2 additions & 3 deletions mgmt_upgrade_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,8 @@ def validate_previous_task_details(task, previous_task_details):
# and as a result it could be a BIT imprecise
if abs(delta.total_seconds()) > 60:
mismatched_details_name_list.append(detail_name)
else:
if current_value != previous_task_details[detail_name]:
mismatched_details_name_list.append(detail_name)
elif current_value != previous_task_details[detail_name]:
mismatched_details_name_list.append(detail_name)
complete_error_description = _create_mismatched_details_error_message(previous_task_details,
current_task_details,
mismatched_details_name_list)
Expand Down
2 changes: 1 addition & 1 deletion performance_regression_row_level_repair_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def preload_data(self, consistency_level=None):

for stress_cmd in prepare_write_cmd:
if consistency_level:
stress_cmd = self._update_cl_in_stress_cmd(
stress_cmd = self._update_cl_in_stress_cmd( # noqa: PLW2901
str_stress_cmd=stress_cmd, consistency_level=consistency_level)
params.update({'stress_cmd': stress_cmd})

Expand Down
16 changes: 16 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[tool.ruff]
lint.select = ["PL"]

lint.ignore = ["E501", "PLR2004"]

target-version = "py310"

force-exclude = true
line-length = 240
respect-gitignore = true


[tool.ruff.lint.pylint]
max-args = 12
max-statements = 100
max-branches = 24
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ python-jenkins==1.7.0
ssh2-python==1.0.0
argus-alm==0.12.3
parameterized==0.8.1
pylint==2.11.1 # Needed for pre-commit hooks
ruff==0.4.7 # Needed for pre-commit hooks
autopep8==1.5.7 # Needed for pre-commit hooks
kubernetes==24.2.0
packaging==21.3
Expand Down
441 changes: 213 additions & 228 deletions requirements.txt

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions sct.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def clean_resources(ctx, post_behavior, user, test_id, logdir, dry_run, backend)
@sct_option('--test-id', 'test_id', help='test id to filter by')
@click.option('--verbose', is_flag=True, default=False, help='if enable, will log progress')
@click.pass_context
def list_resources(ctx, user, test_id, get_all, get_all_running, verbose):
def list_resources(ctx, user, test_id, get_all, get_all_running, verbose): # noqa: PLR0912, PLR0915
# pylint: disable=too-many-locals,too-many-arguments,too-many-branches,too-many-statements

add_file_logger()
Expand Down Expand Up @@ -1270,7 +1270,7 @@ def get_test_results_for_failed_test(test_status, start_time):
@click.option('--runner-ip', type=str, required=False, help="Sct runner ip for the running test")
@click.option('--email-recipients', help="Send email to next recipients")
@click.option('--logdir', help='Directory where to find testrun folder')
def send_email(test_id=None, test_status=None, start_time=None, started_by=None, runner_ip=None,
def send_email(test_id=None, test_status=None, start_time=None, started_by=None, runner_ip=None, # noqa: PLR0912
email_recipients=None, logdir=None):
if started_by is None:
started_by = get_username()
Expand Down Expand Up @@ -1475,11 +1475,11 @@ def prepare_regions(cloud_provider, regions):

for region in regions:
if cloud_provider == "aws":
region = AwsRegion(region_name=region)
region = AwsRegion(region_name=region) # noqa: PLW2901
elif cloud_provider == "azure":
region = AzureRegion(region_name=region)
region = AzureRegion(region_name=region) # noqa: PLW2901
elif cloud_provider == "gce":
region = GceRegion(region_name=region)
region = GceRegion(region_name=region) # noqa: PLW2901
else:
raise Exception(f'Unsupported Cloud provider: `{cloud_provider}')
region.configure()
Expand Down
2 changes: 1 addition & 1 deletion sdcm/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def get_audit_log_rows(node, # pylint: disable=too-many-locals
if '!NOTICE' in line[:120] and 'scylla-audit' in line[:120]:
while line[-2] != '"':
# read multiline audit log (must end with ")
line += log_file.readline()
line += log_file.readline() # noqa: PLW2901
audit_data = line.split(': "', maxsplit=1)[-1]
try:
node, cat, consistency, table, keyspace_name, opr, source, username, error = audit_data.split(
Expand Down
25 changes: 12 additions & 13 deletions sdcm/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,15 +956,14 @@ def start_journal_thread(self):
if self._journal_thread:
self.log.debug("Use %s as logging daemon", type(self._journal_thread).__name__)
self._journal_thread.start()
elif logs_transport == 'syslog-ng':
self.log.debug("Use no logging daemon since log transport is syslog-ng")
else:
if logs_transport == 'syslog-ng':
self.log.debug("Use no logging daemon since log transport is syslog-ng")
else:
TestFrameworkEvent(
source=self.__class__.__name__,
source_method='start_journal_thread',
message="Got no logging daemon by unknown reason"
).publish_or_dump()
TestFrameworkEvent(
source=self.__class__.__name__,
source_method='start_journal_thread',
message="Got no logging daemon by unknown reason"
).publish_or_dump()

def start_coredump_thread(self):
self._coredump_thread = CoredumpExportSystemdThread(self, self._maximum_number_of_cores_to_publish)
Expand Down Expand Up @@ -1223,7 +1222,7 @@ def is_port_used(self, port: int, service_name: str) -> bool:
# this is the case output is empty
return False
else:
self.log.error("Error checking for '%s' on port %s: rc:", service_name, port, result)
self.log.error("Error checking for '%s' on port %s: rc: %s", service_name, port, result)
return False
except Exception as details: # pylint: disable=broad-except
self.log.error("Error checking for '%s' on port %s: %s", service_name, port, details)
Expand Down Expand Up @@ -2699,7 +2698,7 @@ def get_nodes_status(self) -> dict[BaseNode, dict]:
if node := node_ip_map.get(node_ip):
nodes_status[node] = {'status': node_properties['state'],
'dc': dc, 'rack': node_properties['rack']}
else:
else: # noqa: PLR5501
if node_ip:
LOGGER.error("Get nodes statuses. Failed to find a node in cluster by IP: %s", node_ip)

Expand Down Expand Up @@ -3461,7 +3460,7 @@ def create_ssl_context(keyfile: str, certfile: str, truststore: str):
ssl_context.load_verify_locations(cafile=truststore)
return ssl_context

def _create_session(self, node, keyspace, user, password, compression, protocol_version, load_balancing_policy=None, port=None,
def _create_session(self, node, keyspace, user, password, compression, protocol_version, load_balancing_policy=None, port=None, # noqa: PLR0913
ssl_context=None, node_ips=None, connect_timeout=None, verbose=True, connection_bundle_file=None):
if not port:
port = node.CQL_PORT
Expand Down Expand Up @@ -5119,7 +5118,7 @@ def kill_stress_thread(self):
if self.nodes and self.nodes[0].is_kubernetes():
for node in self.nodes:
node.remoter.stop()
else:
else: # noqa: PLR5501
if self.params.get("use_prepared_loaders"):
self.kill_cassandra_stress_thread()
else:
Expand Down Expand Up @@ -5171,7 +5170,7 @@ def _parse_cs_summary(lines):
enable_parse = False

for line in lines:
line = line.strip()
line = line.strip() # noqa: PLW2901
if not line:
continue
# Parse loader & cpu info
Expand Down
8 changes: 4 additions & 4 deletions sdcm/cluster_aws.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class AWSCluster(cluster.BaseCluster): # pylint: disable=too-many-instance-attr
Cluster of Node objects, started on Amazon EC2.
"""

def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments
def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments # noqa: PLR0913
services, credentials, cluster_uuid=None,
ec2_instance_type='c6i.xlarge', ec2_ami_username='root',
ec2_user_data='', ec2_block_device_mappings=None,
Expand Down Expand Up @@ -356,7 +356,7 @@ def update_bootstrap(ec2_user_data, enable_auto_bootstrap):
ec2_user_data.replace('--bootstrap false', '--bootstrap true')
else:
ec2_user_data += ' --bootstrap true '
else:
else: # noqa: PLR5501
if '--bootstrap ' in ec2_user_data:
ec2_user_data.replace('--bootstrap true', '--bootstrap false')
else:
Expand Down Expand Up @@ -844,7 +844,7 @@ def ena_support(self) -> bool:

class ScyllaAWSCluster(cluster.BaseScyllaCluster, AWSCluster):

def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments
def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments # noqa: PLR0913
services, credentials, ec2_instance_type='c6i.xlarge',
ec2_ami_username='centos',
ec2_block_device_mappings=None,
Expand Down Expand Up @@ -1050,7 +1050,7 @@ def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint:

class MonitorSetAWS(cluster.BaseMonitorSet, AWSCluster):

def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments
def __init__(self, ec2_ami_id, ec2_subnet_id, ec2_security_group_ids, # pylint: disable=too-many-arguments # noqa: PLR0913
services, credentials, ec2_instance_type='c6i.xlarge',
ec2_block_device_mappings=None,
ec2_ami_username='centos',
Expand Down
2 changes: 1 addition & 1 deletion sdcm/cluster_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def configure_remote_logging(self) -> None:


class AzureCluster(cluster.BaseCluster): # pylint: disable=too-many-instance-attributes
def __init__(self, image_id, root_disk_size, # pylint: disable=too-many-arguments, too-many-locals
def __init__(self, image_id, root_disk_size, # pylint: disable=too-many-arguments, too-many-locals # noqa: PLR0913
provisioners: List[AzureProvisioner], credentials,
cluster_uuid=None, instance_type='Standard_L8s_v3', region_names=None,
user_name='root', cluster_prefix='cluster',
Expand Down
10 changes: 5 additions & 5 deletions sdcm/cluster_gce.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ class GCECluster(cluster.BaseCluster): # pylint: disable=too-many-instance-attr
"""
_gce_service: compute_v1.InstancesClient

def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments
def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments # noqa: PLR0913
cluster_uuid=None, gce_instance_type='n1-standard-1', gce_region_names=None,
gce_n_local_ssd=1, gce_image_username='root', cluster_prefix='cluster',
node_prefix='node', n_nodes=3, add_disks=None, params=None, node_type=None,
Expand Down Expand Up @@ -336,7 +336,7 @@ def _get_disks_struct(self, name, dc_idx):
gce_disk_struct.append(self._get_local_ssd_disk_struct(name=name, index=i, dc_idx=dc_idx))
if self._add_disks:
for disk_type, disk_size in self._add_disks.items():
disk_size = int(disk_size)
disk_size = int(disk_size) # noqa: PLW2901
if disk_size:
gce_disk_struct.append(self._get_persistent_disk_struct(name=name, disk_size=disk_size,
disk_type=disk_type, dc_idx=dc_idx))
Expand Down Expand Up @@ -542,7 +542,7 @@ def add_nodes(self, count, ec2_user_data='', dc_idx=0, rack=0, enable_auto_boots

class ScyllaGCECluster(cluster.BaseScyllaCluster, GCECluster):

def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments
def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments # noqa: PLR0913
gce_instance_type='n1-standard-1', gce_n_local_ssd=1,
gce_image_username='centos',
user_prefix=None, n_nodes=3, add_disks=None, params=None, gce_datacenter=None, service_accounts=None):
Expand Down Expand Up @@ -578,7 +578,7 @@ def _wait_for_preinstalled_scylla(node):

class LoaderSetGCE(cluster.BaseLoaderSet, GCECluster):

def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments
def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments # noqa: PLR0913
gce_instance_type='n1-standard-1', gce_n_local_ssd=1,
gce_image_username='centos',
user_prefix=None, n_nodes=10, add_disks=None, params=None, gce_datacenter=None):
Expand Down Expand Up @@ -609,7 +609,7 @@ def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_s

class MonitorSetGCE(cluster.BaseMonitorSet, GCECluster):

def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments
def __init__(self, gce_image, gce_image_type, gce_image_size, gce_network, gce_service, credentials, # pylint: disable=too-many-arguments # noqa: PLR0913
gce_instance_type='n1-standard-1', gce_n_local_ssd=1,
gce_image_username='centos', user_prefix=None, n_nodes=1,
targets=None, add_disks=None, params=None, gce_datacenter=None,
Expand Down
2 changes: 1 addition & 1 deletion sdcm/cluster_k8s/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2783,7 +2783,7 @@ def _get_rack_nodes(self, rack: int, dc_idx: int) -> list:
return sorted(
[node for node in self.nodes if node.rack == rack and node.dc_idx == dc_idx], key=lambda n: n.name)

def add_nodes(self, # pylint: disable=too-many-locals,too-many-branches
def add_nodes(self, # pylint: disable=too-many-locals,too-many-branches noqa: PLR0913
count: int,
ec2_user_data: str = "",
# NOTE: 'dc_idx=None' means 'create %count% nodes on each K8S cluster'
Expand Down
4 changes: 2 additions & 2 deletions sdcm/cluster_k8s/eks.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class EksNodePool(CloudK8sNodePool):
disk_type: Literal["standard", "io1", "io2", "gp2", "gp3", "sc1", "st1"]

# pylint: disable=too-many-arguments,too-many-locals
def __init__(
def __init__( # noqa: PLR0913
self,
k8s_cluster: 'EksCluster',
name: str,
Expand Down Expand Up @@ -340,7 +340,7 @@ class EksCluster(KubernetesCluster, EksClusterCleanupMixin): # pylint: disable=
short_cluster_name: str

# pylint: disable=too-many-arguments
def __init__(self,
def __init__(self, # noqa: PLR0913
eks_cluster_version,
ec2_security_group_ids,
ec2_subnet_ids,
Expand Down
4 changes: 2 additions & 2 deletions sdcm/cluster_k8s/gke.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class GkeNodePool(CloudK8sNodePool):
k8s_cluster: 'GkeCluster'

# pylint: disable=too-many-arguments
def __init__(
def __init__( # noqa: PLR0913
self,
k8s_cluster: 'KubernetesCluster',
name: str,
Expand Down Expand Up @@ -267,7 +267,7 @@ class GkeCluster(KubernetesCluster):
pools: Dict[str, GkeNodePool]

# pylint: disable=too-many-arguments,too-many-locals
def __init__(self,
def __init__(self, # noqa: PLR0913
gke_cluster_version,
gke_k8s_release_channel,
gce_disk_size,
Expand Down
5 changes: 2 additions & 3 deletions sdcm/cluster_k8s/mini_k8s.py
Original file line number Diff line number Diff line change
Expand Up @@ -607,9 +607,8 @@ def on_deploy_completed(self): # pylint: disable=too-many-branches
images_to_cache.extend(self.cert_manager_images)
if self.params.get("k8s_local_volume_provisioner_type") != 'static':
images_to_cache.append(self.dynamic_local_volume_provisioner_image)
else:
if provisioner_image := self.static_local_volume_provisioner_image:
images_to_cache.append(provisioner_image)
elif provisioner_image := self.static_local_volume_provisioner_image:
images_to_cache.append(provisioner_image)
if self.params.get("k8s_use_chaos_mesh"):
chaos_mesh_version = ChaosMesh.VERSION
if not chaos_mesh_version.startswith("v"):
Expand Down
4 changes: 2 additions & 2 deletions sdcm/coredump.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def update_coredump_info_with_more_information(self, core_info: CoreDumpInfo):
#
# Coredump could be absent when file was removed
for line in coredump_info:
line = line.strip()
line = line.strip() # noqa: PLW2901
if line.startswith('Executable:'):
executable = line[12:].strip()
elif line.startswith('Command Line:'):
Expand All @@ -407,7 +407,7 @@ def update_coredump_info_with_more_information(self, core_info: CoreDumpInfo):
# Storage: /var/lib/systemd/coredump/core.vi.1000.6c4de4c206a0476e88444e5ebaaac482.18554.1578994298000000.lz4 (inaccessible)
if "inaccessible" in line:
continue
line = line.replace('(present)', '')
line = line.replace('(present)', '') # noqa: PLW2901
corefile = line[line.find(':') + 1:].strip()
elif line.startswith('Timestamp:'):
timestring = None
Expand Down
Loading

0 comments on commit e55ca39

Please sign in to comment.