Merge branch 'bwh/pyflakes' into 'master'

Fix pyflakes errors and warnings; add it to CI configuration

See merge request CodethinkLabs/lorry/lorry-controller!22

14 files changed, 56 insertions, 29 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7984fcd..e51682f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -3,6 +3,8 @@ image: python:3.6-slim-buster
 before_script:
   - apt-get update -y
   - apt-get install -y -qq python3-dev python3-pip
+  # Static checker
+  - apt-get install -y pyflakes3
   # Deps for running tests
   - apt-get install -y -qq cmdtest curl git
   # Deps for lorry-controller
@@ -12,6 +14,21 @@ before_script:
   - pip3 install yoyo-migrations
   - pip3 install pyyaml
   - pip3 install https://gitlab.com/trovekube/cliapp/-/archive/cliapp-1.20180812.1/cliapp-cliapp-1.20180812.1.tar.gz
+
+run-pyflakes:
+  script:
+  - |
+    {
+        find . -path ./.git -prune -o -name '*.py' -print
+        find . -path ./.git -prune -o -type f -perm /111 -print |
+        while read script; do
+            # Check for Python shebang on the first line only
+            if awk '/^#!.*python/ { exit 0 } { exit 1 }' "$script"; then
+                echo "$script"
+            fi
+        done
+    } | xargs pyflakes3
+
 run-check:
   script:
   - sh check
diff --git a/lorry-controller-minion b/lorry-controller-minion
index 1900313..2cde979 100755
--- a/lorry-controller-minion
+++ b/lorry-controller-minion
@@ -111,7 +111,6 @@ class MINION(cliapp.Application):
     def get_job_spec(self):
         host = self.settings['webapp-host']
         port = int(self.settings['webapp-port'])
-        timeout = self.settings['webapp-timeout']
 
         logging.debug('Requesting job from WEBAPP (%s:%s)', host, port)
 
diff --git a/lorry-controller-remove-old-jobs b/lorry-controller-remove-old-jobs
index fcd8b38..a69e505 100755
--- a/lorry-controller-remove-old-jobs
+++ b/lorry-controller-remove-old-jobs
@@ -20,7 +20,6 @@ import json
 import logging
 import time
 import urllib.request, urllib.error, urllib.parse
-import contextlib
 
 import cliapp
 
@@ -144,7 +143,7 @@ class OldJobRemover(cliapp.Application):
     def post(self, path, data):
         url = self.make_url(path)
         with urllib.request.urlopen(url, data.encode('utf-8')) as f:
-            result = f.read()
+            f.read()
 
 
 OldJobRemover().run()
diff --git a/lorrycontroller/__init__.py b/lorrycontroller/__init__.py
index e1128ae..b2510dc 100644
--- a/lorrycontroller/__init__.py
+++ b/lorrycontroller/__init__.py
@@ -65,4 +65,35 @@ def get_upstream_host(host_info):
     return upstream_types[host_info['type']](host_info)
 
 
-__all__ = locals()
+__all__ = [
+    'StateDB',
+    'LorryNotFoundError', 'WrongNumberLorriesRunningJob', 'HostNotFoundError',
+    'LorryControllerRoute',
+    'ReadConfiguration',
+    'Status', 'StatusHTML', 'StatusRenderer',
+    'ListQueue',
+    'ShowLorry', 'ShowLorryHTML',
+    'StartQueue', 'StopQueue',
+    'GiveMeJob',
+    'JobUpdate',
+    'ListRunningJobs',
+    'MoveToTop', 'MoveToBottom',
+    'StopJob',
+    'ListAllJobs', 'ListAllJobsHTML',
+    'ShowJob', 'ShowJobHTML', 'JobShower',
+    'RemoveGhostJobs',
+    'RemoveJob',
+    'LsUpstreams', 'ForceLsUpstream',
+    'PretendTime',
+    'GetMaxJobs', 'SetMaxJobs',
+    'StaticFile',
+    'setup_proxy',
+    'gerrit',
+    'gitano',
+    'gitea',
+    'gitlab',
+    'local',
+    'downstream_types',
+    'upstream_types',
+    'get_upstream_host',
+]
diff --git a/lorrycontroller/gitano.py b/lorrycontroller/gitano.py
index 46174ad..8587737 100644
--- a/lorrycontroller/gitano.py
+++ b/lorrycontroller/gitano.py
@@ -22,7 +22,6 @@ import urllib.request, urllib.error, urllib.parse
 import cliapp
 import requests
 
-import lorrycontroller
 from . import hosts
 
 
diff --git a/lorrycontroller/gitlab.py b/lorrycontroller/gitlab.py
index 266861c..bb603de 100644
--- a/lorrycontroller/gitlab.py
+++ b/lorrycontroller/gitlab.py
@@ -22,7 +22,6 @@ to the targetted group.
 '''
 
 import logging
-import re
 import urllib.parse
 
 try:
diff --git a/lorrycontroller/givemejob.py b/lorrycontroller/givemejob.py
index ee998ea..2f7580f 100644
--- a/lorrycontroller/givemejob.py
+++ b/lorrycontroller/givemejob.py
@@ -69,7 +69,7 @@ class GiveMeJob(lorrycontroller.LorryControllerRoute):
         return (lorry_info['running_job'] is None and due <= now)
 
     @staticmethod
-    def get_upstream_host_repo_metadata(lorry_info):
+    def get_upstream_host_repo_metadata(statedb, lorry_info):
         assert lorry_info['from_host']
         assert lorry_info['from_path']
 
@@ -172,7 +172,7 @@ class GiveMeJob(lorrycontroller.LorryControllerRoute):
 
         host_name = lorry_info['from_host']
         if host_name:
-            metadata = self.get_upstream_host_repo_metadata(lorry_info)
+            metadata = self.get_upstream_host_repo_metadata(statedb, lorry_info)
         else:
             host_name, metadata = self.get_single_repo_metadata(lorry_info)
 
diff --git a/lorrycontroller/jobupdate.py b/lorrycontroller/jobupdate.py
index ec7e533..0b86b63 100644
--- a/lorrycontroller/jobupdate.py
+++ b/lorrycontroller/jobupdate.py
@@ -15,7 +15,6 @@
 
 
 import logging
-import time
 
 import bottle
 
diff --git a/lorrycontroller/maxjobs.py b/lorrycontroller/maxjobs.py
index ce594c2..3f766a4 100644
--- a/lorrycontroller/maxjobs.py
+++ b/lorrycontroller/maxjobs.py
@@ -15,8 +15,6 @@
 
 
 import logging
-import os
-import time
 
 import bottle
 
diff --git a/lorrycontroller/pretendtime.py b/lorrycontroller/pretendtime.py
index 3fd1a70..8a34d0d 100644
--- a/lorrycontroller/pretendtime.py
+++ b/lorrycontroller/pretendtime.py
@@ -14,15 +14,9 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 
-import errno
-import glob
-import json
 import logging
-import os
-import re
 
 import bottle
-import cliapp
 
 import lorrycontroller
 
diff --git a/lorrycontroller/readconf.py b/lorrycontroller/readconf.py
index 23dfff1..b488765 100644
--- a/lorrycontroller/readconf.py
+++ b/lorrycontroller/readconf.py
@@ -174,7 +174,7 @@ class ReadConfiguration(lorrycontroller.LorryControllerRoute):
             return default_interval
         m = re.match('(\d+)\s*(s|m|h|d)?', value, re.I)
         if not m:
-            return default_value
+            return default_interval
 
         number, factor = m.groups()
         factors = {
@@ -209,11 +209,6 @@ class ReadConfiguration(lorrycontroller.LorryControllerRoute):
             timeout = section.get(
                 'lorry-timeout', self.DEFAULT_LORRY_TIMEOUT)
 
-            try:
-                old_lorry_info = statedb.get_lorry_info(path)
-            except lorrycontroller.LorryNotFoundError:
-                old_lorry_info = None
-
             statedb.add_to_lorries(
                 path=path, text=text, from_host='', from_path='',
                 interval=interval, timeout=timeout)
@@ -244,7 +239,7 @@ class ReadConfiguration(lorrycontroller.LorryControllerRoute):
                 except yaml.YAMLError:
                     f.seek(0)
                     obj = json.load(f)
-        except ValueError as e:
+        except ValueError:
             logging.error('YAML and JSON problem in %s', filename)
             return []
 
diff --git a/lorrycontroller/removeghostjobs.py b/lorrycontroller/removeghostjobs.py
index 2b2760c..81ff014 100644
--- a/lorrycontroller/removeghostjobs.py
+++ b/lorrycontroller/removeghostjobs.py
@@ -15,9 +15,6 @@
 
 
 import logging
-import time
-
-import bottle
 
 import lorrycontroller
 
diff --git a/lorrycontroller/showjob.py b/lorrycontroller/showjob.py
index bc82bfe..297b652 100644
--- a/lorrycontroller/showjob.py
+++ b/lorrycontroller/showjob.py
@@ -36,7 +36,7 @@ class JobShower(object):
             'job_id': job_id,
             'host': statedb.get_job_minion_host(job_id),
             'pid': statedb.get_job_minion_pid(job_id),
-            'path': statedb.get_job_path(job_id),
+            'path': path,
             'exit': 'no' if exit is None else exit,
             'disk_usage': disk_usage,
             'disk_usage_nice': self.format_bytesize(disk_usage or 0),
diff --git a/lorrycontroller/stopjob.py b/lorrycontroller/stopjob.py
index f2ead87..6472edb 100644
--- a/lorrycontroller/stopjob.py
+++ b/lorrycontroller/stopjob.py
@@ -32,7 +32,7 @@ class StopJob(lorrycontroller.LorryControllerRoute):
         statedb = self.open_statedb()
         with statedb:
             try:
-                path = statedb.find_lorry_running_job(job_id)
+                statedb.find_lorry_running_job(job_id)
             except lorrycontroller.WrongNumberLorriesRunningJob:
                 logging.warning(
                     "Tried to kill job %s which isn't running" % job_id)