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'+. 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 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] - kernel_version = root_hal_device.getProperty('system.kernel.version') + @cachedproperty + def kernel_package_name(self): + """The kernel package name for the submission.""" + if ROOT_UDI in self.devices: + root_hal_device = self.devices[ROOT_UDI] + kernel_version = root_hal_device.getProperty( + 'system.kernel.version') + else: + kernel_version = self.parsed_data['summary'].get('kernel-release') if kernel_version is None: self._logWarning( 'Submission does not provide property system.kernel.version ' - 'for /org/freedesktop/Hal/devices/computer.', - WARNING_NO_HAL_KERNEL_VERSION) + 'for /org/freedesktop/Hal/devices/computer or a summary ' + 'sub-node .') return None kernel_package_name = 'linux-image-' + kernel_version packages = self.parsed_data['software']['packages'] @@ -1490,8 +1496,7 @@ 'Inconsistent kernel version data: According to HAL the ' 'kernel is %s, but the submission does not know about a ' 'kernel package %s' - % (kernel_version, kernel_package_name), - WARNING_NO_HAL_KERNEL_VERSION) + % (kernel_version, kernel_package_name)) return None return kernel_package_name @@ -2185,10 +2190,9 @@ # drivers, so there is currently no need to search for # for user space printer drivers, for example. if self.driver_name is not None: - kernel_package_name = self.parser.getKernelPackageName() db_driver_set = getUtility(IHWDriverSet) return db_driver_set.getOrCreate( - kernel_package_name, self.driver_name) + self.parser.kernel_package_name, self.driver_name) else: return None === modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py' --- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-22 09:07:26 +0000 +++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-22 11:02:27 +0000 @@ -360,8 +360,8 @@ self.assertErrorMessage( parser.submission_key, "No udev root device defined") - def testKernelPackageName(self): - """Test of SubmissionParser.getKernelPackageName. + def test_kernel_package_name_hal_data(self): + """Test of SubmissionParser.kernel_package_name. Regular case. """ @@ -388,17 +388,19 @@ }, } parser.buildDeviceList(parser.parsed_data) - kernel_package = parser.getKernelPackageName() - self.assertEqual(kernel_package, self.KERNEL_PACKAGE, - 'Unexpected result of SubmissionParser.getKernelPackageName. ' + kernel_package = parser.kernel_package_name + self.assertEqual( + self.KERNEL_PACKAGE, kernel_package, + 'Unexpected value of SubmissionParser.kernel_package_name. ' 'Expected linux-image-2.6.24-19-generic, got %r' % kernel_package) - self.assertEqual(len(self.handler.records), 0, + self.assertEqual( + 0, len(self.handler.records), 'One or more warning messages were logged by ' - 'getKernelPackageName, where zero was expected.') + 'SubmissionParser.kernel_package_name, where zero was expected.') - def testKernelPackageNameInconsistent(self): - """Test of SubmissionParser.getKernelPackageName. + def test_kernel_package_hal_data_name_inconsistent(self): + """Test of SubmissionParser.kernel_package_name. Test a name inconsistency. """ @@ -426,24 +428,24 @@ } parser.submission_key = 'Test of inconsistent kernel package name' parser.buildDeviceList(parser.parsed_data) - kernel_package = parser.getKernelPackageName() - self.assertEqual(kernel_package, None, - 'Unexpected result of SubmissionParser.getKernelPackageName. ' - 'Expected None, got %r' % kernel_package) - self.assertWarningMessage(parser.submission_key, + kernel_package = parser.kernel_package_name + self.assertIs(None, kernel_package) + self.assertWarningMessage( + parser.submission_key, 'Inconsistent kernel version data: According to HAL the ' 'kernel is 2.6.24-19-generic, but the submission does not ' 'know about a kernel package linux-image-2.6.24-19-generic') # The warning appears only once per submission, even if the - # SubmissionParser.getKernelPackageName is called more than once. + # property kernel_package_name is accessed more than once. num_warnings = len(self.handler.records) - parser.getKernelPackageName() - self.assertEqual(num_warnings, len(self.handler.records), + test = parser.kernel_package_name + self.assertEqual( + num_warnings, len(self.handler.records), 'Warning for missing HAL property system.kernel.version ' 'repeated.') - def testKernelPackageNameNoHALData(self): - """Test of SubmissionParser.getKernelPackageName. + def test_kernel_package_name_hal_data_no_kernel_version_in_hal_data(self): + """Test of SubmissionParser.kernel_package_name. Test without HAL property system.kernel.version. """ @@ -469,26 +471,28 @@ } parser.submission_key = 'Test: missing property system.kernel.version' parser.buildDeviceList(parser.parsed_data) - kernel_package = parser.getKernelPackageName() - self.assertEqual(kernel_package, None, - 'Unexpected result of SubmissionParser.getKernelPackageName. ' - 'Expected None, got %r' % kernel_package) - self.assertWarningMessage(parser.submission_key, + self.assertIs(None, parser.kernel_package_name) + self.assertWarningMessage( + parser.submission_key, 'Submission does not provide property system.kernel.version ' - 'for /org/freedesktop/Hal/devices/computer.') + 'for /org/freedesktop/Hal/devices/computer or a summary ' + 'sub-node .') # The warning appears only once per submission, even if the - # SubmissionParser.getKernelPackageName is called more than once. + # property kernel_package_name is accessed more than once. num_warnings = len(self.handler.records) - parser.getKernelPackageName() - self.assertEqual(num_warnings, len(self.handler.records), + test = parser.kernel_package_name + self.assertEqual( + num_warnings, len(self.handler.records), 'Warning for missing HAL property system.kernel.version ' 'repeated.') - def testKernelPackageNameNoPackageData(self): - """Test of SubmissionParser.getKernelPackageName. + def test_kernel_package_name_hal_data_no_package_data(self): + """Test of SubmissionParser.kernel_package_name. - Test without any package data. getKernelPackageName returns - the property system.kernel.version without any further checking. + Test without any package data. In this case, + SubmissionParser.kernel_package_name is the value of the property + system.kernel.version if the root HAL device. No further checks + are done. """ parser = SubmissionParser(self.log) devices = [ @@ -512,8 +516,156 @@ } parser.submission_key = 'Test: missing property system.kernel.version' parser.buildDeviceList(parser.parsed_data) - kernel_package = parser.getKernelPackageName() - self.assertEqual(kernel_package, self.KERNEL_PACKAGE, + kernel_package = parser.kernel_package_name + self.assertEqual( + self.KERNEL_PACKAGE, kernel_package, + 'Unexpected result of SubmissionParser.getKernelPackageName, ' + 'test without any package data. Expected None, got %r' + % kernel_package) + + def test_kernel_package_name_udev_data(self): + """Test of SubmissionParser.kernel_package_name for udev data. + + Variant for udev data, regular case. + """ + parser = SubmissionParser(self.log) + parser.parsed_data = { + 'hardware': { + 'udev': [ + {'P': '/devices/LNXSYSTM:00'} + ], + 'sysfs-attributes': {}, + 'dmi': {}, + }, + 'software': { + 'packages': { + self.KERNEL_PACKAGE: {}, + }, + }, + 'summary': { + 'kernel-release': self.KERNEL_VERSION, + }, + } + parser.buildDeviceList(parser.parsed_data) + kernel_package = parser.kernel_package_name + self.assertEqual( + self.KERNEL_PACKAGE, kernel_package, + 'Unexpected value of SubmissionParser.kernel_package_name. ' + 'Expected linux-image-2.6.24-19-generic, got %r' % kernel_package) + + self.assertEqual( + 0, len(self.handler.records), + 'One or more warning messages were logged by ' + 'SubmissionParser.kernel_package_name, where zero was expected.') + + def test_kernel_package_udev_data_name_inconsistent(self): + """Test of SubmissionParser.kernel_package_name. + + Variant for udev data, name inconsistency. + """ + parser = SubmissionParser(self.log) + parser.parsed_data = { + 'hardware': { + 'udev': [ + {'P': '/devices/LNXSYSTM:00'} + ], + 'sysfs-attributes': {}, + 'dmi': {}, + }, + 'software': { + 'packages': { + 'linux-image-from-obscure-external-source': {}, + }, + }, + 'summary': { + 'kernel-release': self.KERNEL_VERSION, + }, + } + parser.submission_key = 'Test of inconsistent kernel package name' + parser.buildDeviceList(parser.parsed_data) + kernel_package = parser.kernel_package_name + self.assertIs(None, kernel_package) + self.assertWarningMessage( + parser.submission_key, + 'Inconsistent kernel version data: According to HAL the ' + 'kernel is 2.6.24-19-generic, but the submission does not ' + 'know about a kernel package linux-image-2.6.24-19-generic') + # The warning appears only once per submission, even if the + # property kernel_package_name is accessed more than once. + num_warnings = len(self.handler.records) + test = parser.kernel_package_name + self.assertEqual( + num_warnings, len(self.handler.records), + 'Warning for missing HAL property system.kernel.version ' + 'repeated.') + + def test_kernel_package_name_udev_data_no_kernel_version_in_summary(self): + """Test of SubmissionParser.kernel_package_name. + + Test without the summary sub-node . + """ + parser = SubmissionParser(self.log) + parser.parsed_data = { + 'hardware': { + 'udev': [ + {'P': '/devices/LNXSYSTM:00'} + ], + 'sysfs-attributes': {}, + 'dmi': {}, + }, + 'software': { + 'packages': { + self.KERNEL_PACKAGE: {}, + }, + }, + 'summary': {}, + } + parser.submission_key = 'Test: missing property system.kernel.version' + parser.buildDeviceList(parser.parsed_data) + self.assertIs(None, parser.kernel_package_name) + self.assertWarningMessage( + parser.submission_key, + 'Submission does not provide property system.kernel.version ' + 'for /org/freedesktop/Hal/devices/computer or a summary ' + 'sub-node .') + # The warning appears only once per submission, even if the + # property kernel_package_name is accessed more than once. + num_warnings = len(self.handler.records) + test = parser.kernel_package_name + self.assertEqual( + num_warnings, len(self.handler.records), + 'Warning for missing HAL property system.kernel.version ' + 'repeated.') + + def test_kernel_package_name_udev_data_no_package_data(self): + """Test of SubmissionParser.kernel_package_name. + + Variant for udev data, test without any package data. In this case, + SubmissionParser.kernel_package_name is the value of the property + system.kernel.version if the root HAL device. No further checks + are done. + """ + parser = SubmissionParser(self.log) + parser.parsed_data = { + 'hardware': { + 'udev': [ + {'P': '/devices/LNXSYSTM:00'}, + ], + 'sysfs-attributes': {}, + 'dmi': {}, + }, + 'software': { + 'packages': {}, + }, + 'summary': { + 'kernel-release': self.KERNEL_VERSION, + }, + } + parser.submission_key = 'Test: missing property system.kernel.version' + parser.buildDeviceList(parser.parsed_data) + kernel_package = parser.kernel_package_name + self.assertEqual( + self.KERNEL_PACKAGE, kernel_package, 'Unexpected result of SubmissionParser.getKernelPackageName, ' 'test without any package data. Expected None, got %r' % kernel_package)