Merge ~paride/autopkgtest-cloud:pre-commit-pylint into autopkgtest-cloud:master

[Paride Legovini]

Run pylint via pre-commit and therefore in CI. Fix or disable all the pylint warnings.

[Paride Legovini (paride)]

Individual pylint tags are tackled in separate commits.

Changes that need more attention in review:

* In CI I had to add a PPA where I backported pre-commit to Focal. That's under my LP user. Should I move that under canonical-ubuntu-qa?

* Any better way of installing Python dependencies than the yq trick I used in .launchpad.yaml?

* Do we like the two pylint hooks, one with --disable=import-error (meant for dev machines) and one without that flag (running in CI, where Python deps were installed)?

[Tim Andersson (andersson123)]

Looks like this MP also contains a bunch of linting changes that aren't mentioned in the commits? Presumably from running pre-commit/pylint locally. Is there a way to remove this and put it on a separate commit so we can verify the linting changes are only made by a specific linting program?

[Tim Andersson (andersson123)]

I think we should also move the ppa to canonical-ubuntu-qa
There is no better way to install python deps.
I like the two pylint hooks as long as the one without import errors doesn't run in CI

[Paride Legovini (paride)]

Addressed review comments and rebased, however we need Brian to create a PPA under ~canonical-ubuntu-qa.

[Tim Andersson (andersson123)]

It still looks to me like the linting changes are in the diff or am I wrong? I think this is good to go when we have the new ppa and the linting changes are out

[Paride Legovini (paride)]

I am not sure I understand this question:

> It still looks to me like the linting changes are in the diff or am I wrong?

The proposed branch is mostly made up of linting changes.

Are you suggesting adding pylint to pre-commit/lpci in a MP, and then the "make pylint happy" changes in a separate one? That is doable, but will mean landing a change that will introduce a CI step that will fail which is not nice. And the linting changes and pre-commit/lpci changes are in separate commits.

[Paride Legovini (paride)]

I updated the branch to use the canonical-ubuntu-qa/backport-pre-commit PPA. We'll be able to drop once we switch the infra to Jammy. (I may even backport pre-commit to the -backports pocket at some point, given that all it takes is a straightforward no-change rebuild.)

This is ready for review again.

[Tim Andersson (andersson123)]

re-reviewed, LGTM! Just one question about docstring disables in pylint

diff --git a/.launchpad.yaml b/.launchpad.yaml
index a744b71..ee74e0f 100755
--- a/.launchpad.yaml
+++ b/.launchpad.yaml
@@ -5,12 +5,36 @@ pipeline:
 
 jobs:
   pre_commit:
-    series: jammy
+    series: focal
     architectures: amd64
+    package-repositories:
+      - type: apt
+        formats: [deb]
+        suites: [focal]
+        ppa: canonical-ubuntu-qa/backport-pre-commit
+    snaps:
+      - name: yq
     packages:
       - git
       - pre-commit
-    run: pre-commit run --all-files --show-diff-on-failure
+      - pylint
+      # These are not directly declared in layer.yaml.
+      # Maybe they are brought in by some included layer?
+      - python3-systemd
+      - python3-apt
+    run-before: |
+      # Install dependencies as declared in the layer files
+      DEBIAN_FRONTEND=noninteractive \
+        apt-get install -qy \
+        $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \
+          yq -r '.options.basic.packages[]') \
+        $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \
+          yq -r '.options.apt.packages[]') \
+        $(cat charms/focal/autopkgtest-web/layer.yaml | \
+          yq -r '.options.basic.packages[]') \
+        $(cat charms/focal/autopkgtest-web/layer.yaml | \
+          yq -r '.options.apt.packages[]')
+    run: pre-commit run --hook-stage manual --all-files --show-diff-on-failure
   build_charms:
     series: focal
     architectures: amd64
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 0735120..d3bc39f 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -22,3 +22,28 @@ repos:
     rev: 5.12.0
     hooks:
       - id: isort
+  - repo: local
+    hooks:
+      # Run pylint with --disable import-error.
+      # This is meant to run on developers' machines where not all the
+      # Python modules may be installed.
+      - id: pylint
+        name: pylint
+        stages:
+          [commit-msg, post-checkout, post-commit, post-merge, post-rewrite,
+           pre-commit, pre-merge-commit, pre-push, pre-rebase,
+           prepare-commit-msg]
+        entry: pylint
+        args:
+          - "--disable=import-error"
+        language: system
+        types: [python]
+      # Run pylint without disabling import-error.
+      # This is meant to run in CI (pre-commit run --hook-stage manual).
+      # The CI environment is expected to have all the required dependencies.
+      - id: pylint
+        name: pylint (with import errors)
+        stages: [manual]
+        entry: pylint
+        language: system
+        types: [python]
diff --git a/.pylintrc b/.pylintrc
index a831f0a..b6b77fa 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -2,4 +2,48 @@
 
 jobs=0
 
-disable=invalid-name, import-error, no-name-in-module
+[MESSAGES CONTROL]
+
+disable=
+  R,
+  anomalous-backslash-in-string,
+  arguments-differ,
+  bad-continuation,  # leave this to black
+  bad-option-value,  # for older versions of pylint
+  broad-except,
+  broad-exception-caught,
+  consider-using-f-string,
+  duplicate-string-formatting-argument,
+  fixme,
+  global-statement,
+  global-variable-not-assigned,
+  invalid-name,
+  logging-format-interpolation,
+  logging-fstring-interpolation,
+  logging-not-lazy,
+  missing-class-docstring,
+  missing-function-docstring,
+  missing-module-docstring,
+  missing-timeout,
+  possibly-unused-variable,
+  protected-access,
+  redefined-builtin,
+  redefined-outer-name,
+  unspecified-encoding,
+  unused-argument,
+  wrong-import-order,  # leave this to isort
+
+[REPORTS]
+
+# Show just the errors, no full report
+reports=no
+score=no
+
+[TYPECHECK]
+
+ignored-modules=
+  amulet,
+  charmhelpers,
+  charms,
+  lib,
+  utils,
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances
index 727634e..024ad93 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances
@@ -6,7 +6,6 @@ import re
 import socket
 import subprocess
 import time
-from urllib.error import HTTPError
 
 import novaclient.client
 import novaclient.exceptions
@@ -109,7 +108,6 @@ for instance in nova.servers.list():
             instance.delete()
         except novaclient.exceptions.NotFound:
             logging.warning("Couldn't delete instance: not found")
-            pass
         continue
 
     if not instance.name.startswith("adt-"):
@@ -144,7 +142,6 @@ for instance in nova.servers.list():
             )
         except novaclient.exceptions.NotFound:
             logging.warning("Couldn't delete instance: not found")
-            pass
 
     # check matching adt-run process for instance name
     try:
@@ -180,7 +177,6 @@ for instance in nova.servers.list():
                 )
             except novaclient.exceptions.NotFound:
                 logging.warning("Couldn't delete instance: not found")
-                pass
     except IndexError:
         logging.warning("instance %s has invalid name" % instance.name)
 
@@ -189,4 +185,3 @@ for instance in nova.servers.list():
             influx_client.write_points(measurements)
         except InfluxDBClientError as err:
             logging.warning("Write to InfluxDB failed: %s" % err)
-            pass
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member
index 75e4c74..c512b22 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member
@@ -49,6 +49,7 @@ def usage():
 
 try:
     role = sys.argv[1]
+    to_join = None
     if role in ("leader", "node"):
         to_join = sys.argv[2]
 except IndexError:
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package
index 641f771..e48f3f4 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package
@@ -73,6 +73,7 @@ def setup_image(image_path, source):
     )
 
     # find and install proposed binaries for source
+    # pylint: disable=line-too-long
     img_shell.stdin.write(
         (
             """
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp
index 9ff9df2..76e51ca 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp
@@ -2,9 +2,8 @@
 # Filter out AMQP requests that match a given regex
 
 import logging
-import optparse
+import optparse  # pylint: disable=deprecated-module
 import re
-import sys
 import urllib.parse
 
 import amqplib.client_0_8 as amqp
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream
index 9e49803..c255239 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream
@@ -3,10 +3,8 @@
 
 import json
 import logging
-import optparse
+import optparse  # pylint: disable=deprecated-module
 import os
-import re
-import sys
 import urllib.parse
 from collections import defaultdict
 
@@ -65,6 +63,7 @@ def filter_amqp(options, host):
             try:
                 (delivery_tag, old_submit_time) = seen[pkg][pr]
                 if old_submit_time <= submit_time:
+                    # pylint: disable=line-too-long
                     logging.info(
                         f"{dry_run}We have seen PR {pr} in {queue_name} before: acking the previous request"
                     )
@@ -99,6 +98,7 @@ def main():
         help="additionally show queue items that are not removed",
     )
 
+    # pylint: disable=unused-variable
     opts, args = parser.parse_args()
 
     logging.basicConfig(
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics
index f11d3ca..7224dd3 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics
@@ -84,10 +84,9 @@ def get_units():
             continue
 
         try:
-            (region, arch, n) = name_cloud.split("-", -1)
+            (_, arch, _) = name_cloud.split("-", -1)
         except ValueError:
             # [e-mail]
-            (region, n) = name_cloud.split("-", -1)
             arch = "amd64"
         (active, error) = counts.setdefault(arch, (0, 0))
 
@@ -111,7 +110,7 @@ def get_remotes():
         if not r.startswith("lxd"):
             continue
 
-        (_, arch, ip) = r.split("-", 3)
+        (_, arch, _) = r.split("-", 3)
         (cluster_active, cluster_error) = cluster_counts.setdefault(
             arch, (0, 0)
         )
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test
index 4e258a6..799c905 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test
@@ -17,6 +17,7 @@ p.add_argument(
     help="GitHub PR API URL (e. g. https://api.github.com/repos/JoeDev/coolproj/pulls/1",
 )
 p.add_argument(
+    # pylint: disable=line-too-long
     "test_url",
     help="autopkgtest URL (https://autopkgtest.ubuntu.com/request.cgi?release=xenial&arch=i386&...)",
 )
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
index f59de0e..2584323 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
@@ -122,7 +122,7 @@ def parse_args():
     # verify syntax of triggers
     for t in args.trigger:
         try:
-            (src, ver) = t.split("/")
+            (_, _) = t.split("/")
         except ValueError:
             parser.error(
                 'Invalid trigger format "%s", must be "sourcepkg/version"' % t
@@ -131,7 +131,7 @@ def parse_args():
     # verify syntax of PPAs
     for t in args.ppa:
         try:
-            (user, name) = t.split("/")
+            (_, _) = t.split("/")
         except ValueError:
             parser.error(
                 'Invalid ppa format "%s", must be "lpuser/ppaname"' % t
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release
index 05b2aab..3f14045 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release
@@ -40,6 +40,8 @@ def copy_result(rel_path, source, target):
     to_path = target.split("-")[-1] + rel_path
 
     try:
+        # pylint: disable=used-before-assignment
+        # false positive with older pylint versions
         print("Getting %s" % from_path)
         headers, contents = swift_con.get_object(source, from_path)
     except swiftclient.exceptions.ClientException:
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
index 1bbcbae..2af9dc1 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
@@ -4,6 +4,8 @@
 #
 # Requirements: python3-amqplib python3-swiftclient python3-influxdb
 # Requirements for running autopkgtest from git: python3-debian libdpkg-perl
+#
+# pylint: disable=too-many-lines,line-too-long
 
 import argparse
 import configparser
@@ -267,6 +269,9 @@ def read_per_package_configs(cfg):
             }
 
             for entry in entries:
+                package = None
+                arch = None
+                release = None
                 try:
                     (package, arch, release) = entry.split("/", 3)
                 except ValueError:
@@ -564,9 +569,7 @@ def cleanup_and_sleep(out_dir):
     """Empty the output dir for the next run, otherwise autopkgtest complains"""
     shutil.rmtree(out_dir)
     os.mkdir(out_dir)
-    running_test = False
     time.sleep(300)
-    running_test = True
 
 
 def request(msg):
@@ -799,13 +802,6 @@ def request(msg):
                                 "PPA user %s, name %s has GPG fingerprint %s"
                                 % (ppauser, ppaname, fingerprint)
                             )
-                        except (IOError, ValueError, KeyError) as e:
-                            logging.error(
-                                'Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.'
-                                % e
-                            )
-                            msg.channel.basic_ack(msg.delivery_tag)
-                            return
                         except HTTPError as e:
                             # It's quite common to get 503s from LP; retry a
                             # few times.
@@ -813,6 +809,13 @@ def request(msg):
                                 raise
                             logging.warning("Got error 503 from launchpad API")
                             time.sleep(10)
+                        except (IOError, ValueError, KeyError) as e:
+                            logging.error(
+                                'Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.'
+                                % e
+                            )
+                            msg.channel.basic_ack(msg.delivery_tag)
+                            return
                         else:
                             break
                     else:
diff --git a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
index 6a87cbd..a7f6c19 100644
--- a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
+++ b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
@@ -132,7 +132,7 @@ def get_units():
     lxd_object_paths = defaultdict(lambda: defaultdict(dict))
 
     for unit in units:
-        (name, _, _, active, _, _, object_path, _, _, _) = unit
+        (name, _, _, _, _, _, object_path, _, _, _) = unit
         if name.startswith("build-adt-image@") and name.endswith(".timer"):
             name_release_region_arch = name[16:][:-6]
             (release, region, arch) = name_release_region_arch.split("-", -1)
@@ -334,6 +334,7 @@ def set_up_systemd_units(target_cloud_config, target_lxd_config, releases):
             releases_to_disable = currently_enabled_releases - set(releases)
 
             if releases_to_disable:
+                # pylint: disable=line-too-long
                 print(
                     "Disabling build-adt-image timers for {region}/{arch}/{releases_to_disable}".format(
                         **locals()
diff --git a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
index 12b0b66..2095e42 100644
--- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
+++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
@@ -26,7 +26,6 @@ from charms.reactive import (
     when_not,
     when_not_all,
 )
-from charms.reactive.relations import endpoint_from_flag
 from utils import UnixUser, install_autodep8
 
 AUTOPKGTEST_LOCATION = os.path.expanduser("~ubuntu/autopkgtest")
@@ -131,6 +130,7 @@ def set_up_systemd_units():
         dest = os.path.join(os.path.sep, "etc", "systemd", "system", base)
 
         def link_and_enable():
+            # pylint: disable=cell-var-from-loop
             os.symlink(unit, dest)
             if "@" not in base:
                 subprocess.check_call(["systemctl", "enable", base])
@@ -240,6 +240,7 @@ def clear_rabbitmq():
 
 @when("config.changed.nova-rcs")
 def update_nova_rcs():
+    # pylint: disable=import-outside-toplevel
     import base64
     from io import BytesIO
     from tarfile import TarFile
@@ -303,6 +304,7 @@ def enable_units_initially():
     "config.changed.releases",
 )
 def enable_disable_units():
+    # pylint: disable=import-outside-toplevel
     from lib.systemd import set_up_systemd_units
 
     nworkers = config().get("n-workers") or ""
@@ -422,6 +424,7 @@ def write_swift_config():
 )
 @when_any("config.set.nova-rcs", "config.set.lxd-remotes")
 def write_worker_config():
+    # pylint: disable=import-outside-toplevel
     import configparser
 
     nworkers = config().get("n-workers") or ""
@@ -457,7 +460,7 @@ def write_worker_config():
             "/NET_NAME/": config().get("net-name"),
         }
 
-        for k in replacements:
+        for k in replacements:  # pylint: disable=consider-using-dict-items
             if replacements[k]:
                 s = s.replace(k, replacements[k])
 
@@ -535,6 +538,7 @@ def write_mirror():
 @when("autopkgtest.reload-needed")
 @when_not("autopkgtest.daemon-reload-needed")
 def reload_systemd_units():
+    # pylint: disable=import-outside-toplevel
     from lib.systemd import reload_units
 
     reload_units()
diff --git a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
index ccea20f..842348b 100644
--- a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
+++ b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
@@ -8,7 +8,6 @@ from charmhelpers.core.hookenv import charm_dir, config
 from charms.layer import status
 from charms.reactive import (
     clear_flag,
-    hook,
     set_flag,
     when,
     when_all,
diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
index efdc9b7..d7d7146 100755
--- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi
+++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
@@ -7,7 +7,6 @@ import json
 import os
 import re
 import sqlite3
-import urllib.parse
 from collections import OrderedDict
 from wsgiref.handlers import CGIHandler
 
@@ -168,7 +167,7 @@ def get_source_versions(db, release):
 
     srcs = {}
     for pkg, ver in db.execute(
-        "SELECT package, version " "FROM current_version " "WHERE release = ?",
+        "SELECT package, version FROM current_version WHERE release = ?",
         (release,),
     ):
         srcs[pkg] = ver
@@ -185,6 +184,7 @@ def success_count_for_release_and_arch(db, release, arch, src_versions):
     # but succeeded for a trigger that is not published), don't count it as
     # success
     cur_pkg = None
+    # pylint: disable=unused-variable
     for pkg, triggers, code in db.execute(
         "SELECT test.package, triggers, exitcode "
         "FROM test, result, current_version "
@@ -213,6 +213,7 @@ def success_count_for_release_and_arch(db, release, arch, src_versions):
             # it can happen that src_versions does not have a trigger source
             # pacakge if that trigger source got removed in the final release
             if src_versions.get(src) != ver:
+                # pylint: disable=line-too-long
                 # logging.debug('%s/%s/%s: ignoring non-current trigger %s', release, arch, pkg, trigger)
                 continue
         # if we arrive here, we have a passing test result for pkg with current
@@ -267,6 +268,7 @@ def package_overview(package, _=None):
         package=package,
         releases=[
             release
+            # pylint: disable=consider-iterating-dictionary
             for release in results.keys()
             if release in SUPPORTED_UBUNTU_RELEASES
         ],
@@ -340,7 +342,11 @@ def running():
     for c in queue_info:
         for r in releases:
             for a in arches:
-                (queue_length, queue_items) = (
+                # pylint: disable=unused-variable
+                (
+                    queue_length,
+                    queue_items,
+                ) = (
                     queue_info.get(c, {}).get(r, {}).get(a, (0, []))
                 )
                 queue_lengths.setdefault(c, {}).setdefault(r, {})[
@@ -367,12 +373,12 @@ def running():
 
 @app.route("/queue_size.json")
 def queuesize_json():
-    out = {}
     queue_info = get_queue_info()[2]
     # Strip the number of queue items, this is just their contents
-    for context in queue_info:
+    for context in queue_info:  # pylint: disable=consider-using-dict-items
         for release in queue_info[context]:
             for arch in queue_info[context][release]:
+                # pylint: disable=unused-variable
                 (queue_size, queue_items) = queue_info[context][release][arch]
                 queue_info[context][release][arch] = len(queue_items)
     return json.dumps(queue_info, indent=2)
@@ -380,12 +386,12 @@ def queuesize_json():
 
 @app.route("/queues.json")
 def queues_json():
-    out = {}
     queue_info = get_queue_info()[2]
     # Strip the number of queue items, this is just their contents
-    for context in queue_info:
+    for context in queue_info:  # pylint: disable=consider-using-dict-items
         for release in queue_info[context]:
             for arch in queue_info[context][release]:
+                # pylint: disable=unused-variable
                 (queue_size, queue_items) = queue_info[context][release][arch]
                 queue_info[context][release][arch] = queue_items
     return flask.Response(
@@ -416,7 +422,7 @@ def statistics():
 
     # ensure we don't run into KeyErrors in the jinja template
     data = {}
-    for release in release_arches:
+    for release in release_arches:  # pylint: disable=consider-using-dict-items
         data[release] = {}
         for arch in release_arches[release]:
             data[release][arch] = {}
@@ -450,7 +456,7 @@ def statistics():
             pass
 
     # number of packages with tests that pass
-    for release in release_arches:
+    for release in release_arches:  # pylint: disable=consider-using-dict-items
         sources = get_source_versions(db_con, release)
         for arch in release_arches[release]:
             data[release][arch][
diff --git a/charms/focal/autopkgtest-web/webcontrol/cache-amqp b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
index 1f72747..b626171 100755
--- a/charms/focal/autopkgtest-web/webcontrol/cache-amqp
+++ b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
@@ -62,6 +62,7 @@ class AutopkgtestQueueContents:
                                 routing_key=queue_name,
                             )
                         else:  # not the leader
+                            # pylint: disable=line-too-long
                             logging.error(
                                 "We are not the leader, and there is no semaphore queue yet, we can't do anything - exiting."
                             )
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-all-results b/charms/focal/autopkgtest-web/webcontrol/download-all-results
index 0ae5c4e..53a3983 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-all-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-all-results
@@ -42,10 +42,10 @@ def list_remote_container(container_url):
             url += f"&marker={urllib.parse.quote(start)}"
 
         LOGGER.debug('Retrieving "%s"', url)
-        for retry in range(5):
+        for _ in range(5):
             try:
                 resp = urlopen(url)
-            except http.client.RemoteDisconnected as e:
+            except http.client.RemoteDisconnected:
                 LOGGER.debug("Got disconnected, sleeping")
                 time.sleep(5)
                 continue
@@ -82,6 +82,7 @@ def list_remote_container(container_url):
 def list_our_results(release):
     LOGGER.debug("Finding already recorded results for %s", release)
     c = db_con.cursor()
+    # pylint: disable=line-too-long
     c.execute(
         "SELECT run_id FROM result INNER JOIN test ON test.id = result.test_id WHERE test.release=?",
         (release,),
@@ -108,7 +109,7 @@ def fetch_one_result(url):
         LOGGER.error("Failure to fetch %s: %s", url, str(e))
         # we tolerate "not found" (something went wrong on uploading the
         # result), but other things indicate infrastructure problems
-        if hasattr(e, "code") and e.code == 404:
+        if hasattr(e, "code") and e.code == 404:  # pylint: disable=no-member
             return
         sys.exit(1)
 
@@ -119,7 +120,7 @@ def fetch_one_result(url):
                 srcver = (
                     tar.extractfile("testpkg-version").read().decode().strip()
                 )
-            except KeyError as e:
+            except KeyError:
                 # not found
                 if exitcode in (4, 12, 20):
                     # repair it
@@ -137,7 +138,7 @@ def fetch_one_result(url):
                 requester = (
                     tar.extractfile("requester").read().decode().strip()
                 )
-            except KeyError as e:
+            except KeyError:
                 requester = ""
     except (KeyError, ValueError, tarfile.TarError) as e:
         LOGGER.debug("%s is damaged, ignoring: %s", url, str(e))
@@ -194,8 +195,6 @@ def fetch_one_result(url):
 def fetch_container(release, container_url):
     """Download new results from a swift container"""
 
-    marker = ""
-
     try:
         our_results = list_our_results(release)
         known_results = list_remote_container(container_url)
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-results b/charms/focal/autopkgtest-web/webcontrol/download-results
index 419af47..0f14155 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-results
@@ -7,7 +7,6 @@ import os
 import socket
 import sqlite3
 import urllib.parse
-from urllib.request import urlopen
 
 import amqplib.client_0_8 as amqp
 from helpers.utils import get_test_id, init_db
diff --git a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
index ee7b6d0..9ae1e89 100644
--- a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
+++ b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
@@ -15,13 +15,12 @@ from flask import (
     session,
 )
 from flask_openid import OpenID
+from helpers.utils import setup_key
+from request.submit import Submit
 from werkzeug.middleware.proxy_fix import ProxyFix
 
 sys.path.append("..")
 
-from helpers.utils import setup_key
-from request.submit import Submit
-
 HTML = """
 <!doctype html>
 <html>
diff --git a/charms/focal/autopkgtest-web/webcontrol/publish-db b/charms/focal/autopkgtest-web/webcontrol/publish-db
index f35c9e8..76a2177 100755
--- a/charms/focal/autopkgtest-web/webcontrol/publish-db
+++ b/charms/focal/autopkgtest-web/webcontrol/publish-db
@@ -4,11 +4,9 @@
 # This is being used for statistics.
 
 import configparser
-import fcntl
 import gzip
 import logging
 import os
-import shutil
 import sqlite3
 import tempfile
 import urllib.request
@@ -156,6 +154,7 @@ def get_sources(db_con, release):
                     )
                     temp_file.seek(0)
                     with gzip.open(temp_file) as fd:
+                        # pylint: disable=c-extension-no-member
                         for section in apt_pkg.TagFile(fd):
                             db_con.execute(
                                 "INSERT INTO current_version "
@@ -177,7 +176,6 @@ def get_sources(db_con, release):
             except urllib.error.HTTPError as e:
                 if e.code == 304:
                     logging.debug("url {} not modified".format(url))
-                    pass
 
 
 if __name__ == "__main__":
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
index bc283a7..ac2d5c0 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
@@ -63,6 +63,7 @@ class Submit:
         assert self.amqp_creds.scheme == "amqp"
         logging.debug("AMQP credentials: %s" % repr(self.amqp_creds))
 
+    # pylint: disable=dangerous-default-value
     def validate_distro_request(
         self, release, arch, package, triggers, requester, ppas=[], **kwargs
     ):
@@ -132,10 +133,10 @@ class Submit:
         for trigger in triggers:
             try:
                 trigsrc, trigver = trigger.split("/")
-            except ValueError:
+            except ValueError as exc:
                 raise ValueError(
                     "Malformed trigger, must be srcpackage/version"
-                )
+                ) from exc
             # Debian Policy 5.6.1 and 5.6.12
             if not NAME.match(trigsrc) or not VERSION.match(trigver):
                 raise ValueError("Malformed trigger")
@@ -197,6 +198,7 @@ class Submit:
                 "service." % (package, trigsrc)
             )
 
+    # pylint: disable=dangerous-default-value
     def validate_git_request(
         self, release, arch, package, ppas=[], env=[], **kwargs
     ):
@@ -303,7 +305,7 @@ class Submit:
                 )
 
     @classmethod
-    def post_json(klass, url, data, auth_file, project):
+    def post_json(cls, url, data, auth_file, project):
         """Send POST request with JSON data via basic auth.
 
         'data' is a dictionary which will be posted to 'url' in JSON encoded
@@ -388,7 +390,7 @@ class Submit:
             )
         else:
             c.execute(
-                "SELECT count(arch) FROM test " "WHERE package=? AND arch=?",
+                "SELECT count(arch) FROM test WHERE package=? AND arch=?",
                 (package, arch),
             )
         return c.fetchone()[0] > 0
@@ -459,18 +461,19 @@ class Submit:
         )
         return code >= 200 and code < 300
 
+    # pylint: disable=dangerous-default-value
     def in_allowed_team(self, person, package=[], teams=[]):
         """Check if person is in ALLOWED_TEAMS"""
 
         for team in teams or ALLOWED_TEAMS:
-            (code, response) = self.lp_request("~%s/participants" % team, {})
+            (_, response) = self.lp_request("~%s/participants" % team, {})
             for e in response.get("entries", []):
                 if e.get("name") == person:
                     return True
         return False
 
     @classmethod
-    def lp_request(klass, obj, query):
+    def lp_request(cls, obj, query):
         """Do a Launchpad REST request
 
         Request https://api.launchpad.net/1.0/<obj>?<query>.
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py
index 44067b9..42dac0c 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py
@@ -1,3 +1,4 @@
+# pylint: disable=no-value-for-parameter,no-member
 """Test the Flask app."""
 
 import os
diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
index 334722f..6362803 100755
--- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
+++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
@@ -21,7 +21,7 @@ external_url = None
 
 def result_matches_job(result_url, params):
     # download result.tar and get exit code and testinfo
-    for retry in range(5):
+    for _ in range(5):
         try:
             with urllib.request.urlopen(result_url + "/result.tar") as f:
                 tar_bytes = io.BytesIO(f.read())
diff --git a/mojo/add-floating-ip b/mojo/add-floating-ip
index 57c9839..0a816bf 100755
--- a/mojo/add-floating-ip
+++ b/mojo/add-floating-ip
@@ -8,6 +8,7 @@
 import json
 import os
 import subprocess
+import sys
 
 import novaclient.client
 
@@ -94,14 +95,14 @@ def get_unit_floating_ip(unit_name):
         try:
             # Get the existing one
             fip = nova_tenant.floating_ips.find(ip=myip)
-        except Exception:
+        except Exception as exc:
             # If this happens you're going to need to either get that back in the list,
             # or blow away the state file so it gets a new IP.
             raise (
                 RuntimeError(
                     "Desired IP {} not in floating ips list!".format(myip)
                 )
-            )
+            ) from exc
 
     if fip.instance_id:
         # If it's already associated, ensure it's associated to us
@@ -147,8 +148,6 @@ add-floating-ip
 
 
 def main():
-    import sys
-
     if len(sys.argv) >= 2:
         args = sys.argv[1:]
     elif "targets" in os.environ: