Adds Python linting in CI (#1939)

.flake8

@@ -0,0 +1,5 @@
+[flake8]
+per-file-ignores =
+    # line too long
+    *.py: E501
+    bin/migrate.py: E501

.github/workflows/python-lint.yaml

@@ -0,0 +1,38 @@
+name: Run Python Linter
+
+on:
+  push:
+    branches:
+      - 'master'
+    paths:
+      - '**/*.py'
+  pull_request:
+    branches:
+      - master
+    paths:
+      - '**/*.py'
+
+jobs:
+  run-python-linter:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.7"]
+    steps:
+    - uses: actions/checkout@v3
+
+    - 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 -r requirements/requirements.linter.txt
+
+    - name: Analyzing the code with flake8
+      run: |
+        if [ -n "$(git ls-files '**/*.py')" ]; then
+          flake8 $(git ls-files '**/*.py')
+        fi

bin/migrate.py

@@ -5,19 +5,29 @@
 from __future__ import unicode_literals
 import sqlite3
 import os
-import shutil
-import subprocess
 from contextlib import contextmanager
 import datetime
 
 configdir = os.path.join(os.getenv('HOME'), '.screenly/')
 database = os.path.join(configdir, 'screenly.db')
 
 comma = ','.join
-quest = lambda l: '=?,'.join(l) + '=?'
-query_read_all = lambda keys: 'SELECT ' + comma(keys) + ' FROM assets ORDER BY name'
-query_update = lambda keys: 'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id=?'
-mkdict = lambda keys: (lambda row: dict([(keys[ki], v) for ki, v in enumerate(row)]))
+
+
+def quest(values):
+    return '=?,'.join(values) + '=?'
+
+
+def query_read_all(keys):
+    return 'SELECT ' + comma(keys) + ' FROM assets ORDER BY name'
+
+
+def query_update(keys):
+    return 'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id=?'
+
+
+def mkdict(keys):
+    return (lambda row: dict([(keys[ki], v) for ki, v in enumerate(row)]))
 
 
 def is_active(asset):
@@ -57,7 +67,7 @@ def open_db_get_cursor():
         yield (cursor, conn)
         cursor.close()
 
-# ✂--------
+
 query_add_play_order = """
 begin transaction;
 alter table assets add play_order integer default 0;
@@ -89,7 +99,8 @@ def migrate_add_column(col, script):
                 asset.update({'play_order': 0})
                 update(cursor, asset['asset_id'], asset)
                 conn.commit()
-# ✂--------
+
+
 query_create_assets_table = """
 create table assets(
 asset_id text primary key,
@@ -123,7 +134,8 @@ def migrate_make_asset_id_primary_key():
         with open_db_get_cursor() as (cursor, _):
             cursor.executescript(query_make_asset_id_primary_key)
             print('asset_id is primary key')
-# ✂--------
+
+
 query_add_is_enabled_and_nocache = """
 begin transaction;
 alter table assets add is_enabled integer default 0;
@@ -145,7 +157,8 @@ def migrate_add_is_enabled_and_nocache():
                 update(cursor, asset['asset_id'], asset)
                 conn.commit()
             print(f'Added new columns ({col})')
-# ✂--------
+
+
 query_drop_filename = """BEGIN TRANSACTION;
 CREATE TEMPORARY TABLE assets_backup(asset_id, name, uri, md5, start_date, end_date, duration, mimetype);
 INSERT INTO assets_backup SELECT asset_id, name, uri, md5, start_date, end_date, duration, mimetype FROM assets;

docs/developer-documentation.md

@@ -88,6 +88,48 @@ $ docker compose \
 
 We've also provided a [checklist](/docs/qa-checklist.md) that can serve as a guide for testing Anthias manually.
 
+## Linting Python code locally
+
+The project uses `flake8` for linting the Python codebase. While the linter is being run on the CI/CD pipeline,
+you can also run it locally. There are several ways to do this.
+
+### Run the linter using `act`
+
+[`act`](https://nektosact.com/) lets you run GitHub Actions locally. This is useful for testing the CI/CD pipeline locally.
+Installation instructions can be found [here](https://nektosact.com/installation/index.html).
+
+After installing and setting up `act`, run the following command:
+
+```bash
+$ act -W .github/workflows/python-lint.yaml
+```
+
+The command above will run the linter on the all the Python files in the repository. If you want to run the linter
+on a specific file, you can try the commands in the next section.
+
+### Running the linter using `venv`
+
+First, create a virtual environment and install the dependencies:
+
+```bash
+$ python3 -m venv venv/
+$ source venv/bin/activate
+$ pip install -r requirements/requirements.linter.txt
+```
+
+To run the linter on all the Python files in the repository, run the following command:
+
+```bash
+$ flake8 $(git ls-files '**/*.py')
+```
+
+To run the linter on a specific file, run the following command:
+
+```bash
+$ flake8 path/to/file.py
+```
+
+
 ## Managing releases
 ### Creating a new release
 

host_agent.py

@@ -17,6 +17,7 @@
 CMD_TO_ARGV = {b'reboot': ['/usr/bin/sudo', '-n', '/usr/bin/systemctl', 'reboot'],
                b'shutdown': ['/usr/bin/sudo', '-n', '/usr/bin/systemctl', 'poweroff']}
 
+
 def execute_host_command(cmd_name):
     cmd = CMD_TO_ARGV.get(cmd_name, None)
     if cmd is None:
@@ -28,12 +29,14 @@ def execute_host_command(cmd_name):
         phandle = subprocess.run(cmd)
         logging.info("Host command %s (%s) returned %s", cmd_name, cmd, phandle.returncode)
 
+
 def process_message(message):
     if (message.get('type', '') == 'message' and message.get('channel', b'') == CHANNEL_NAME):
         execute_host_command(message.get('data', b''))
     else:
         logging.info("Received unsolicited message: %s", message)
 
+
 def subscriber_loop():
     # Connect to redis on localhost and wait for messages
     logging.info("Connecting to redis...")
@@ -44,6 +47,7 @@ def subscriber_loop():
     for message in pubsub.listen():
         process_message(message)
 
+
 if __name__ == '__main__':
     # Init logging
     logging.basicConfig()

lib/backup_helper.py

@@ -1,6 +1,6 @@
 from __future__ import unicode_literals
+import logging
 import tarfile
-import sh
 import sys
 from datetime import datetime
 from os import path, getenv, makedirs, remove
@@ -9,6 +9,7 @@
 default_archive_name = "screenly-backup"
 static_dir = "screenly/static"
 
+
 def create_backup(name=default_archive_name):
     home = getenv('HOME')
     archive_name = "{}-{}.tar.gz".format(
@@ -40,7 +41,7 @@ def recover(file_path):
     HOME = getenv('HOME')
     if not HOME:
         logging.error('No HOME variable')
-        sys.exit(1) # Alternatively, we can raise an Exception using a custom message, or we can create a new class that extends Exception.
+        sys.exit(1)  # Alternatively, we can raise an Exception using a custom message, or we can create a new class that extends Exception.
 
     with tarfile.open(file_path, "r:gz") as tar:
         for directory in directories:

lib/db.py

@@ -2,9 +2,10 @@
 from __future__ import unicode_literals
 import sqlite3
 from contextlib import contextmanager
-from . import queries
 
-conn = lambda db: sqlite3.connect(db, detect_types=sqlite3.PARSE_DECLTYPES)
+
+def conn(db):
+    return sqlite3.connect(db, detect_types=sqlite3.PARSE_DECLTYPES)
 
 
 @contextmanager

lib/diagnostics.py

@@ -29,7 +29,7 @@ def get_display_power():
     try:
         cec.init()
         tv = cec.Device(cec.CECDEVICE_TV)
-    except:
+    except Exception:
         return 'CEC error'
 
     try:
@@ -119,9 +119,11 @@ def get_debian_version():
 def get_raspberry_code():
     return raspberry_pi_helper.parse_cpu_info().get('hardware', "Unknown")
 
+
 def get_raspberry_model():
     return raspberry_pi_helper.parse_cpu_info().get('model', "Unknown")
 
+
 def compile_report():
     """
     Compile report with various data points.

lib/errors.py

@@ -1,4 +1,6 @@
 from __future__ import unicode_literals
+
+
 class SigalrmException(Exception):
     pass
 

lib/github.py

@@ -24,6 +24,7 @@
 ANALYTICS_MEASURE_ID = 'G-S3VX8HTPK7'
 ANALYTICS_API_SECRET = 'G8NcBpRIS9qBsOj3ODK8gw'
 
+
 def handle_github_error(exc, action):
     # After failing, dont retry until backoff timer expires
     r.set('github-api-error', action)

lib/queries.py

@@ -1,21 +1,46 @@
 from __future__ import unicode_literals
 
 comma = ','.join
-quest = lambda l: '=?,'.join(l) + '=?'
-quest_2 = lambda l, c: ', '.join([('%s=CASE ' % x) + ("WHEN asset_id=? THEN ? " * c) + 'ELSE asset_id END' for x in l])
+
+
+def quest(values):
+    return '=?,'.join(values) + '=?'
+
+
+def quest_2(values, c):
+    return ', '.join([('%s=CASE ' % x) + ("WHEN asset_id=? THEN ? " * c) + 'ELSE asset_id END' for x in values])
+
 
 exists_table = "SELECT name FROM sqlite_master WHERE type='table' AND name='assets'"
 
-read_all = lambda keys: 'select ' + comma(keys) + ' from assets order by play_order'
-read = lambda keys: 'select ' + comma(keys) + ' from assets where asset_id=?'
-create = lambda keys: 'insert into assets (' + comma(keys) + ') values (' + comma(['?'] * len(keys)) + ')'
+
+def read_all(keys):
+    return 'select ' + comma(keys) + ' from assets order by play_order'
+
+
+def read(keys):
+    return 'select ' + comma(keys) + ' from assets where asset_id=?'
+
+
+def create(keys):
+    return 'insert into assets (' + comma(keys) + ') values (' + comma(['?'] * len(keys)) + ')'
+
+
 remove = 'delete from assets where asset_id=?'
-update = lambda keys: 'update assets set ' + quest(keys) + ' where asset_id=?'
 
-multiple_update = lambda keys, count: \
-    'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id IN (' + comma(['?'] * count) + ')'
-multiple_update_not_in = lambda keys, count: \
-    'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id NOT IN (' + comma(['?'] * count) + ')'
 
-multiple_update_with_case = lambda keys, count: 'UPDATE assets SET ' + quest_2(keys, count) + \
-                                                ' WHERE asset_id IN (' + comma(['?'] * count) + ')'
+def update(keys):
+    return 'update assets set ' + quest(keys) + ' where asset_id=?'
+
+
+def multiple_update(keys, count):
+    return 'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id IN (' + comma(['?'] * count) + ')'
+
+
+def multiple_update_not_in(keys, count):
+    return 'UPDATE assets SET ' + quest(keys) + ' WHERE asset_id NOT IN (' + comma(['?'] * count) + ')'
+
+
+def multiple_update_with_case(keys, count):
+    return 'UPDATE assets SET ' + quest_2(keys, count) + \
+        ' WHERE asset_id IN (' + comma(['?'] * count) + ')'

lib/raspberry_pi_helper.py

@@ -1,4 +1,6 @@
 from __future__ import unicode_literals
+
+
 def parse_cpu_info():
     """
     Extracts the various Raspberry Pi related data

lib/utils.py

@@ -1,7 +1,6 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 from future import standard_library
-standard_library.install_aliases()
 from builtins import str
 from builtins import range
 import certifi
@@ -27,6 +26,8 @@
 
 from .assets_helper import update
 
+standard_library.install_aliases()
+
 
 arch = machine()
 
@@ -38,6 +39,7 @@
 except ImportError:
     pass
 
+
 def string_to_bool(string):
     return bool(strtobool(str(string)))
 
@@ -256,7 +258,7 @@ def url_fails(url):
     If it is streaming
     """
     if urlparse(url).scheme in ('rtsp', 'rtmp'):
-        run_mplayer = mplayer('-identify', '-frames', '0', '-nosound', url)
+        run_mplayer = mplayer('-identify', '-frames', '0', '-nosound', url)  # noqa: F821
         for line in run_mplayer.split('\n'):
             if 'Clip info:' in line:
                 return False
@@ -363,6 +365,7 @@ def generate_perfect_paper_password(pw_length=10, has_symbols=True):
 def connect_to_redis():
     return redis.Redis(host='redis', decode_responses=True, port=6379, db=0)
 
+
 def is_docker():
     return os.path.isfile('/.dockerenv')
 

requirements/requirements.linter.txt

@@ -0,0 +1 @@
+flake8==5.0.4

requirements/requirements.local.txt

@@ -3,3 +3,4 @@
 click==8.1.7
 requests==2.32.3
 tenacity==8.4.1
+flake8==5.0.4

send_zmq_message.py

@@ -18,6 +18,7 @@ def get_portal_url():
     else:
         return f'{gateway}:{port}'
 
+
 def get_message(action):
     if action == 'setup_wifi':
         data = {