ddeb-retriever

Merge ~bdrung/ddeb-retriever/+git/main:pylint into ddeb-retriever:main

  1. Git
  2. lp:~bdrung/ddeb-retriever/+git/main
  3. pylint
  4. Merge into main
Proposed by Benjamin Drung
Status: Needs review
Proposed branch: ~bdrung/ddeb-retriever/+git/main:pylint
Merge into: ddeb-retriever:main
Diff against target: 204 lines (+32/-34)
3 files modified
archive_tools.py (+17/-17)
ddeb_retriever.py (+5/-6)
lpinfo.py (+10/-11)
Reviewer Review Type Date Requested Status
Steve Langasek Pending
Review via email: mp+437384@code.launchpad.net

Description of the change

This merge request contains some fixes/improvement for things that pylint or other linters complain about.

1. fix: Iterate over apt_pkg.TagFile

pylint complains about E1136: Value 'parser.section' is unsubscriptable
(unsubscriptable-object). `apt_pkg.TagFile` claims to be an iterator
over `apt_pkg.TagSection`.

So iterate over `apt_pkg.TagFile` for the sections instead of calling
`step()` and accessing the `section` property.

This change was tested to also work on Ubuntu 16.04 "xenial" with
Python 3.5.2 and python3-apt 1.1.0~beta1ubuntu0.16.04.12.

2. Use `functools.cache` for `_get_lp()` to avoid using a global variable
and make the code more readable.

3. refactor: Mark unused variables as such

4. fix: Use lazy % formatting in logging functions

5. Use `timestamp()` method of datetime instead of calculating it manually.

To post a comment you must log in.

Unmerged commits

6eb54c2... by Benjamin Drung

refactor: Use datetime.timestamp()

Use `timestamp()` method of datetime instead of calculating it manually.

Signed-off-by: Benjamin Drung <email address hidden>

f47a5a5... by Benjamin Drung

fix: Use lazy % formatting in logging functions

Signed-off-by: Benjamin Drung <email address hidden>

608b211... by Benjamin Drung

refactor: Mark unused variables as such

Name unused variables `_` to indicate to pylint and other linters that
these variables are not used.

Signed-off-by: Benjamin Drung <email address hidden>

8639da0... by Benjamin Drung

refactor: Use functools.cache for _get_lp()

Use `functools.cache` for `_get_lp()` to avoid using a global variable
and make the code more readable.

Signed-off-by: Benjamin Drung <email address hidden>

e207297... by Benjamin Drung

fix: Iterate over apt_pkg.TagFile

pylint complains about E1136: Value 'parser.section' is unsubscriptable
(unsubscriptable-object). `apt_pkg.TagFile` claims to be an iterator
over `apt_pkg.TagSection`.

So iterate over `apt_pkg.TagFile` for the sections instead of calling
`step()` and accessing the `section` property.

This change was tested to also work on Ubuntu 16.04 "xenial" with
Python 3.5.2 and python3-apt 1.1.0~beta1ubuntu0.16.04.12.

Signed-off-by: Benjamin Drung <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/archive_tools.py b/archive_tools.py
2index c8cf1aa..ecfb57f 100644
3--- a/archive_tools.py
4+++ b/archive_tools.py
5@@ -66,13 +66,13 @@ def debcontrol_map(url, map_key='Package'):
6
7 map = {}
8 parser = apt_pkg.TagFile(tagfile)
9- while parser.step():
10+ for section in parser:
11 pkg = {}
12- for key in parser.section.keys():
13+ for key in section.keys():
14 if key != map_key:
15- pkg[key] = parser.section.get(key)
16+ pkg[key] = section.get(key)
17 if pkg:
18- map[parser.section.get(map_key)] = pkg
19+ map[section.get(map_key)] = pkg
20 tagfile.close()
21 return map
22
23@@ -156,9 +156,9 @@ def debcontrol_dominate(infile, outfile, group_by='Package',
24 '''
25 max = {} # group_by -> (sort_key_value, text)
26 parser = apt_pkg.TagFile(infile)
27- while parser.step():
28- if cmp(max.get(parser.section[group_by], ('', ''))[0], parser.section[sort_key]) < 0:
29- max[parser.section[group_by]] = (parser.section[sort_key], str(parser.section))
30+ for section in parser:
31+ if cmp(max.get(section[group_by], ('', ''))[0], section[sort_key]) < 0:
32+ max[section[group_by]] = (section[sort_key], str(section))
33
34 ks = sorted(max)
35 first = True
36@@ -292,7 +292,7 @@ Tree "dists/%s" {
37 listfile = open(os.path.join('lists', '%s-%s' % (component, arch)), 'w')
38 for f in glob.glob(os.path.join(pooldir, '*/*/*%s%s' % (arch, ext))):
39 try:
40- (pkg, ver, suffix) = os.path.basename(f).split('_')
41+ (pkg, ver, _) = os.path.basename(f).split('_')
42 except ValueError:
43 logging.warning('Cannot split file name %s, unknown format; ignoring', f)
44 continue
45@@ -350,8 +350,8 @@ def _packages_files(path, files):
46
47 with open(path) as f:
48 parser = apt_pkg.TagFile(f)
49- while parser.step():
50- f = parser.section['Filename']
51+ for section in parser:
52+ f = section['Filename']
53 if f.startswith('./'):
54 f = f[2:]
55 files.add(f)
56@@ -363,9 +363,9 @@ def _sources_files(path, files):
57
58 with open(path) as f:
59 parser = apt_pkg.TagFile(f)
60- while parser.step():
61- d = parser.section.get('Directory')
62- for l in parser.section['Files'].splitlines():
63+ for section in parser:
64+ d = section.get('Directory')
65+ for l in section['Files'].splitlines():
66 fields = l.split()
67 assert len(fields) == 3
68 if d:
69@@ -390,7 +390,7 @@ def archive_cleanup(archive_root, keep_days=0):
70 distroot = '.'
71
72 indexed_files = set([])
73- for root, dirs, files in os.walk(distroot):
74+ for root, _, files in os.walk(distroot):
75 if root == '.':
76 root = ''
77 elif root.startswith('./'):
78@@ -424,7 +424,7 @@ def archive_cleanup(archive_root, keep_days=0):
79 poolroot = 'pool'
80 if not os.path.isdir(poolroot):
81 poolroot = '.'
82- for root, dirs, files in os.walk(poolroot):
83+ for root, _, files in os.walk(poolroot):
84 if root == '.':
85 root = ''
86 if root.startswith('./'):
87@@ -475,7 +475,7 @@ def archive_dominate(archive_root, keep_days=0):
88 if not os.path.isdir(distroot):
89 distroot = archive_root
90
91- for root, dirs, files in os.walk(distroot):
92+ for root, _, files in os.walk(distroot):
93 if 'Packages' in files:
94 path = os.path.join(root, 'Packages')
95 with open(path) as old, open(path + '.new', 'w') as new:
96@@ -751,7 +751,7 @@ def _find_files(dir, strip_prefix=None):
97 else:
98 plen = 0
99 result = set([])
100- for root, dirs, files in os.walk(dir):
101+ for root, _, files in os.walk(dir):
102 for f in files:
103 result.add(os.path.join(root, f)[plen:])
104 return result
105diff --git a/ddeb_retriever.py b/ddeb_retriever.py
106index 2f1ac4d..5bc38b0 100644
107--- a/ddeb_retriever.py
108+++ b/ddeb_retriever.py
109@@ -79,7 +79,7 @@ def install_from_librarian(pub, url, ddeb_archive_root):
110 '''
111 ddeb = urllib.parse.unquote(os.path.basename(url))
112 try:
113- (dbgsymname, version, arch) = ddeb.split('_')
114+ (dbgsymname, _, arch) = ddeb.split('_')
115 assert arch.endswith('.ddeb')
116 (arch, _) = arch.split('.')
117 assert dbgsymname.endswith('-dbgsym')
118@@ -163,7 +163,7 @@ def main(argv=None):
119 # order during long transactions.
120 real_threshold = lp_threshold - datetime.timedelta(hours=1)
121 logging.info(
122- 'Retrieving Launchpad publications since %s' % real_threshold)
123+ 'Retrieving Launchpad publications since %s', real_threshold)
124 else:
125 real_threshold = None
126 logging.info('Retrieving all Launchpad publications')
127@@ -217,8 +217,8 @@ def main(argv=None):
128 installed_pockets = set()
129 for i, pub in enumerate(debug_pubs):
130 logging.info(
131- 'Installing %d/%d: %s' %
132- (i + 1, len(debug_pubs), pub.display_name))
133+ 'Installing %d/%d: %s',
134+ i + 1, len(debug_pubs), pub.display_name)
135 already_present = False
136 pub_installed = False
137 if pub.binary_package_name in ddeb_map:
138@@ -239,8 +239,7 @@ def main(argv=None):
139 installed_pockets.add(get_suite_for_publication(pub))
140
141 if latest_date_created is not None:
142- epoch = datetime.datetime.fromtimestamp(0, tz=UTC)
143- new_threshold = (latest_date_created - epoch).total_seconds()
144+ new_threshold = latest_date_created.timestamp()
145 with open(lp_threshold_path, 'w') as lp_threshold_file:
146 lp_threshold_file.write("%d\n" % new_threshold)
147
148diff --git a/lpinfo.py b/lpinfo.py
149index c5410b8..841b75a 100644
150--- a/lpinfo.py
151+++ b/lpinfo.py
152@@ -1,18 +1,17 @@
153+import functools
154 import logging
155 import os
156
157 from launchpadlib.launchpad import Launchpad
158
159-lp = None
160-
161
162+@functools.cache
163 def _get_lp():
164- global lp
165- if lp is None:
166- lp = Launchpad.login_anonymously(
167- 'ddeb-retriever',
168- os.environ.get('LAUNCHPAD_INSTANCE', 'production'),
169- version='devel')
170+ return Launchpad.login_anonymously(
171+ 'ddeb-retriever',
172+ os.environ.get('LAUNCHPAD_INSTANCE', 'production'),
173+ version='devel'
174+ )
175
176
177 def get_series(distribution):
178@@ -20,7 +19,7 @@ def get_series(distribution):
179
180 Return name -> (components, architectures)
181 '''
182- _get_lp()
183+ lp = _get_lp()
184 info = {}
185 for s in lp.distributions[distribution].series:
186 # FIXME: disco is still marked as supported in LP, but EOL
187@@ -38,7 +37,7 @@ def get_series(distribution):
188 def get_buildds():
189 '''Return list of available buildds'''
190
191- _get_lp()
192+ lp = _get_lp()
193 buildds = []
194
195 for builder in lp.builders:
196@@ -55,7 +54,7 @@ def get_buildds():
197 def get_binary_publications(distribution, created_since_date=None):
198 '''Get recently-published binaries for the given distribution'''
199
200- _get_lp()
201+ lp = _get_lp()
202 archive = lp.distributions[distribution].main_archive
203 # It's important to omit the status filter here, even if we later decide
204 # that we only care about the Published status (although at the moment

Subscribers

People subscribed via source and target branches