Install and use Ruff to check and format python

.github/workflows/python_lint.yml

@@ -0,0 +1,29 @@
+name: Check python syntax
+
+on:
+  pull_request:
+
+jobs:
+  ruff:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.12"]
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Set Python Version
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ matrix.python-version }}
+
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements-dev.txt
+
+    - name: Python Ruff Lint and Format
+      run: |
+        ruff check --output-format=github .
+        ruff format --check

.ruff.toml

@@ -0,0 +1,17 @@
+line-length = 79
+
+[lint]
+# Default `select` is: "E4", "E7", "E9", "F"
+# `E` for "pycodestyle" (subset)
+# `F` for "Pyflakes"
+
+# In addition, we also enable:
+# `Q` for "flake8-quotes"
+# `I` for "isort"
+extend-select = ["Q", "I"]
+
+[lint.per-file-ignores]
+"powa/__init__.py" = ["E402"]
+
+[lint.isort]
+no-sections = true

CONTRIBUTING.md

@@ -15,6 +15,25 @@ This project relies on a few frameworks / libraries including:
 For the following steps, we assume you use PoWA web in debug mode (for example
 by running `./run-powa.py` or using podman dev environment).
 
+### Python syntax and formatting
+
+Python syntax and formatting must conform to ruff. CI checks new code with ruff.
+
+If not already available, you can create a virtualenv for developpement purpose:
+
+```shell
+python3 -m venv .venv
+source .venv/bin/activate
+pip install -r requirements-dev.txt
+```
+
+You can then do a syntax check and format check by running the following command:
+
+``` shell
+ruff check
+ruff format --check
+```
+
 ## Requirements
 
  - A recent version of `NodeJS` (16+) and `npm` are required.

powa/__init__.py

@@ -1,37 +1,50 @@
 from __future__ import print_function
+
 """
 Powa main application.
 """
+
 import os
 import re
 
-__VERSION__ = '5.0.0dev'
+__VERSION__ = "5.0.0dev"
 
 ver_tmp = re.sub("(alpha|beta|dev)[0-9]*", "", __VERSION__)
-__VERSION_NUM__ = [int(part) for part in (ver_tmp.split('.'))]
+__VERSION_NUM__ = [int(part) for part in (ver_tmp.split("."))]
 
 POWA_ROOT = os.path.dirname(__file__)
 
-from tornado.web import Application, URLSpec as U
-from powa.options import parse_options
+# Import from powa.options must go before tornado.options
+from powa.options import parse_options  # noqa: I001
 from tornado.options import options
-from powa import ui_modules, ui_methods
+from tornado.web import Application
+from tornado.web import URLSpec as U
+
+from powa import ui_methods, ui_modules
+from powa.collector import (
+    CollectorDbCatRefreshHandler,
+    CollectorForceSnapshotHandler,
+    CollectorReloadHandler,
+)
+from powa.config import (
+    RemoteConfigOverview,
+    RepositoryConfigOverview,
+)
+from powa.database import DatabaseOverview, DatabaseSelector
 from powa.framework import AuthHandler
-from powa.user import LoginHandler, LogoutHandler
+from powa.function import FunctionOverview
+from powa.io import (
+    ByBackendTypeIoOverview,
+    ByContextIoOverview,
+    ByObjIoOverview,
+)
 from powa.overview import Overview
-from powa.server import ServerSelector, ServerOverview
-from powa.database import DatabaseSelector, DatabaseOverview
-from powa.query import QueryOverview
 from powa.qual import QualOverview
-from powa.function import FunctionOverview
-from powa.config import RepositoryConfigOverview, RemoteConfigOverview
-from powa.collector import (CollectorReloadHandler,
-                            CollectorForceSnapshotHandler,
-                            CollectorDbCatRefreshHandler)
-from powa.wizard import IndexSuggestionHandler
-from powa.io import (ByBackendTypeIoOverview, ByObjIoOverview,
-                     ByContextIoOverview)
+from powa.query import QueryOverview
+from powa.server import ServerOverview, ServerSelector
 from powa.slru import ByNameSlruOverview
+from powa.user import LoginHandler, LogoutHandler
+from powa.wizard import IndexSuggestionHandler
 
 
 class IndexHandler(AuthHandler):
@@ -52,38 +65,59 @@ def make_app(**kwargs):
     URLS = [
         U(r"%slogin/" % options.url_prefix, LoginHandler, name="login"),
         U(r"%slogout/" % options.url_prefix, LogoutHandler, name="logout"),
-        U(r"%sreload_collector/" % options.url_prefix, CollectorReloadHandler,
-          name="reload_collector"),
-        U(r"%sforce_snapshot/(\d+)" % options.url_prefix,
-          CollectorForceSnapshotHandler, name="force_snapshot"),
-        U(r"%srefresh_db_cat/" % options.url_prefix,
-          CollectorDbCatRefreshHandler, name="refresh_db_cat"),
-        U(r"%sserver/select" % options.url_prefix, ServerSelector,
-          name="server_selector"),
-        U(r"%sdatabase/select" % options.url_prefix, DatabaseSelector,
-          name="database_selector"),
+        U(
+            r"%sreload_collector/" % options.url_prefix,
+            CollectorReloadHandler,
+            name="reload_collector",
+        ),
+        U(
+            r"%sforce_snapshot/(\d+)" % options.url_prefix,
+            CollectorForceSnapshotHandler,
+            name="force_snapshot",
+        ),
+        U(
+            r"%srefresh_db_cat/" % options.url_prefix,
+            CollectorDbCatRefreshHandler,
+            name="refresh_db_cat",
+        ),
+        U(
+            r"%sserver/select" % options.url_prefix,
+            ServerSelector,
+            name="server_selector",
+        ),
+        U(
+            r"%sdatabase/select" % options.url_prefix,
+            DatabaseSelector,
+            name="database_selector",
+        ),
         U(r"%s" % options.url_prefix, IndexHandler, name="index"),
-        U(r"%sserver/(\d+)/database/([^\/]+)/suggest/" % options.url_prefix,
-          IndexSuggestionHandler, name="index_suggestion")
+        U(
+            r"%sserver/(\d+)/database/([^\/]+)/suggest/" % options.url_prefix,
+            IndexSuggestionHandler,
+            name="index_suggestion",
+        ),
     ]
 
-    for dashboard in (Overview,
-                      ServerOverview,
-                      DatabaseOverview,
-                      QueryOverview,
-                      QualOverview,
-                      FunctionOverview,
-                      RepositoryConfigOverview,
-                      RemoteConfigOverview,
-                      ByBackendTypeIoOverview,
-                      ByObjIoOverview,
-                      ByContextIoOverview,
-                      ByNameSlruOverview):
+    for dashboard in (
+        Overview,
+        ServerOverview,
+        DatabaseOverview,
+        QueryOverview,
+        QualOverview,
+        FunctionOverview,
+        RepositoryConfigOverview,
+        RemoteConfigOverview,
+        ByBackendTypeIoOverview,
+        ByObjIoOverview,
+        ByContextIoOverview,
+        ByNameSlruOverview,
+    ):
         URLS.extend(dashboard.url_specs(options.url_prefix))
 
     _cls = Application
-    if 'legacy_wsgi' in kwargs:
+    if "legacy_wsgi" in kwargs:
         from tornado.wsgi import WSGIApplication
+
         _cls = WSGIApplication
 
     return _cls(
@@ -95,4 +129,5 @@ def make_app(**kwargs):
         static_url_prefix=("%sstatic/" % options.url_prefix),
         cookie_secret=options.cookie_secret,
         template_path=os.path.join(POWA_ROOT, "templates"),
-        **kwargs)
+        **kwargs,
+    )

powa/collector.py

@@ -2,7 +2,9 @@
 Dashboard for the powa-collector summary page, and other infrastructure for the
 collector handling.
 """
+
 from __future__ import absolute_import
+
 import json
 from powa.dashboards import MetricGroupDef
 from powa.framework import AuthHandler
@@ -23,44 +25,46 @@ def post_process(self, data, server, **kwargs):
             JOIN {powa}.powa_snapshot_metas m ON m.srvid = s.id
             WHERE s.id = %(server)s"""
 
-        row = self.execute(sql, params={'server': server})
+        row = self.execute(sql, params={"server": server})
 
         # unexisting server, bail out
-        if (len(row) != 1):
-            data["messages"] = {'alert': ["This server does not exists"]}
+        if len(row) != 1:
+            data["messages"] = {"alert": ["This server does not exists"]}
             return data
 
-        status = 'unknown'
-        if (server == '0'):
+        status = "unknown"
+        if server == "0":
             status = self.execute("""SELECT
                     CASE WHEN count(*) = 1 THEN 'running'
                     ELSE 'stopped'
                     END AS status
                     FROM pg_stat_activity
                     WHERE application_name LIKE 'PoWA - %%'""")[0]["status"]
         else:
-            raw = self.notify_collector('WORKERS_STATUS', [server], 2)
+            raw = self.notify_collector("WORKERS_STATUS", [server], 2)
 
             status = None
             # did we receive a valid answer?
-            if (len(raw) != 0 and "OK" in raw[0]):
+            if len(raw) != 0 and "OK" in raw[0]:
                 # just keep the first one
                 tmp = raw[0]["OK"]
-                if (server in tmp):
+                if server in tmp:
                     status = json.loads(tmp)[server]
 
-        if (status is None):
-            return {'messages': {"warning": ["Could not get status for this "
-                                             "instance"]},
-                    'data': []}
+        if status is None:
+            return {
+                "messages": {
+                    "warning": ["Could not get status for this " "instance"]
+                },
+                "data": [],
+            }
 
-        msg = 'Collector status for this instance: ' + status
+        msg = "Collector status for this instance: " + status
         level = "alert"
-        if (status == "running"):
+        if status == "running":
             level = "success"
 
-        return {'messages': {level: [msg]},
-                'data': []}
+        return {"messages": {level: [msg]}, "data": []}
 
 
 class CollectorReloadHandler(AuthHandler):
@@ -69,12 +73,12 @@ class CollectorReloadHandler(AuthHandler):
     def get(self):
         res = False
 
-        answers = self.notify_collector('RELOAD')
+        answers = self.notify_collector("RELOAD")
 
         # iterate over the results.  If at least one OK is received, report
         # success, otherwise failure
         for a in answers:
-            if ("OK" in a):
+            if "OK" in a:
                 res = True
                 break
 
@@ -85,7 +89,7 @@ class CollectorForceSnapshotHandler(AuthHandler):
     """Request an immediate snapshot on the given server."""
 
     def get(self, server):
-        answers = self.notify_collector('FORCE_SNAPSHOT', [server])
+        answers = self.notify_collector("FORCE_SNAPSHOT", [server])
 
         self.render_json(answers)
 
@@ -95,11 +99,11 @@ class CollectorDbCatRefreshHandler(AuthHandler):
 
     def post(self):
         payload = json.loads(self.request.body.decode("utf8"))
-        nb_db = len(payload['dbnames'])
-        args = [payload['srvid'], str(nb_db)]
-        if (nb_db > 0):
-            args.extend(payload['dbnames']);
+        nb_db = len(payload["dbnames"])
+        args = [payload["srvid"], str(nb_db)]
+        if nb_db > 0:
+            args.extend(payload["dbnames"])
 
-        answers = self.notify_collector('REFRESH_DB_CAT', args)
+        answers = self.notify_collector("REFRESH_DB_CAT", args)
 
         self.render_json(answers)

powa/compat.py

@@ -5,29 +5,37 @@
     http://pypi.python.org/pypi/six/
 
 """
+
 from __future__ import absolute_import
+
+import json
 import psycopg2
 from psycopg2 import extensions
-import json
 
 # If psycopg2 < 2.5, register json type
-psycopg2_version = tuple(psycopg2.__version__.split(' ')[0].split('.'))
-if psycopg2_version < ('2', '5'):
+psycopg2_version = tuple(psycopg2.__version__.split(" ")[0].split("."))
+if psycopg2_version < ("2", "5"):
     JSON_OID = 114
     newtype = extensions.new_type(
-        (JSON_OID,), "JSON", lambda data, cursor: json.loads(data))
+        (JSON_OID,), "JSON", lambda data, cursor: json.loads(data)
+    )
     extensions.register_type(newtype)
 
+
 def with_metaclass(meta, *bases):
     """Create a base class with a metaclass."""
+
     # This requires a bit of explanation: the basic idea is to make a dummy
     # metaclass for one level of class instantiation that replaces itself with
     # the actual metaclass.
     class metaclass(meta):
         """The actual metaclass."""
+
         def __new__(cls, name, _, d):
             return meta(name, bases, d)
-    return type.__new__(metaclass, 'temporary_class', (), {})
+
+    return type.__new__(metaclass, "temporary_class", (), {})
+
 
 class classproperty(object):
     """