Add pylint to workflows

.github/workflows/pylint.yml

@@ -0,0 +1,31 @@
+on: push
+jobs:
+  check-permissions:
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'run-no-ci') }}
+    uses: ./.github/workflows/check-permissions.yml
+    with:
+      github-event-name: ${{ github.event_name}}
+  pylint:
+    needs: [ check-permissions ]
+    runs-on: ubuntu-22.04
+    strategy:
+      matrix:
+        python-version: ["3.9"]
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set Up python ${{ matrix.python-version }}
+        uses: actions/setup-python@v3
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install pylint psycopg2 pytest toml aiohttp backoff requests allure-pytest \
+          pytest-httpserver prometheus_client asyncpg PyJWT httpx psutil boto3 mypy_boto3_s3 \
+          zstandard numpy pandas pgvector pytest-lazy-fixture pg8000 websockets
+      - name: Analyzing code for errors with pylint
+        run: |
+          pylint -E --ignored-modules psycopg2 $(git ls-files '*.py')
+      - name: Analyzing code for fixed warnings
+        run: |
+          pylint --disable=all --enable=w1203 $(git ls-files '*.py')

scripts/benchmark_durations.py

@@ -118,18 +118,18 @@ def main(args: argparse.Namespace):
                 logging.info("fetching benchmarks...")
                 cur.execute(BENCHMARKS_DURATION_QUERY, (percentile, interval_days))
                 rows = cur.fetchall()
-    except psycopg2.OperationalError as exc:
-        logging.error("cannot fetch benchmarks duration from the DB due to an error", exc)
+    except psycopg2.OperationalError:
+        logging.error("cannot fetch benchmarks duration from the DB due to an error")
         rows = []
         res = FALLBACK_DURATION
 
     for row in rows:
         pytest_name = f"{row['parent_suite'].replace('.', '/')}/{row['suite']}.py::{row['name']}"
         duration = row["percentile_ms"] / 1000
-        logging.info(f"\t{pytest_name}: {duration}")
+        logging.info("\t%s: %s", pytest_name, duration)
         res[pytest_name] = duration
 
-    logging.info(f"saving results to {output.name}")
+    logging.info("saving results to %s", output.name)
     json.dump(res, output, indent=2)
 
 

scripts/flaky_tests.py

@@ -43,8 +43,8 @@ def main(args: argparse.Namespace):
                 logging.info("fetching flaky tests...")
                 cur.execute(FLAKY_TESTS_QUERY, (interval_days,))
                 rows = cur.fetchall()
-    except psycopg2.OperationalError as exc:
-        logging.error("cannot fetch flaky tests from the DB due to an error", exc)
+    except psycopg2.OperationalError:
+        logging.error("cannot fetch flaky tests from the DB due to an error")
         rows = []
 
     # If a test run has non-default PAGESERVER_VIRTUAL_FILE_IO_ENGINE (i.e. not empty, not tokio-epoll-uring),
@@ -93,10 +93,10 @@ def get_pageserver_default_tenant_config_compaction_algorithm() -> Optional[Dict
         res[row["parent_suite"]][row["suite"]][parametrized_test] = True
 
         logging.info(
-            f"\t{row['parent_suite'].replace('.', '/')}/{row['suite']}.py::{parametrized_test}"
+            "\t%s/%s.py::%s", row['parent_suite'].replace('.', '/'), row['suite'], parametrized_test
         )
 
-    logging.info(f"saving results to {output.name}")
+    logging.info("saving results to %s", output.name)
     json.dump(res, output, indent=2)
 
 

scripts/force_layer_download.py

@@ -124,7 +124,7 @@ async def do_timeline(client: Client, tenant_id, timeline_id):
 
     while True:
         st = await client.timeline_poll_download_remote_layers_status(tenant_id, timeline_id)
-        logging.info(f"{tenant_id}:{timeline_id} state is: {st}")
+        logging.info("%s:%s state is: %s", tenant_id, timeline_id, st)
 
         if spawned["task_id"] != st["task_id"]:
             raise ClientException("download task ids changed while polling")
@@ -161,7 +161,7 @@ async def taskq_handler(task_q, result_q):
         except asyncio.QueueEmpty:
             logging.debug("taskq_handler observed empty task_q, returning")
             return
-        logging.info(f"starting task {id}")
+        logging.info("starting task %s", id)
         try:
             res = await fut
         except Exception as e:
@@ -172,7 +172,7 @@ async def taskq_handler(task_q, result_q):
 async def print_progress(result_q, tasks):
     while True:
         await asyncio.sleep(10)
-        logging.info(f"{result_q.qsize()} / {len(tasks)} tasks done")
+        logging.info("%s / %s tasks done", result_q.qsize(), len(tasks))
 
 
 async def main_impl(args, report_out, client: Client):
@@ -205,13 +205,13 @@ async def main_impl(args, report_out, client: Client):
 
     logging.info("expanded spec:")
     for tid, tlid in tenant_and_timline_ids:
-        logging.info(f"{tid}:{tlid}")
+        logging.info("%s:%s", tid, tlid)
 
     logging.info("remove duplicates after expanding spec")
     tmp = list(set(tenant_and_timline_ids))
     assert len(tmp) <= len(tenant_and_timline_ids)
     if len(tmp) != len(tenant_and_timline_ids):
-        logging.info(f"spec had {len(tenant_and_timline_ids) - len(tmp)} duplicates")
+        logging.info("spec had %s duplicates", len(tenant_and_timline_ids) - len(tmp))
     tenant_and_timline_ids = tmp
 
     logging.info("create tasks and process them at specified concurrency")
@@ -244,7 +244,7 @@ async def main_impl(args, report_out, client: Client):
 
     report = defaultdict(list)
     for id, result in results:
-        logging.info(f"result for {id}: {result}")
+        logging.info("result for %s: %s", id, result)
         if isinstance(result, Completed):
             if result.status["failed_download_count"] == 0:
                 report["completed_without_errors"].append(id)

scripts/sk_cleanup_tenants/script.py

@@ -26,7 +26,7 @@
 
 trash_dir: Path = args.trash_dir
 dry_run: bool = args.dry_run
-logging.info(f"dry_run={dry_run}")
+logging.info("dry_run=%s", dry_run)
 sk_id: int = args.safekeeper_id
 sk_host: str = args.safekeeper_host
 
@@ -94,10 +94,10 @@ def cleanup_tenant(tenant_id):
     assert tenant_dir.exists(), f"{tenant_dir}"
     assert tenant_dir.is_dir(), f"{tenant_dir}"
 
-    logging.info(f"copying {tenant_dir} to {tenant_dir_in_trash}")
+    logging.info("copying %s to %s", tenant_dir, tenant_dir_in_trash)
     shutil.copytree(src=tenant_dir, dst=tenant_dir_in_trash, symlinks=False, dirs_exist_ok=False)
 
-    logging.info(f"deleting {tenant_dir}")
+    logging.info("deleting %s", tenant_dir)
     call_delete_tenant_api(tenant_id)
 
     logging.info("tenant is now deleted, checking that it's gone")
@@ -106,27 +106,27 @@ def cleanup_tenant(tenant_id):
 
 if os.path.exists("script.pid"):
     logging.info(
-        f"script is already running, with pid={Path('script.pid').read_text()}. Terminate it first."
+        "script is already running, with pid=%s. Terminate it first.", Path('script.pid').read_text()
     )
     exit(1)
 
 with open("script.pid", "w", encoding="utf-8") as f:
     f.write(str(os.getpid()))
 
-logging.info(f"started script.py, pid={os.getpid()}")
+logging.info("started script.py, pid=%s", os.getpid())
 
 for line in sys.stdin:
     tenant_id = line.strip()
     try:
-        logging.info(f"start tenant {tenant_id}")
+        logging.info("start tenant %s", tenant_id)
         cleanup_tenant(tenant_id)
-        logging.info(f"done tenant {tenant_id}")
+        logging.info("done tenant %s", tenant_id)
     except KeyboardInterrupt:
         print("KeyboardInterrupt exception is caught")
         break
     except:  # noqa: E722
-        logging.exception(f"failed to clean up tenant {tenant_id}")
+        logging.exception("failed to clean up tenant %s", tenant_id)
 
-logging.info(f"finished script.py, pid={os.getpid()}")
+logging.info("finished script.py, pid=%s", os.getpid())
 
 os.remove("script.pid")

test_runner/fixtures/benchmark_fixture.py

@@ -79,6 +79,8 @@ def parse_from_stdout(
 
         # we know significant parts of these values from test input
         # but to be precise take them from output
+        scale = number_of_clients = number_of_threads = \
+            number_of_transactions_actually_processed = latency_average = tps = None
         for line in stdout_lines:
             # scaling factor: 5
             if line.startswith("scaling factor:"):

test_runner/fixtures/broker.py

@@ -27,11 +27,11 @@ def check_status(self):
 
     def try_start(self):
         if self.handle is not None:
-            log.debug(f"storage_broker is already running on port {self.port}")
+            log.debug("storage_broker is already running on port %s", self.port)
             return
 
         listen_addr = self.listen_addr()
-        log.info(f'starting storage_broker to listen incoming connections at "{listen_addr}"')
+        log.info('starting storage_broker to listen incoming connections at "%s"', listen_addr)
         with open(self.logfile, "wb") as logfile:
             args = [
                 str(self.neon_binpath / "storage_broker"),

test_runner/fixtures/compute_reconfigure.py

@@ -49,7 +49,7 @@ def compute_reconfigure_listener(make_httpserver):
     def handler(request: Request):
         assert request.json is not None
         body: dict[str, Any] = request.json
-        log.info(f"notify-attach request: {body}")
+        log.info("notify-attach request: %s", body)
 
         if self.on_notify is not None:
             self.on_notify(body)

test_runner/fixtures/flaky.py

@@ -40,7 +40,7 @@ def pytest_collection_modifyitems(config: Config, items: List[pytest.Item]):
     try:
         flaky_tests = json.loads(content)
     except ValueError:
-        log.error(f"Can't parse {content} as json")
+        log.error("Can't parse %s as json", content)
         return
 
     for item in items:
@@ -54,7 +54,7 @@ def pytest_collection_modifyitems(config: Config, items: List[pytest.Item]):
 
         if flaky_tests.get(parent_suite, {}).get(suite, {}).get(name, False):
             # Rerun 3 times = 1 original run + 2 reruns
-            log.info(f"Marking {item.nodeid} as flaky. It will be rerun up to 3 times")
+            log.info("Marking %s as flaky. It will be rerun up to 3 times", item.nodeid)
             item.add_marker(pytest.mark.flaky(reruns=2))
 
             # pytest-rerunfailures is not compatible with pytest-timeout (timeout is not set for reruns),

test_runner/fixtures/metrics.py

@@ -48,7 +48,7 @@ def get_metric_value(
         metrics = self.get_metrics()
         results = metrics.query_all(name, filter=filter)
         if not results:
-            log.info(f'could not find metric "{name}"')
+            log.info('could not find metric "%s"', name)
             return None
         assert len(results) == 1, f"metric {name} with given filters is not unique, got: {results}"
         return results[0].value
@@ -81,7 +81,7 @@ def get_metrics_values(
 
         if not absence_ok:
             if len(result) != len(names):
-                log.info(f"Metrics found: {metrics.metrics}")
+                log.info("Metrics found: %s", metrics.metrics)
                 raise RuntimeError(f"could not find all metrics {' '.join(names)}")
 
         return result