Merge pull request #122 from victormlg/linting

CFE-4632: Added python linting in github actions

Author: olehermanse

.github/workflows/ci.yml

@@ -1,19 +1,26 @@
 name: Continuous Integration
 
 on:
+  push:
+    branches: [master]
   pull_request:
-    types: [opened, reopened]
+    branches: [master]
 
 jobs:
   unit_tests:
     name: Run Unit Tests
     runs-on: ubuntu-24.04
+    permissions:
+      contents: read
     steps:
     - uses: actions/checkout@v4
       with:
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y python3
+      run: |
+        sudo apt-get update -y && sudo apt-get install -y python3
+        pip install flake8 pyright black pyflakes
+        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
     - name: Install cfbs
       run: pip install cfbs
     - name: Check the status with cfbs
@@ -22,3 +29,5 @@ jobs:
       run: cfbs validate
     - name: Check the formatting
       run: cfbs --check pretty ./cfbs.json
+    - name: Linting
+      run: ./ci/linting.sh

ci/linting.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+
+set -e
+
+echo "Running flake8"
+flake8 . --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160
+
+echo "Running pyright"
+pyright .
+
+shopt -s globstar
+echo "Running black"
+black --check .
+
+echo "Running pyflakes"
+pyflakes .

examples/git-using-lib/git_using_lib.py

@@ -10,7 +10,7 @@ def validate_promise(self, promiser, attributes, metadata):
         if not promiser.startswith("/"):
             raise ValidationError(f"File path '{promiser}' must be absolute")
         if "repository" not in attributes:
-            raise ValidationError(f"Attribute 'repository' is required")
+            raise ValidationError("Attribute 'repository' is required")
 
     def evaluate_promise(self, promiser, attributes, metadata):
         url = attributes["repository"]

examples/gpg/gpg.py

@@ -42,8 +42,7 @@
 """
 
 import json
-from subprocess import Popen, PIPE
-import sys
+from subprocess import Popen, PIPE, TimeoutExpired
 from cfengine_module_library import PromiseModule, ValidationError, Result
 
 
@@ -109,9 +108,9 @@ def validate_promise(self, promiser, attributes, metadata):
             raise ValidationError(
                 f"Promiser '{promiser}' for 'gpg_keys' promise must be an absolute path"
             )
-        if not "keylist" in attributes:
+        if "keylist" not in attributes:
             raise ValidationError(
-                f"Required attribute 'keylist' missing for 'gpg_keys' promise"
+                "Required attribute 'keylist' missing for 'gpg_keys' promise"
             )
 
     def evaluate_promise(self, promiser, attributes, metadata):

examples/rss/rss.py

@@ -1,4 +1,7 @@
-import requests, html, re, os, random
+import requests
+import re
+import os
+import random
 import xml.etree.ElementTree as ET
 from cfengine_module_library import PromiseModule, ValidationError, Result
 
@@ -42,7 +45,7 @@ def validate_promise(self, promiser, attributes, metadata):
             # check that attribute select has a valid type
             if type(select) is not str:
                 raise ValidationError(
-                    f"Invalid type for attribute select: expected string"
+                    "Invalid type for attribute select: expected string"
                 )
 
             # check that attribute select has a valid value
@@ -159,18 +162,18 @@ def _write_promiser(self, item, promiser):
             return Result.NOT_KEPT
 
     def _is_win_file(self, path):
-        return re.search(r"^[a-zA-Z]:\\[\\\S|*\S]?.*$", path) != None
+        return re.search(r"^[a-zA-Z]:\\[\\\S|*\S]?.*$", path) is not None
 
     def _is_unix_file(self, path):
-        return re.search(r"^(/[^/ ]*)+/?$", path) != None
+        return re.search(r"^(/[^/ ]*)+/?$", path) is not None
 
     def _is_url(self, path):
         return (
             re.search(
                 r"^http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+",
                 path,
             )
-            != None
+            is not None
         )
 
 

examples/site-up/site_up.py

@@ -31,7 +31,7 @@ def evaluate_promise(self, promiser, attributes, metadata):
 
         error = None
         try:
-            code = urllib.request.urlopen(url, context=ssl_ctx).getcode()
+            urllib.request.urlopen(url, context=ssl_ctx).getcode()
             self.log_verbose(f"Site '{url}' is UP!")
             return Result.KEPT
         except urllib.error.HTTPError as e:

libraries/python/cfengine_module_library.py

@@ -226,6 +226,8 @@ def _handle_request(self, request):
             "debug",
         ]
 
+        promiser = None
+        attributes = {}
         if operation in ["validate_promise", "evaluate_promise"]:
             promiser = request["promiser"]
             attributes = request.get("attributes", {})
@@ -404,7 +406,7 @@ def _handle_evaluate(self, promiser, attributes, request):
             assert results is not None  # Most likely someone forgot to return something
 
             # evaluate_promise should return either a result or a (result, result_classes) pair
-            if type(results) == str:
+            if isinstance(results, str):
                 self._result = results
             else:
                 assert len(results) == 2