Merge ~paride/autopkgtest-cloud:pre-commit-pylint into autopkgtest-cloud:master
- Git
- lp:~paride/autopkgtest-cloud
- pre-commit-pylint
- Merge into master
Status: | Merged |
---|---|
Merged at revision: | dbef7e8d5b717d51d47ce76bc8ada8057e0acf1b |
Proposed branch: | ~paride/autopkgtest-cloud:pre-commit-pylint |
Merge into: | autopkgtest-cloud:master |
Diff against target: |
845 lines (+168/-65) 26 files modified
.launchpad.yaml (+26/-2) .pre-commit-config.yaml (+25/-0) .pylintrc (+45/-1) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances (+0/-5) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member (+1/-0) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package (+1/-0) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp (+1/-2) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream (+3/-3) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics (+2/-3) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test (+1/-0) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest (+2/-2) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release (+2/-0) charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker (+12/-9) charms/focal/autopkgtest-cloud-worker/lib/systemd.py (+2/-1) charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py (+6/-2) charms/focal/autopkgtest-web/reactive/autopkgtest_web.py (+0/-1) charms/focal/autopkgtest-web/webcontrol/browse.cgi (+15/-9) charms/focal/autopkgtest-web/webcontrol/cache-amqp (+1/-0) charms/focal/autopkgtest-web/webcontrol/download-all-results (+6/-7) charms/focal/autopkgtest-web/webcontrol/download-results (+0/-1) charms/focal/autopkgtest-web/webcontrol/private_results/app.py (+2/-3) charms/focal/autopkgtest-web/webcontrol/publish-db (+1/-3) charms/focal/autopkgtest-web/webcontrol/request/submit.py (+9/-6) charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py (+1/-0) charms/focal/autopkgtest-web/webcontrol/update-github-jobs (+1/-1) mojo/add-floating-ip (+3/-4) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Andersson | Pending | ||
Canonical's Ubuntu QA | Pending | ||
Review via email: mp+445630@code.launchpad.net |
Commit message
Run pylint via pre-commit and therefore in CI. Fix or disable all the pylint warnings.
Description of the change
Paride Legovini (paride) wrote : | # |
Tim Andersson (andersson123) : | # |
Tim Andersson (andersson123) : | # |
Tim Andersson (andersson123) : | # |
Tim Andersson (andersson123) : | # |
Tim Andersson (andersson123) wrote : | # |
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) wrote : | # |
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) : | # |
Paride Legovini (paride) wrote : | # |
Addressed review comments and rebased, however we need Brian to create a PPA under ~canonical-
Tim Andersson (andersson123) wrote : | # |
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) wrote : | # |
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) wrote : | # |
I updated the branch to use the canonical-
This is ready for review again.
Tim Andersson (andersson123) : | # |
Tim Andersson (andersson123) wrote : | # |
re-reviewed, LGTM! Just one question about docstring disables in pylint
Paride Legovini (paride) : | # |
Preview Diff
1 | diff --git a/.launchpad.yaml b/.launchpad.yaml |
2 | index a744b71..ee74e0f 100755 |
3 | --- a/.launchpad.yaml |
4 | +++ b/.launchpad.yaml |
5 | @@ -5,12 +5,36 @@ pipeline: |
6 | |
7 | jobs: |
8 | pre_commit: |
9 | - series: jammy |
10 | + series: focal |
11 | architectures: amd64 |
12 | + package-repositories: |
13 | + - type: apt |
14 | + formats: [deb] |
15 | + suites: [focal] |
16 | + ppa: canonical-ubuntu-qa/backport-pre-commit |
17 | + snaps: |
18 | + - name: yq |
19 | packages: |
20 | - git |
21 | - pre-commit |
22 | - run: pre-commit run --all-files --show-diff-on-failure |
23 | + - pylint |
24 | + # These are not directly declared in layer.yaml. |
25 | + # Maybe they are brought in by some included layer? |
26 | + - python3-systemd |
27 | + - python3-apt |
28 | + run-before: | |
29 | + # Install dependencies as declared in the layer files |
30 | + DEBIAN_FRONTEND=noninteractive \ |
31 | + apt-get install -qy \ |
32 | + $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \ |
33 | + yq -r '.options.basic.packages[]') \ |
34 | + $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \ |
35 | + yq -r '.options.apt.packages[]') \ |
36 | + $(cat charms/focal/autopkgtest-web/layer.yaml | \ |
37 | + yq -r '.options.basic.packages[]') \ |
38 | + $(cat charms/focal/autopkgtest-web/layer.yaml | \ |
39 | + yq -r '.options.apt.packages[]') |
40 | + run: pre-commit run --hook-stage manual --all-files --show-diff-on-failure |
41 | build_charms: |
42 | series: focal |
43 | architectures: amd64 |
44 | diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml |
45 | index 0735120..d3bc39f 100644 |
46 | --- a/.pre-commit-config.yaml |
47 | +++ b/.pre-commit-config.yaml |
48 | @@ -22,3 +22,28 @@ repos: |
49 | rev: 5.12.0 |
50 | hooks: |
51 | - id: isort |
52 | + - repo: local |
53 | + hooks: |
54 | + # Run pylint with --disable import-error. |
55 | + # This is meant to run on developers' machines where not all the |
56 | + # Python modules may be installed. |
57 | + - id: pylint |
58 | + name: pylint |
59 | + stages: |
60 | + [commit-msg, post-checkout, post-commit, post-merge, post-rewrite, |
61 | + pre-commit, pre-merge-commit, pre-push, pre-rebase, |
62 | + prepare-commit-msg] |
63 | + entry: pylint |
64 | + args: |
65 | + - "--disable=import-error" |
66 | + language: system |
67 | + types: [python] |
68 | + # Run pylint without disabling import-error. |
69 | + # This is meant to run in CI (pre-commit run --hook-stage manual). |
70 | + # The CI environment is expected to have all the required dependencies. |
71 | + - id: pylint |
72 | + name: pylint (with import errors) |
73 | + stages: [manual] |
74 | + entry: pylint |
75 | + language: system |
76 | + types: [python] |
77 | diff --git a/.pylintrc b/.pylintrc |
78 | index a831f0a..b6b77fa 100644 |
79 | --- a/.pylintrc |
80 | +++ b/.pylintrc |
81 | @@ -2,4 +2,48 @@ |
82 | |
83 | jobs=0 |
84 | |
85 | -disable=invalid-name, import-error, no-name-in-module |
86 | +[MESSAGES CONTROL] |
87 | + |
88 | +disable= |
89 | + R, |
90 | + anomalous-backslash-in-string, |
91 | + arguments-differ, |
92 | + bad-continuation, # leave this to black |
93 | + bad-option-value, # for older versions of pylint |
94 | + broad-except, |
95 | + broad-exception-caught, |
96 | + consider-using-f-string, |
97 | + duplicate-string-formatting-argument, |
98 | + fixme, |
99 | + global-statement, |
100 | + global-variable-not-assigned, |
101 | + invalid-name, |
102 | + logging-format-interpolation, |
103 | + logging-fstring-interpolation, |
104 | + logging-not-lazy, |
105 | + missing-class-docstring, |
106 | + missing-function-docstring, |
107 | + missing-module-docstring, |
108 | + missing-timeout, |
109 | + possibly-unused-variable, |
110 | + protected-access, |
111 | + redefined-builtin, |
112 | + redefined-outer-name, |
113 | + unspecified-encoding, |
114 | + unused-argument, |
115 | + wrong-import-order, # leave this to isort |
116 | + |
117 | +[REPORTS] |
118 | + |
119 | +# Show just the errors, no full report |
120 | +reports=no |
121 | +score=no |
122 | + |
123 | +[TYPECHECK] |
124 | + |
125 | +ignored-modules= |
126 | + amulet, |
127 | + charmhelpers, |
128 | + charms, |
129 | + lib, |
130 | + utils, |
131 | diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances |
132 | index 727634e..024ad93 100755 |
133 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances |
134 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/cleanup-instances |
135 | @@ -6,7 +6,6 @@ import re |
136 | import socket |
137 | import subprocess |
138 | import time |
139 | -from urllib.error import HTTPError |
140 | |
141 | import novaclient.client |
142 | import novaclient.exceptions |
143 | @@ -109,7 +108,6 @@ for instance in nova.servers.list(): |
144 | instance.delete() |
145 | except novaclient.exceptions.NotFound: |
146 | logging.warning("Couldn't delete instance: not found") |
147 | - pass |
148 | continue |
149 | |
150 | if not instance.name.startswith("adt-"): |
151 | @@ -144,7 +142,6 @@ for instance in nova.servers.list(): |
152 | ) |
153 | except novaclient.exceptions.NotFound: |
154 | logging.warning("Couldn't delete instance: not found") |
155 | - pass |
156 | |
157 | # check matching adt-run process for instance name |
158 | try: |
159 | @@ -180,7 +177,6 @@ for instance in nova.servers.list(): |
160 | ) |
161 | except novaclient.exceptions.NotFound: |
162 | logging.warning("Couldn't delete instance: not found") |
163 | - pass |
164 | except IndexError: |
165 | logging.warning("instance %s has invalid name" % instance.name) |
166 | |
167 | @@ -189,4 +185,3 @@ for instance in nova.servers.list(): |
168 | influx_client.write_points(measurements) |
169 | except InfluxDBClientError as err: |
170 | logging.warning("Write to InfluxDB failed: %s" % err) |
171 | - pass |
172 | 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 |
173 | index 75e4c74..c512b22 100755 |
174 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member |
175 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-armhf-cluster-member |
176 | @@ -49,6 +49,7 @@ def usage(): |
177 | |
178 | try: |
179 | role = sys.argv[1] |
180 | + to_join = None |
181 | if role in ("leader", "node"): |
182 | to_join = sys.argv[2] |
183 | except IndexError: |
184 | 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 |
185 | index 641f771..e48f3f4 100755 |
186 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package |
187 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-with-proposed-package |
188 | @@ -73,6 +73,7 @@ def setup_image(image_path, source): |
189 | ) |
190 | |
191 | # find and install proposed binaries for source |
192 | + # pylint: disable=line-too-long |
193 | img_shell.stdin.write( |
194 | ( |
195 | """ |
196 | diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp |
197 | index 9ff9df2..76e51ca 100755 |
198 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp |
199 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp |
200 | @@ -2,9 +2,8 @@ |
201 | # Filter out AMQP requests that match a given regex |
202 | |
203 | import logging |
204 | -import optparse |
205 | +import optparse # pylint: disable=deprecated-module |
206 | import re |
207 | -import sys |
208 | import urllib.parse |
209 | |
210 | import amqplib.client_0_8 as amqp |
211 | 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 |
212 | index 9e49803..c255239 100755 |
213 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream |
214 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/filter-amqp-dupes-upstream |
215 | @@ -3,10 +3,8 @@ |
216 | |
217 | import json |
218 | import logging |
219 | -import optparse |
220 | +import optparse # pylint: disable=deprecated-module |
221 | import os |
222 | -import re |
223 | -import sys |
224 | import urllib.parse |
225 | from collections import defaultdict |
226 | |
227 | @@ -65,6 +63,7 @@ def filter_amqp(options, host): |
228 | try: |
229 | (delivery_tag, old_submit_time) = seen[pkg][pr] |
230 | if old_submit_time <= submit_time: |
231 | + # pylint: disable=line-too-long |
232 | logging.info( |
233 | f"{dry_run}We have seen PR {pr} in {queue_name} before: acking the previous request" |
234 | ) |
235 | @@ -99,6 +98,7 @@ def main(): |
236 | help="additionally show queue items that are not removed", |
237 | ) |
238 | |
239 | + # pylint: disable=unused-variable |
240 | opts, args = parser.parse_args() |
241 | |
242 | logging.basicConfig( |
243 | diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics |
244 | index f11d3ca..7224dd3 100755 |
245 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics |
246 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/metrics |
247 | @@ -84,10 +84,9 @@ def get_units(): |
248 | continue |
249 | |
250 | try: |
251 | - (region, arch, n) = name_cloud.split("-", -1) |
252 | + (_, arch, _) = name_cloud.split("-", -1) |
253 | except ValueError: |
254 | # autopkgtest@lcy01-1.service |
255 | - (region, n) = name_cloud.split("-", -1) |
256 | arch = "amd64" |
257 | (active, error) = counts.setdefault(arch, (0, 0)) |
258 | |
259 | @@ -111,7 +110,7 @@ def get_remotes(): |
260 | if not r.startswith("lxd"): |
261 | continue |
262 | |
263 | - (_, arch, ip) = r.split("-", 3) |
264 | + (_, arch, _) = r.split("-", 3) |
265 | (cluster_active, cluster_error) = cluster_counts.setdefault( |
266 | arch, (0, 0) |
267 | ) |
268 | 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 |
269 | index 4e258a6..799c905 100755 |
270 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test |
271 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/retry-github-test |
272 | @@ -17,6 +17,7 @@ p.add_argument( |
273 | help="GitHub PR API URL (e. g. https://api.github.com/repos/JoeDev/coolproj/pulls/1", |
274 | ) |
275 | p.add_argument( |
276 | + # pylint: disable=line-too-long |
277 | "test_url", |
278 | help="autopkgtest URL (https://autopkgtest.ubuntu.com/request.cgi?release=xenial&arch=i386&...)", |
279 | ) |
280 | diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest |
281 | index f59de0e..2584323 100755 |
282 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest |
283 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest |
284 | @@ -122,7 +122,7 @@ def parse_args(): |
285 | # verify syntax of triggers |
286 | for t in args.trigger: |
287 | try: |
288 | - (src, ver) = t.split("/") |
289 | + (_, _) = t.split("/") |
290 | except ValueError: |
291 | parser.error( |
292 | 'Invalid trigger format "%s", must be "sourcepkg/version"' % t |
293 | @@ -131,7 +131,7 @@ def parse_args(): |
294 | # verify syntax of PPAs |
295 | for t in args.ppa: |
296 | try: |
297 | - (user, name) = t.split("/") |
298 | + (_, _) = t.split("/") |
299 | except ValueError: |
300 | parser.error( |
301 | 'Invalid ppa format "%s", must be "lpuser/ppaname"' % t |
302 | 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 |
303 | index 05b2aab..3f14045 100755 |
304 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release |
305 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/seed-new-release |
306 | @@ -40,6 +40,8 @@ def copy_result(rel_path, source, target): |
307 | to_path = target.split("-")[-1] + rel_path |
308 | |
309 | try: |
310 | + # pylint: disable=used-before-assignment |
311 | + # false positive with older pylint versions |
312 | print("Getting %s" % from_path) |
313 | headers, contents = swift_con.get_object(source, from_path) |
314 | except swiftclient.exceptions.ClientException: |
315 | diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker |
316 | index 1bbcbae..2af9dc1 100755 |
317 | --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker |
318 | +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker |
319 | @@ -4,6 +4,8 @@ |
320 | # |
321 | # Requirements: python3-amqplib python3-swiftclient python3-influxdb |
322 | # Requirements for running autopkgtest from git: python3-debian libdpkg-perl |
323 | +# |
324 | +# pylint: disable=too-many-lines,line-too-long |
325 | |
326 | import argparse |
327 | import configparser |
328 | @@ -267,6 +269,9 @@ def read_per_package_configs(cfg): |
329 | } |
330 | |
331 | for entry in entries: |
332 | + package = None |
333 | + arch = None |
334 | + release = None |
335 | try: |
336 | (package, arch, release) = entry.split("/", 3) |
337 | except ValueError: |
338 | @@ -564,9 +569,7 @@ def cleanup_and_sleep(out_dir): |
339 | """Empty the output dir for the next run, otherwise autopkgtest complains""" |
340 | shutil.rmtree(out_dir) |
341 | os.mkdir(out_dir) |
342 | - running_test = False |
343 | time.sleep(300) |
344 | - running_test = True |
345 | |
346 | |
347 | def request(msg): |
348 | @@ -799,13 +802,6 @@ def request(msg): |
349 | "PPA user %s, name %s has GPG fingerprint %s" |
350 | % (ppauser, ppaname, fingerprint) |
351 | ) |
352 | - except (IOError, ValueError, KeyError) as e: |
353 | - logging.error( |
354 | - 'Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.' |
355 | - % e |
356 | - ) |
357 | - msg.channel.basic_ack(msg.delivery_tag) |
358 | - return |
359 | except HTTPError as e: |
360 | # It's quite common to get 503s from LP; retry a |
361 | # few times. |
362 | @@ -813,6 +809,13 @@ def request(msg): |
363 | raise |
364 | logging.warning("Got error 503 from launchpad API") |
365 | time.sleep(10) |
366 | + except (IOError, ValueError, KeyError) as e: |
367 | + logging.error( |
368 | + 'Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.' |
369 | + % e |
370 | + ) |
371 | + msg.channel.basic_ack(msg.delivery_tag) |
372 | + return |
373 | else: |
374 | break |
375 | else: |
376 | diff --git a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py |
377 | index 6a87cbd..a7f6c19 100644 |
378 | --- a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py |
379 | +++ b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py |
380 | @@ -132,7 +132,7 @@ def get_units(): |
381 | lxd_object_paths = defaultdict(lambda: defaultdict(dict)) |
382 | |
383 | for unit in units: |
384 | - (name, _, _, active, _, _, object_path, _, _, _) = unit |
385 | + (name, _, _, _, _, _, object_path, _, _, _) = unit |
386 | if name.startswith("build-adt-image@") and name.endswith(".timer"): |
387 | name_release_region_arch = name[16:][:-6] |
388 | (release, region, arch) = name_release_region_arch.split("-", -1) |
389 | @@ -334,6 +334,7 @@ def set_up_systemd_units(target_cloud_config, target_lxd_config, releases): |
390 | releases_to_disable = currently_enabled_releases - set(releases) |
391 | |
392 | if releases_to_disable: |
393 | + # pylint: disable=line-too-long |
394 | print( |
395 | "Disabling build-adt-image timers for {region}/{arch}/{releases_to_disable}".format( |
396 | **locals() |
397 | diff --git a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py |
398 | index 12b0b66..2095e42 100644 |
399 | --- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py |
400 | +++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py |
401 | @@ -26,7 +26,6 @@ from charms.reactive import ( |
402 | when_not, |
403 | when_not_all, |
404 | ) |
405 | -from charms.reactive.relations import endpoint_from_flag |
406 | from utils import UnixUser, install_autodep8 |
407 | |
408 | AUTOPKGTEST_LOCATION = os.path.expanduser("~ubuntu/autopkgtest") |
409 | @@ -131,6 +130,7 @@ def set_up_systemd_units(): |
410 | dest = os.path.join(os.path.sep, "etc", "systemd", "system", base) |
411 | |
412 | def link_and_enable(): |
413 | + # pylint: disable=cell-var-from-loop |
414 | os.symlink(unit, dest) |
415 | if "@" not in base: |
416 | subprocess.check_call(["systemctl", "enable", base]) |
417 | @@ -240,6 +240,7 @@ def clear_rabbitmq(): |
418 | |
419 | @when("config.changed.nova-rcs") |
420 | def update_nova_rcs(): |
421 | + # pylint: disable=import-outside-toplevel |
422 | import base64 |
423 | from io import BytesIO |
424 | from tarfile import TarFile |
425 | @@ -303,6 +304,7 @@ def enable_units_initially(): |
426 | "config.changed.releases", |
427 | ) |
428 | def enable_disable_units(): |
429 | + # pylint: disable=import-outside-toplevel |
430 | from lib.systemd import set_up_systemd_units |
431 | |
432 | nworkers = config().get("n-workers") or "" |
433 | @@ -422,6 +424,7 @@ def write_swift_config(): |
434 | ) |
435 | @when_any("config.set.nova-rcs", "config.set.lxd-remotes") |
436 | def write_worker_config(): |
437 | + # pylint: disable=import-outside-toplevel |
438 | import configparser |
439 | |
440 | nworkers = config().get("n-workers") or "" |
441 | @@ -457,7 +460,7 @@ def write_worker_config(): |
442 | "/NET_NAME/": config().get("net-name"), |
443 | } |
444 | |
445 | - for k in replacements: |
446 | + for k in replacements: # pylint: disable=consider-using-dict-items |
447 | if replacements[k]: |
448 | s = s.replace(k, replacements[k]) |
449 | |
450 | @@ -535,6 +538,7 @@ def write_mirror(): |
451 | @when("autopkgtest.reload-needed") |
452 | @when_not("autopkgtest.daemon-reload-needed") |
453 | def reload_systemd_units(): |
454 | + # pylint: disable=import-outside-toplevel |
455 | from lib.systemd import reload_units |
456 | |
457 | reload_units() |
458 | diff --git a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py |
459 | index ccea20f..842348b 100644 |
460 | --- a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py |
461 | +++ b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py |
462 | @@ -8,7 +8,6 @@ from charmhelpers.core.hookenv import charm_dir, config |
463 | from charms.layer import status |
464 | from charms.reactive import ( |
465 | clear_flag, |
466 | - hook, |
467 | set_flag, |
468 | when, |
469 | when_all, |
470 | diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi |
471 | index efdc9b7..d7d7146 100755 |
472 | --- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi |
473 | +++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi |
474 | @@ -7,7 +7,6 @@ import json |
475 | import os |
476 | import re |
477 | import sqlite3 |
478 | -import urllib.parse |
479 | from collections import OrderedDict |
480 | from wsgiref.handlers import CGIHandler |
481 | |
482 | @@ -168,7 +167,7 @@ def get_source_versions(db, release): |
483 | |
484 | srcs = {} |
485 | for pkg, ver in db.execute( |
486 | - "SELECT package, version " "FROM current_version " "WHERE release = ?", |
487 | + "SELECT package, version FROM current_version WHERE release = ?", |
488 | (release,), |
489 | ): |
490 | srcs[pkg] = ver |
491 | @@ -185,6 +184,7 @@ def success_count_for_release_and_arch(db, release, arch, src_versions): |
492 | # but succeeded for a trigger that is not published), don't count it as |
493 | # success |
494 | cur_pkg = None |
495 | + # pylint: disable=unused-variable |
496 | for pkg, triggers, code in db.execute( |
497 | "SELECT test.package, triggers, exitcode " |
498 | "FROM test, result, current_version " |
499 | @@ -213,6 +213,7 @@ def success_count_for_release_and_arch(db, release, arch, src_versions): |
500 | # it can happen that src_versions does not have a trigger source |
501 | # pacakge if that trigger source got removed in the final release |
502 | if src_versions.get(src) != ver: |
503 | + # pylint: disable=line-too-long |
504 | # logging.debug('%s/%s/%s: ignoring non-current trigger %s', release, arch, pkg, trigger) |
505 | continue |
506 | # if we arrive here, we have a passing test result for pkg with current |
507 | @@ -267,6 +268,7 @@ def package_overview(package, _=None): |
508 | package=package, |
509 | releases=[ |
510 | release |
511 | + # pylint: disable=consider-iterating-dictionary |
512 | for release in results.keys() |
513 | if release in SUPPORTED_UBUNTU_RELEASES |
514 | ], |
515 | @@ -340,7 +342,11 @@ def running(): |
516 | for c in queue_info: |
517 | for r in releases: |
518 | for a in arches: |
519 | - (queue_length, queue_items) = ( |
520 | + # pylint: disable=unused-variable |
521 | + ( |
522 | + queue_length, |
523 | + queue_items, |
524 | + ) = ( |
525 | queue_info.get(c, {}).get(r, {}).get(a, (0, [])) |
526 | ) |
527 | queue_lengths.setdefault(c, {}).setdefault(r, {})[ |
528 | @@ -367,12 +373,12 @@ def running(): |
529 | |
530 | @app.route("/queue_size.json") |
531 | def queuesize_json(): |
532 | - out = {} |
533 | queue_info = get_queue_info()[2] |
534 | # Strip the number of queue items, this is just their contents |
535 | - for context in queue_info: |
536 | + for context in queue_info: # pylint: disable=consider-using-dict-items |
537 | for release in queue_info[context]: |
538 | for arch in queue_info[context][release]: |
539 | + # pylint: disable=unused-variable |
540 | (queue_size, queue_items) = queue_info[context][release][arch] |
541 | queue_info[context][release][arch] = len(queue_items) |
542 | return json.dumps(queue_info, indent=2) |
543 | @@ -380,12 +386,12 @@ def queuesize_json(): |
544 | |
545 | @app.route("/queues.json") |
546 | def queues_json(): |
547 | - out = {} |
548 | queue_info = get_queue_info()[2] |
549 | # Strip the number of queue items, this is just their contents |
550 | - for context in queue_info: |
551 | + for context in queue_info: # pylint: disable=consider-using-dict-items |
552 | for release in queue_info[context]: |
553 | for arch in queue_info[context][release]: |
554 | + # pylint: disable=unused-variable |
555 | (queue_size, queue_items) = queue_info[context][release][arch] |
556 | queue_info[context][release][arch] = queue_items |
557 | return flask.Response( |
558 | @@ -416,7 +422,7 @@ def statistics(): |
559 | |
560 | # ensure we don't run into KeyErrors in the jinja template |
561 | data = {} |
562 | - for release in release_arches: |
563 | + for release in release_arches: # pylint: disable=consider-using-dict-items |
564 | data[release] = {} |
565 | for arch in release_arches[release]: |
566 | data[release][arch] = {} |
567 | @@ -450,7 +456,7 @@ def statistics(): |
568 | pass |
569 | |
570 | # number of packages with tests that pass |
571 | - for release in release_arches: |
572 | + for release in release_arches: # pylint: disable=consider-using-dict-items |
573 | sources = get_source_versions(db_con, release) |
574 | for arch in release_arches[release]: |
575 | data[release][arch][ |
576 | diff --git a/charms/focal/autopkgtest-web/webcontrol/cache-amqp b/charms/focal/autopkgtest-web/webcontrol/cache-amqp |
577 | index 1f72747..b626171 100755 |
578 | --- a/charms/focal/autopkgtest-web/webcontrol/cache-amqp |
579 | +++ b/charms/focal/autopkgtest-web/webcontrol/cache-amqp |
580 | @@ -62,6 +62,7 @@ class AutopkgtestQueueContents: |
581 | routing_key=queue_name, |
582 | ) |
583 | else: # not the leader |
584 | + # pylint: disable=line-too-long |
585 | logging.error( |
586 | "We are not the leader, and there is no semaphore queue yet, we can't do anything - exiting." |
587 | ) |
588 | diff --git a/charms/focal/autopkgtest-web/webcontrol/download-all-results b/charms/focal/autopkgtest-web/webcontrol/download-all-results |
589 | index 0ae5c4e..53a3983 100755 |
590 | --- a/charms/focal/autopkgtest-web/webcontrol/download-all-results |
591 | +++ b/charms/focal/autopkgtest-web/webcontrol/download-all-results |
592 | @@ -42,10 +42,10 @@ def list_remote_container(container_url): |
593 | url += f"&marker={urllib.parse.quote(start)}" |
594 | |
595 | LOGGER.debug('Retrieving "%s"', url) |
596 | - for retry in range(5): |
597 | + for _ in range(5): |
598 | try: |
599 | resp = urlopen(url) |
600 | - except http.client.RemoteDisconnected as e: |
601 | + except http.client.RemoteDisconnected: |
602 | LOGGER.debug("Got disconnected, sleeping") |
603 | time.sleep(5) |
604 | continue |
605 | @@ -82,6 +82,7 @@ def list_remote_container(container_url): |
606 | def list_our_results(release): |
607 | LOGGER.debug("Finding already recorded results for %s", release) |
608 | c = db_con.cursor() |
609 | + # pylint: disable=line-too-long |
610 | c.execute( |
611 | "SELECT run_id FROM result INNER JOIN test ON test.id = result.test_id WHERE test.release=?", |
612 | (release,), |
613 | @@ -108,7 +109,7 @@ def fetch_one_result(url): |
614 | LOGGER.error("Failure to fetch %s: %s", url, str(e)) |
615 | # we tolerate "not found" (something went wrong on uploading the |
616 | # result), but other things indicate infrastructure problems |
617 | - if hasattr(e, "code") and e.code == 404: |
618 | + if hasattr(e, "code") and e.code == 404: # pylint: disable=no-member |
619 | return |
620 | sys.exit(1) |
621 | |
622 | @@ -119,7 +120,7 @@ def fetch_one_result(url): |
623 | srcver = ( |
624 | tar.extractfile("testpkg-version").read().decode().strip() |
625 | ) |
626 | - except KeyError as e: |
627 | + except KeyError: |
628 | # not found |
629 | if exitcode in (4, 12, 20): |
630 | # repair it |
631 | @@ -137,7 +138,7 @@ def fetch_one_result(url): |
632 | requester = ( |
633 | tar.extractfile("requester").read().decode().strip() |
634 | ) |
635 | - except KeyError as e: |
636 | + except KeyError: |
637 | requester = "" |
638 | except (KeyError, ValueError, tarfile.TarError) as e: |
639 | LOGGER.debug("%s is damaged, ignoring: %s", url, str(e)) |
640 | @@ -194,8 +195,6 @@ def fetch_one_result(url): |
641 | def fetch_container(release, container_url): |
642 | """Download new results from a swift container""" |
643 | |
644 | - marker = "" |
645 | - |
646 | try: |
647 | our_results = list_our_results(release) |
648 | known_results = list_remote_container(container_url) |
649 | diff --git a/charms/focal/autopkgtest-web/webcontrol/download-results b/charms/focal/autopkgtest-web/webcontrol/download-results |
650 | index 419af47..0f14155 100755 |
651 | --- a/charms/focal/autopkgtest-web/webcontrol/download-results |
652 | +++ b/charms/focal/autopkgtest-web/webcontrol/download-results |
653 | @@ -7,7 +7,6 @@ import os |
654 | import socket |
655 | import sqlite3 |
656 | import urllib.parse |
657 | -from urllib.request import urlopen |
658 | |
659 | import amqplib.client_0_8 as amqp |
660 | from helpers.utils import get_test_id, init_db |
661 | diff --git a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py |
662 | index ee7b6d0..9ae1e89 100644 |
663 | --- a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py |
664 | +++ b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py |
665 | @@ -15,13 +15,12 @@ from flask import ( |
666 | session, |
667 | ) |
668 | from flask_openid import OpenID |
669 | +from helpers.utils import setup_key |
670 | +from request.submit import Submit |
671 | from werkzeug.middleware.proxy_fix import ProxyFix |
672 | |
673 | sys.path.append("..") |
674 | |
675 | -from helpers.utils import setup_key |
676 | -from request.submit import Submit |
677 | - |
678 | HTML = """ |
679 | <!doctype html> |
680 | <html> |
681 | diff --git a/charms/focal/autopkgtest-web/webcontrol/publish-db b/charms/focal/autopkgtest-web/webcontrol/publish-db |
682 | index f35c9e8..76a2177 100755 |
683 | --- a/charms/focal/autopkgtest-web/webcontrol/publish-db |
684 | +++ b/charms/focal/autopkgtest-web/webcontrol/publish-db |
685 | @@ -4,11 +4,9 @@ |
686 | # This is being used for statistics. |
687 | |
688 | import configparser |
689 | -import fcntl |
690 | import gzip |
691 | import logging |
692 | import os |
693 | -import shutil |
694 | import sqlite3 |
695 | import tempfile |
696 | import urllib.request |
697 | @@ -156,6 +154,7 @@ def get_sources(db_con, release): |
698 | ) |
699 | temp_file.seek(0) |
700 | with gzip.open(temp_file) as fd: |
701 | + # pylint: disable=c-extension-no-member |
702 | for section in apt_pkg.TagFile(fd): |
703 | db_con.execute( |
704 | "INSERT INTO current_version " |
705 | @@ -177,7 +176,6 @@ def get_sources(db_con, release): |
706 | except urllib.error.HTTPError as e: |
707 | if e.code == 304: |
708 | logging.debug("url {} not modified".format(url)) |
709 | - pass |
710 | |
711 | |
712 | if __name__ == "__main__": |
713 | diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py |
714 | index bc283a7..ac2d5c0 100644 |
715 | --- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py |
716 | +++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py |
717 | @@ -63,6 +63,7 @@ class Submit: |
718 | assert self.amqp_creds.scheme == "amqp" |
719 | logging.debug("AMQP credentials: %s" % repr(self.amqp_creds)) |
720 | |
721 | + # pylint: disable=dangerous-default-value |
722 | def validate_distro_request( |
723 | self, release, arch, package, triggers, requester, ppas=[], **kwargs |
724 | ): |
725 | @@ -132,10 +133,10 @@ class Submit: |
726 | for trigger in triggers: |
727 | try: |
728 | trigsrc, trigver = trigger.split("/") |
729 | - except ValueError: |
730 | + except ValueError as exc: |
731 | raise ValueError( |
732 | "Malformed trigger, must be srcpackage/version" |
733 | - ) |
734 | + ) from exc |
735 | # Debian Policy 5.6.1 and 5.6.12 |
736 | if not NAME.match(trigsrc) or not VERSION.match(trigver): |
737 | raise ValueError("Malformed trigger") |
738 | @@ -197,6 +198,7 @@ class Submit: |
739 | "service." % (package, trigsrc) |
740 | ) |
741 | |
742 | + # pylint: disable=dangerous-default-value |
743 | def validate_git_request( |
744 | self, release, arch, package, ppas=[], env=[], **kwargs |
745 | ): |
746 | @@ -303,7 +305,7 @@ class Submit: |
747 | ) |
748 | |
749 | @classmethod |
750 | - def post_json(klass, url, data, auth_file, project): |
751 | + def post_json(cls, url, data, auth_file, project): |
752 | """Send POST request with JSON data via basic auth. |
753 | |
754 | 'data' is a dictionary which will be posted to 'url' in JSON encoded |
755 | @@ -388,7 +390,7 @@ class Submit: |
756 | ) |
757 | else: |
758 | c.execute( |
759 | - "SELECT count(arch) FROM test " "WHERE package=? AND arch=?", |
760 | + "SELECT count(arch) FROM test WHERE package=? AND arch=?", |
761 | (package, arch), |
762 | ) |
763 | return c.fetchone()[0] > 0 |
764 | @@ -459,18 +461,19 @@ class Submit: |
765 | ) |
766 | return code >= 200 and code < 300 |
767 | |
768 | + # pylint: disable=dangerous-default-value |
769 | def in_allowed_team(self, person, package=[], teams=[]): |
770 | """Check if person is in ALLOWED_TEAMS""" |
771 | |
772 | for team in teams or ALLOWED_TEAMS: |
773 | - (code, response) = self.lp_request("~%s/participants" % team, {}) |
774 | + (_, response) = self.lp_request("~%s/participants" % team, {}) |
775 | for e in response.get("entries", []): |
776 | if e.get("name") == person: |
777 | return True |
778 | return False |
779 | |
780 | @classmethod |
781 | - def lp_request(klass, obj, query): |
782 | + def lp_request(cls, obj, query): |
783 | """Do a Launchpad REST request |
784 | |
785 | Request https://api.launchpad.net/1.0/<obj>?<query>. |
786 | diff --git a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py |
787 | index 44067b9..42dac0c 100644 |
788 | --- a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py |
789 | +++ b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_app.py |
790 | @@ -1,3 +1,4 @@ |
791 | +# pylint: disable=no-value-for-parameter,no-member |
792 | """Test the Flask app.""" |
793 | |
794 | import os |
795 | diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs |
796 | index 334722f..6362803 100755 |
797 | --- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs |
798 | +++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs |
799 | @@ -21,7 +21,7 @@ external_url = None |
800 | |
801 | def result_matches_job(result_url, params): |
802 | # download result.tar and get exit code and testinfo |
803 | - for retry in range(5): |
804 | + for _ in range(5): |
805 | try: |
806 | with urllib.request.urlopen(result_url + "/result.tar") as f: |
807 | tar_bytes = io.BytesIO(f.read()) |
808 | diff --git a/mojo/add-floating-ip b/mojo/add-floating-ip |
809 | index 57c9839..0a816bf 100755 |
810 | --- a/mojo/add-floating-ip |
811 | +++ b/mojo/add-floating-ip |
812 | @@ -8,6 +8,7 @@ |
813 | import json |
814 | import os |
815 | import subprocess |
816 | +import sys |
817 | |
818 | import novaclient.client |
819 | |
820 | @@ -94,14 +95,14 @@ def get_unit_floating_ip(unit_name): |
821 | try: |
822 | # Get the existing one |
823 | fip = nova_tenant.floating_ips.find(ip=myip) |
824 | - except Exception: |
825 | + except Exception as exc: |
826 | # If this happens you're going to need to either get that back in the list, |
827 | # or blow away the state file so it gets a new IP. |
828 | raise ( |
829 | RuntimeError( |
830 | "Desired IP {} not in floating ips list!".format(myip) |
831 | ) |
832 | - ) |
833 | + ) from exc |
834 | |
835 | if fip.instance_id: |
836 | # If it's already associated, ensure it's associated to us |
837 | @@ -147,8 +148,6 @@ add-floating-ip |
838 | |
839 | |
840 | def main(): |
841 | - import sys |
842 | - |
843 | if len(sys.argv) >= 2: |
844 | args = sys.argv[1:] |
845 | elif "targets" in os.environ: |
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)?