Add linting workflow + cleanup

.github/workflows/python-app.yml

@@ -0,0 +1,37 @@
+# This workflow will install Python dependencies, run tests and lint with a single version of Python
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions
+
+name: Python application
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python 3.10
+      uses: actions/setup-python@v2
+      with:
+        python-version: "3.10"
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install flake8 pytest
+        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+    - name: Lint with flake8
+      run: |
+        # stop the build if there are Python syntax errors or undefined names
+        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+        flake8 . --count --max-complexity=10 --max-line-length=127 --statistics --exclude ./migrations/
+    # TODO: add testing here once there are tests
+    # - name: Test with pytest
+    #  run: |
+    #    pytest

app/__init__.py

@@ -1 +0,0 @@
-from app.app import app

app/app.py

@@ -15,6 +15,7 @@
 import time
 import traceback
 
+from app import models
 
 app = Flask(__name__)
 
@@ -40,16 +41,14 @@
     '2': 'inbetween'
 }
 
-from app import models
-from app import cron
-
 
 def get_mattermost_id(username):
     '''Given a mattermost username, return the user id. Don't call this with stale data'''
     try:
         response = mm_driver.users.get_user_by_username(username)
         return response['id']
-    except:
+    # TODO: only KeyError?
+    except Exception:
         return None
 
 
@@ -112,6 +111,7 @@ def mattermost_response(message, ephemeral=False):
 def mattermost_doorkeeper_message(message, webhook=config.doorkeeper_webhook):
     requests.post(webhook, json={"text": message})
 
+
 # Removes @ from username if @ was prepended
 def get_actual_username(username):
     return username.lstrip('@')
@@ -125,14 +125,22 @@ def authorize(admin_user):
     tokens = request.values.get('text').strip().split()
     if not tokens:
         # list authorized user
-        response = '\n'.join(f'{"**" if u.admin else ""}{u.username}{" ADMIN**" if u.admin else ""}' for u in models.User.query.filter_by(authorized=True).order_by(models.User.username))
+        response = ''
+        for u in models.User.query.filter_by(authorized=True).order_by(models.User.username):
+            response += f'{"**" if u.admin else ""}{u.username}{" ADMIN**" if u.admin else ""}' + '\n'
         return mattermost_response(response, ephemeral=True)
     if len(tokens) > 2:
-        return mattermost_response("To authorize a user: /authorize username [admin]\nTo list authorized users: /authorize", ephemeral=True)
+        return mattermost_response(
+            "To authorize a user: /authorize username [admin]\nTo list authorized users: /authorize",
+            ephemeral=True
+        )
     to_authorize_username = get_actual_username(tokens[0])
     to_authorize_id = get_mattermost_id(to_authorize_username)
     if to_authorize_id is None:
-        return mattermost_response("User '{}' does not seem to exist in Mattermost".format(to_authorize_username), ephemeral=True)
+        return mattermost_response(
+            "User '{}' does not seem to exist in Mattermost".format(to_authorize_username),
+            ephemeral=True
+        )
     as_admin = len(tokens) == 2 and tokens[1] == 'admin'
     user = models.User.query.filter_by(mattermost_id=to_authorize_id).first()
     if not user:
@@ -178,6 +186,7 @@ def lockbot_request(command):
     r = requests.post(config.lockbot_url, payload, headers={'HMAC': calculated_hmac})
     return DOOR_STATUS[r.text]
 
+
 @app.route('/door', methods=['POST'])
 @requires_token('door')
 @requires_regular
@@ -193,6 +202,7 @@ def door(user):
         mattermost_doorkeeper_message(f'door was {translated_state_before_command}, {user.username} tried to {command} door')
     return mattermost_response(translated_state_before_command, ephemeral=True)
 
+
 @app.route('/spaceapi.json')
 def spaceapi():
     door_status = lockbot_request('status')
@@ -228,6 +238,7 @@ def spaceapi():
     response.headers.add('Access-Control-Allow-Origin', '*')
     return response
 
+
 @app.route('/doorkeeper', methods=['POST'])
 def doorkeeper():
     raw_data = request.get_data()
@@ -237,41 +248,43 @@ def doorkeeper():
         print(f"WRONG: {hmac_header} != {calculated_hash}", file=sys.stderr)
         return abort(401)
 
-    data_dict = {l.split('=')[0]: l.split('=')[1] for l in raw_data.decode('utf8').split('&')}
+    data_dict = {data.split('=')[0]: data.split('=')[1] for data in raw_data.decode('utf8').split('&')}
     cmd = data_dict['cmd']
     reason = data_dict['why']
     value = data_dict['val']
     try:
-        requests.post(config.kelderapi_doorkeeper_url, json=data_dict, headers={'Token': config.kelderapi_doorkeeper_key}, timeout=3)
-    except:
-        mattermost_doorkeeper_message(f"Posting {data_dict} to kelderapi failed\n```{traceback.format_exc()}\n```", webhook=config.debug_webhook)
-    if reason == 'mattermore':
-        if cmd == 'status':
-            return ''
-        msg = f'"{cmd}" command from Mattermost handled'
-    elif reason == 'boot':
-        msg = 'lockbot booted'
-    elif reason == 'panic':
-        msg = f'@sysadmin: the door panicked with reason {cmd}'
-    elif reason == 'state':
-        msg = f'The door is now {DOOR_STATUS[value]}'
-    elif reason == 'chal':
+        requests.post(config.kelderapi_doorkeeper_url, json=data_dict, headers={'Token': config.kelderapi_doorkeeper_key},
+                      timeout=3)
+    except Exception:
+        mattermost_doorkeeper_message(f"Posting {data_dict} to kelderapi failed\n```{traceback.format_exc()}\n```",
+                                      webhook=config.debug_webhook)
+
+    reason_map = {
+        'mattermore': '' if cmd == 'reason' else f'"{cmd}" command from Mattermost handled',
+        'boot': 'lockbot booted',
+        'panic': f'@sysadmin: the door panicked with reason {cmd}',
+        'state': f'The door is now {DOOR_STATUS[value]}',
+        'chal': '',
+        'delaybutton': 'Delayed door close button was pressed',
+    }
+    msg = reason_map.get(reason, f'Unhandled message type: {cmd},{reason},{value}')
+
+    if not msg:
         return ''
-    elif reason == 'delaybutton':
-        msg = 'Delayed door close button was pressed'
-    else:
-        msg = f'Unhandled message type: {cmd},{why},{val}'
+
     mattermost_doorkeeper_message(msg, webhook=config.debug_webhook)
     return "OK"
 
+
 @app.route('/cammiechat', methods=['POST'])
 @requires_token('cammiechat')
 @requires_regular
 def cammiechat(user):
     headers = {
         "X-Username": user.username
     }
-    requests.post("https://kelder.zeus.ugent.be/messages/", data=request.values.get('text').strip(), headers=headers, timeout=5)
+    requests.post("https://kelder.zeus.ugent.be/messages/", data=request.values.get('text').strip(), headers=headers,
+                  timeout=5)
     return mattermost_response("Message sent", ephemeral=True)
 
 
@@ -334,6 +347,7 @@ def json_quotes():
     response.headers.add('Access-Control-Allow-Origin', '*')
     return response
 
+
 RESTO_TEMPLATE = """
 # Restomenu
 
@@ -366,11 +380,11 @@ def json_quotes():
     "vegi_table": {"vegetarian", "vegan"},
 }
 
+
 @app.route('/resto', methods=['GET'])
 def resto_menu():
     today = datetime.today()
-    url = "https://zeus.ugent.be/hydra/api/2.0/resto/menu/nl-sterre/{}/{}/{}.json"\
-            .format(today.year, today.month, today.day)
+    url = "https://zeus.ugent.be/hydra/api/2.0/resto/menu/nl-sterre/{}/{}/{}.json".format(today.year, today.month, today.day)
     resto = requests.get(url, timeout=5).json()
 
     if not resto["open"]:
@@ -407,11 +421,12 @@ def format_items(items):
             )
 
         return RESTO_TEMPLATE.format(
-                **{k: table_for(v) for k,v in RESTO_TABLES.items()},
+                **{k: table_for(v) for k, v in RESTO_TABLES.items()},
                 uncategorized=uncategorized,
                 vegetable_table="\n".join(resto["vegetables"])
                 )
 
+
 @app.route('/resto.json', methods=['GET'])
 def resto_menu_json():
     return mattermost_response(resto_menu(), ephemeral=True)

app/cron.py

@@ -1,9 +1,7 @@
 from app import models
 from app.app import db, mm_driver, config
 from app.app import app
-from flask import current_app
 from flask_apscheduler import APScheduler
-import atexit
 import requests
 from bs4 import BeautifulSoup
 
@@ -30,7 +28,6 @@ def get_dict_news():
     return result
 
 
-
 def post_dict_news(n):
     message = f'**DICT NIEUWS** op {n["date"]}: [{n["message"]}]({n["link"]})'
     print(f"Posting {message}")
@@ -46,7 +43,7 @@ def dict_news_task():
         dict_config = models.KeyValue.query.filter_by(keyname=DICT_NEWS_KEY).first() or models.KeyValue(DICT_NEWS_KEY, "111")
         news_items = get_dict_news()
         current_maxseen = int(dict_config.value)
-        for news_item in get_dict_news():
+        for news_item in news_items:
             if news_item['id'] > current_maxseen:
                 current_maxseen = news_item['id']
                 post_dict_news(news_item)

app/models.py

@@ -34,7 +34,7 @@ class Quote(db.Model):
         default=datetime.utcnow
     )
 
-    QUOTEE_REGEX = re.compile('\W*([a-zA-Z\-_0-9]+).*')
+    QUOTEE_REGEX = re.compile(r'\W*([a-zA-Z\-_0-9]+).*')
 
     def __repr__(self):
         return "<Quote {} \"{}\">".format(self.quoter, self.quote)

import_quotes.py

@@ -1,8 +1,7 @@
 #!/usr/bin/env python3
 
-from flask import Flask, request, Response, abort, render_template
+from flask import Flask
 from flask_sqlalchemy import SQLAlchemy
-from flask_migrate import Migrate
 from app import models
 from datetime import datetime
 import config
@@ -16,7 +15,6 @@
     split = line.split("\t")
     assert len(split) == 4, "Too much tabs at line \"{}\", {}".format(line, len(split))
     quoter, channel, quote_text, created_at = split
-    quote = models.Quote(quoter, quote_text, channel, created_at = datetime.strptime(created_at.strip(), "%Y-%m-%d %H:%M:%S"))
+    quote = models.Quote(quoter, quote_text, channel, created_at=datetime.strptime(created_at.strip(), "%Y-%m-%d %H:%M:%S"))
     db.session.add(quote)
 db.session.commit()
-

passenger_wsgi.py

@@ -8,5 +8,3 @@
     os.execl(INTERP, INTERP, *sys.argv)
 
 sys.path.append(os.getcwd())
-
-from app import app as application