Launchpad itself

Merge lp:~adeuring/launchpad/bug-458029-kernel-package-name-for-udev-submissions into lp:launchpad

  1. bug-458029-kernel-package-name-for-udev-submissions
  2. Merge into devel
Proposed by Abel Deuring
Status: Merged
Approved by: Muharem Hrnjadovic
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~adeuring/launchpad/bug-458029-kernel-package-name-for-udev-submissions
Merge into: lp:launchpad
Diff against target: 429 lines
3 files modified
lib/canonical/launchpad/scripts/hwdbsubmissions.py (+22/-10)
lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py (+3/-0)
lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py (+194/-36)
To merge this branch: bzr merge lp:~adeuring/launchpad/bug-458029-kernel-package-name-for-udev-submissions
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) Approve
Review via email: mp+13781@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :
Download full text (16.9 KiB)

This branch changes the method SubmissionParser.getKernelPackageName() into a property and makes it "aware" of submissions containig udev data instaed of HAL data.

One detail of the data about a device we store in the HWDB is a driver used to control a given device and the deb package of this driver. For kernel drivers, the package name is 'linux-image'+<kernel-version>.

Most submissions have two sources for the kernel version and the package name:

- The submitted data may contain the set of installed packages
- Submissions with HAL data, the root HAL device has a property 'system.kernel.version'; submissions with udev data have an XML node <summary><kernel-release>

Two source for this data mean that sone inconcistencies are possible; the method/proerty has some checks for these inconsistencies.

A problem with early versions of the processing was that getKernelPackageName is called for nearly each device of a submission (typically more 50), which led to a log file cluttered with 50 or more identical warnings, if there was a name inconsistency.

In a later version of the script, I introduced the optional parameter warning_id, which allow to suppress the repeated logging of warning with the same ID. The tests of getKernelPackageName/kernel_package_name ensure that a warning appears only once, even if the method/property is used more than once.

We have the nice feature of cached properties, so I declared the property as cachable, allowing to omit the warning_id parameter from the _logWarning call, and having the additional benefit of being a bit faster.

This branch is based on the reviewed but not yet merged branch lp:~adeuring/launchpad/bug-457475-udev-device-id; a diff of the changes in this branch is below.

test: ./bin/test --test=test_hwdb_submission_processing

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/scripts/hwdbsubmissions.py
  lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py

== Pyflakes notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    22: redefinition of unused 'etree' from line 20

== Pylint notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    20: [F0401] Unable to import 'xml.etree.cElementTree' (No module named etree)

These complaints are not related to my changes.

=== modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
--- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-21 16:49:03 +0000
+++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-22 11:04:03 +0000
@@ -33,6 +33,7 @@

 from canonical.lazr.xml import RelaxNGValidator

+from canonical.cachedproperty import cachedproperty
 from canonical.config import config
 from canonical.librarian.interfaces import LibrarianServerError
 from canonical.launchpad.interfaces.hwdb import (
@@ -1470,15 +1471,20 @@
         del self.devices['/devices']
         return True

- def getKernelPackageName(self):
- """Return the kernel package name of the submission,"""
- root_hal_device = self.devices[ROOT_UDI]
- kern...

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Hello Abel!

This looks good. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
2--- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-21 10:06:31 +0000
3+++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-22 11:50:26 +0000
4@@ -33,6 +33,7 @@
5
6 from canonical.lazr.xml import RelaxNGValidator
7
8+from canonical.cachedproperty import cachedproperty
9 from canonical.config import config
10 from canonical.librarian.interfaces import LibrarianServerError
11 from canonical.launchpad.interfaces.hwdb import (
12@@ -503,6 +504,7 @@
13 devices = []
14 device = None
15 line_number = 0
16+ device_id = 0
17
18 for line_number, line in enumerate(udev_data):
19 if len(line) == 0:
20@@ -518,9 +520,11 @@
21
22 key, value = record
23 if device is None:
24+ device_id += 1
25 device = {
26 'E': {},
27 'S': [],
28+ 'id': device_id,
29 }
30 devices.append(device)
31 # Some attribute lines have a space character after the
32@@ -1467,15 +1471,20 @@
33 del self.devices['/devices']
34 return True
35
36- def getKernelPackageName(self):
37- """Return the kernel package name of the submission,"""
38- root_hal_device = self.devices[ROOT_UDI]
39- kernel_version = root_hal_device.getProperty('system.kernel.version')
40+ @cachedproperty
41+ def kernel_package_name(self):
42+ """The kernel package name for the submission."""
43+ if ROOT_UDI in self.devices:
44+ root_hal_device = self.devices[ROOT_UDI]
45+ kernel_version = root_hal_device.getProperty(
46+ 'system.kernel.version')
47+ else:
48+ kernel_version = self.parsed_data['summary'].get('kernel-release')
49 if kernel_version is None:
50 self._logWarning(
51 'Submission does not provide property system.kernel.version '
52- 'for /org/freedesktop/Hal/devices/computer.',
53- WARNING_NO_HAL_KERNEL_VERSION)
54+ 'for /org/freedesktop/Hal/devices/computer or a summary '
55+ 'sub-node <kernel-release>.')
56 return None
57 kernel_package_name = 'linux-image-' + kernel_version
58 packages = self.parsed_data['software']['packages']
59@@ -1487,8 +1496,7 @@
60 'Inconsistent kernel version data: According to HAL the '
61 'kernel is %s, but the submission does not know about a '
62 'kernel package %s'
63- % (kernel_version, kernel_package_name),
64- WARNING_NO_HAL_KERNEL_VERSION)
65+ % (kernel_version, kernel_package_name))
66 return None
67 return kernel_package_name
68
69@@ -2182,10 +2190,9 @@
70 # drivers, so there is currently no need to search for
71 # for user space printer drivers, for example.
72 if self.driver_name is not None:
73- kernel_package_name = self.parser.getKernelPackageName()
74 db_driver_set = getUtility(IHWDriverSet)
75 return db_driver_set.getOrCreate(
76- kernel_package_name, self.driver_name)
77+ self.parser.kernel_package_name, self.driver_name)
78 else:
79 return None
80
81@@ -2776,6 +2783,11 @@
82 return None
83 return controller
84
85+ @property
86+ def id(self):
87+ return self.udev['id']
88+
89+
90 class ProcessingLoop(object):
91 """An `ITunableLoop` for processing HWDB submissions."""
92
93
94=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py'
95--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py 2009-10-12 14:50:22 +0000
96+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py 2009-10-22 11:50:26 +0000
97@@ -635,6 +635,7 @@
98 'MODALIAS': 'acpi:LNXSYSTM:',
99 },
100 'S': [],
101+ 'id': 1,
102 },
103 {
104 'P': '/devices/pci0000:00/0000:00:1a.0',
105@@ -643,6 +644,7 @@
106 'DEVPATH': '/devices/pci0000:00/0000:00:1a.0',
107 },
108 'S': ['char/189:256'],
109+ 'id': 2,
110 },
111 ],
112 result,
113@@ -704,6 +706,7 @@
114 'E': {},
115 'S': [],
116 'W': '2',
117+ 'id': 1,
118 },
119 ],
120 result,
121
122=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
123--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-21 10:06:31 +0000
124+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-22 11:50:26 +0000
125@@ -360,8 +360,8 @@
126 self.assertErrorMessage(
127 parser.submission_key, "No udev root device defined")
128
129- def testKernelPackageName(self):
130- """Test of SubmissionParser.getKernelPackageName.
131+ def test_kernel_package_name_hal_data(self):
132+ """Test of SubmissionParser.kernel_package_name.
133
134 Regular case.
135 """
136@@ -388,17 +388,19 @@
137 },
138 }
139 parser.buildDeviceList(parser.parsed_data)
140- kernel_package = parser.getKernelPackageName()
141- self.assertEqual(kernel_package, self.KERNEL_PACKAGE,
142- 'Unexpected result of SubmissionParser.getKernelPackageName. '
143+ kernel_package = parser.kernel_package_name
144+ self.assertEqual(
145+ self.KERNEL_PACKAGE, kernel_package,
146+ 'Unexpected value of SubmissionParser.kernel_package_name. '
147 'Expected linux-image-2.6.24-19-generic, got %r' % kernel_package)
148
149- self.assertEqual(len(self.handler.records), 0,
150+ self.assertEqual(
151+ 0, len(self.handler.records),
152 'One or more warning messages were logged by '
153- 'getKernelPackageName, where zero was expected.')
154+ 'SubmissionParser.kernel_package_name, where zero was expected.')
155
156- def testKernelPackageNameInconsistent(self):
157- """Test of SubmissionParser.getKernelPackageName.
158+ def test_kernel_package_hal_data_name_inconsistent(self):
159+ """Test of SubmissionParser.kernel_package_name.
160
161 Test a name inconsistency.
162 """
163@@ -426,24 +428,24 @@
164 }
165 parser.submission_key = 'Test of inconsistent kernel package name'
166 parser.buildDeviceList(parser.parsed_data)
167- kernel_package = parser.getKernelPackageName()
168- self.assertEqual(kernel_package, None,
169- 'Unexpected result of SubmissionParser.getKernelPackageName. '
170- 'Expected None, got %r' % kernel_package)
171- self.assertWarningMessage(parser.submission_key,
172+ kernel_package = parser.kernel_package_name
173+ self.assertIs(None, kernel_package)
174+ self.assertWarningMessage(
175+ parser.submission_key,
176 'Inconsistent kernel version data: According to HAL the '
177 'kernel is 2.6.24-19-generic, but the submission does not '
178 'know about a kernel package linux-image-2.6.24-19-generic')
179 # The warning appears only once per submission, even if the
180- # SubmissionParser.getKernelPackageName is called more than once.
181+ # property kernel_package_name is accessed more than once.
182 num_warnings = len(self.handler.records)
183- parser.getKernelPackageName()
184- self.assertEqual(num_warnings, len(self.handler.records),
185+ test = parser.kernel_package_name
186+ self.assertEqual(
187+ num_warnings, len(self.handler.records),
188 'Warning for missing HAL property system.kernel.version '
189 'repeated.')
190
191- def testKernelPackageNameNoHALData(self):
192- """Test of SubmissionParser.getKernelPackageName.
193+ def test_kernel_package_name_hal_data_no_kernel_version_in_hal_data(self):
194+ """Test of SubmissionParser.kernel_package_name.
195
196 Test without HAL property system.kernel.version.
197 """
198@@ -469,26 +471,28 @@
199 }
200 parser.submission_key = 'Test: missing property system.kernel.version'
201 parser.buildDeviceList(parser.parsed_data)
202- kernel_package = parser.getKernelPackageName()
203- self.assertEqual(kernel_package, None,
204- 'Unexpected result of SubmissionParser.getKernelPackageName. '
205- 'Expected None, got %r' % kernel_package)
206- self.assertWarningMessage(parser.submission_key,
207+ self.assertIs(None, parser.kernel_package_name)
208+ self.assertWarningMessage(
209+ parser.submission_key,
210 'Submission does not provide property system.kernel.version '
211- 'for /org/freedesktop/Hal/devices/computer.')
212+ 'for /org/freedesktop/Hal/devices/computer or a summary '
213+ 'sub-node <kernel-release>.')
214 # The warning appears only once per submission, even if the
215- # SubmissionParser.getKernelPackageName is called more than once.
216+ # property kernel_package_name is accessed more than once.
217 num_warnings = len(self.handler.records)
218- parser.getKernelPackageName()
219- self.assertEqual(num_warnings, len(self.handler.records),
220+ test = parser.kernel_package_name
221+ self.assertEqual(
222+ num_warnings, len(self.handler.records),
223 'Warning for missing HAL property system.kernel.version '
224 'repeated.')
225
226- def testKernelPackageNameNoPackageData(self):
227- """Test of SubmissionParser.getKernelPackageName.
228+ def test_kernel_package_name_hal_data_no_package_data(self):
229+ """Test of SubmissionParser.kernel_package_name.
230
231- Test without any package data. getKernelPackageName returns
232- the property system.kernel.version without any further checking.
233+ Test without any package data. In this case,
234+ SubmissionParser.kernel_package_name is the value of the property
235+ system.kernel.version if the root HAL device. No further checks
236+ are done.
237 """
238 parser = SubmissionParser(self.log)
239 devices = [
240@@ -512,8 +516,156 @@
241 }
242 parser.submission_key = 'Test: missing property system.kernel.version'
243 parser.buildDeviceList(parser.parsed_data)
244- kernel_package = parser.getKernelPackageName()
245- self.assertEqual(kernel_package, self.KERNEL_PACKAGE,
246+ kernel_package = parser.kernel_package_name
247+ self.assertEqual(
248+ self.KERNEL_PACKAGE, kernel_package,
249+ 'Unexpected result of SubmissionParser.getKernelPackageName, '
250+ 'test without any package data. Expected None, got %r'
251+ % kernel_package)
252+
253+ def test_kernel_package_name_udev_data(self):
254+ """Test of SubmissionParser.kernel_package_name for udev data.
255+
256+ Variant for udev data, regular case.
257+ """
258+ parser = SubmissionParser(self.log)
259+ parser.parsed_data = {
260+ 'hardware': {
261+ 'udev': [
262+ {'P': '/devices/LNXSYSTM:00'}
263+ ],
264+ 'sysfs-attributes': {},
265+ 'dmi': {},
266+ },
267+ 'software': {
268+ 'packages': {
269+ self.KERNEL_PACKAGE: {},
270+ },
271+ },
272+ 'summary': {
273+ 'kernel-release': self.KERNEL_VERSION,
274+ },
275+ }
276+ parser.buildDeviceList(parser.parsed_data)
277+ kernel_package = parser.kernel_package_name
278+ self.assertEqual(
279+ self.KERNEL_PACKAGE, kernel_package,
280+ 'Unexpected value of SubmissionParser.kernel_package_name. '
281+ 'Expected linux-image-2.6.24-19-generic, got %r' % kernel_package)
282+
283+ self.assertEqual(
284+ 0, len(self.handler.records),
285+ 'One or more warning messages were logged by '
286+ 'SubmissionParser.kernel_package_name, where zero was expected.')
287+
288+ def test_kernel_package_udev_data_name_inconsistent(self):
289+ """Test of SubmissionParser.kernel_package_name.
290+
291+ Variant for udev data, name inconsistency.
292+ """
293+ parser = SubmissionParser(self.log)
294+ parser.parsed_data = {
295+ 'hardware': {
296+ 'udev': [
297+ {'P': '/devices/LNXSYSTM:00'}
298+ ],
299+ 'sysfs-attributes': {},
300+ 'dmi': {},
301+ },
302+ 'software': {
303+ 'packages': {
304+ 'linux-image-from-obscure-external-source': {},
305+ },
306+ },
307+ 'summary': {
308+ 'kernel-release': self.KERNEL_VERSION,
309+ },
310+ }
311+ parser.submission_key = 'Test of inconsistent kernel package name'
312+ parser.buildDeviceList(parser.parsed_data)
313+ kernel_package = parser.kernel_package_name
314+ self.assertIs(None, kernel_package)
315+ self.assertWarningMessage(
316+ parser.submission_key,
317+ 'Inconsistent kernel version data: According to HAL the '
318+ 'kernel is 2.6.24-19-generic, but the submission does not '
319+ 'know about a kernel package linux-image-2.6.24-19-generic')
320+ # The warning appears only once per submission, even if the
321+ # property kernel_package_name is accessed more than once.
322+ num_warnings = len(self.handler.records)
323+ test = parser.kernel_package_name
324+ self.assertEqual(
325+ num_warnings, len(self.handler.records),
326+ 'Warning for missing HAL property system.kernel.version '
327+ 'repeated.')
328+
329+ def test_kernel_package_name_udev_data_no_kernel_version_in_summary(self):
330+ """Test of SubmissionParser.kernel_package_name.
331+
332+ Test without the summary sub-node <kernel-release>.
333+ """
334+ parser = SubmissionParser(self.log)
335+ parser.parsed_data = {
336+ 'hardware': {
337+ 'udev': [
338+ {'P': '/devices/LNXSYSTM:00'}
339+ ],
340+ 'sysfs-attributes': {},
341+ 'dmi': {},
342+ },
343+ 'software': {
344+ 'packages': {
345+ self.KERNEL_PACKAGE: {},
346+ },
347+ },
348+ 'summary': {},
349+ }
350+ parser.submission_key = 'Test: missing property system.kernel.version'
351+ parser.buildDeviceList(parser.parsed_data)
352+ self.assertIs(None, parser.kernel_package_name)
353+ self.assertWarningMessage(
354+ parser.submission_key,
355+ 'Submission does not provide property system.kernel.version '
356+ 'for /org/freedesktop/Hal/devices/computer or a summary '
357+ 'sub-node <kernel-release>.')
358+ # The warning appears only once per submission, even if the
359+ # property kernel_package_name is accessed more than once.
360+ num_warnings = len(self.handler.records)
361+ test = parser.kernel_package_name
362+ self.assertEqual(
363+ num_warnings, len(self.handler.records),
364+ 'Warning for missing HAL property system.kernel.version '
365+ 'repeated.')
366+
367+ def test_kernel_package_name_udev_data_no_package_data(self):
368+ """Test of SubmissionParser.kernel_package_name.
369+
370+ Variant for udev data, test without any package data. In this case,
371+ SubmissionParser.kernel_package_name is the value of the property
372+ system.kernel.version if the root HAL device. No further checks
373+ are done.
374+ """
375+ parser = SubmissionParser(self.log)
376+ parser.parsed_data = {
377+ 'hardware': {
378+ 'udev': [
379+ {'P': '/devices/LNXSYSTM:00'},
380+ ],
381+ 'sysfs-attributes': {},
382+ 'dmi': {},
383+ },
384+ 'software': {
385+ 'packages': {},
386+ },
387+ 'summary': {
388+ 'kernel-release': self.KERNEL_VERSION,
389+ },
390+ }
391+ parser.submission_key = 'Test: missing property system.kernel.version'
392+ parser.buildDeviceList(parser.parsed_data)
393+ kernel_package = parser.kernel_package_name
394+ self.assertEqual(
395+ self.KERNEL_PACKAGE, kernel_package,
396 'Unexpected result of SubmissionParser.getKernelPackageName, '
397 'test without any package data. Expected None, got %r'
398 % kernel_package)
399@@ -3036,7 +3188,8 @@
400 'DEVPATH': '/devices/LNXSYSTM:00',
401 'MODALIAS': 'acpi:LNXSYSTM:',
402 'SUBSYSTEM': 'acpi',
403- }
404+ },
405+ 'id': 1,
406 }
407
408 self.root_device_dmi_data = {
409@@ -3520,7 +3673,7 @@
410 },
411 }
412
413- def test_device_id(self):
414+ def test_device_device_id(self):
415 """Test of UdevDevice.device_id."""
416 device = UdevDevice(None, self.pci_sata_controller)
417 self.assertEqual(
418@@ -4185,6 +4338,11 @@
419 'provide bus, vendor ID, product ID or product name: None None '
420 'None None /devices/pci0000:00/0000:00:1d.7/usb1/1-1/1-1:1.0')
421
422+ def test_device_id(self):
423+ """Each UdevDevice has a property 'id'."""
424+ device = UdevDevice(None, self.root_device)
425+ self.assertEqual(1, device.id)
426+
427
428 class TestHWDBSubmissionTablePopulation(TestCaseHWDB):
429 """Tests of the HWDB popoluation with submitted data."""