Bug#739497: reproducible.json now exists
Hi Paul,
On Tue, 20 Jan 2015, Paul Wise wrote:
> + def get_build_reproducibility_content(self):
> + url = 'https://reproducible.debian.net/reproducible.json'
> + return get_resource_content(url)
> +
> + def get_build_reproducibility(self):
> + reproducibilities = self.get_build_reproducibility_content()
> + reproducibilities = json.loads(reproducibilities)
> + reproducibilities = dict([(item['package'], item['status']) for item in reproducibilities])
> + return reproducibilities
> +
> + def update_action_item(self, package, status):
> + action_item = package.get_action_item_for_type(self.action_item_type.type_name)
> + if action_item is None:
> + action_item = ActionItem(
> + package=package,
> + item_type=self.action_item_type,
> + severity=ActionItem.SEVERITY_NORMAL)
> +
> + if not (status and status in self.ITEM_DESCRIPTION and self.ITEM_DESCRIPTION[status]):
> + return
Don't you want to delete the action item here if the new status is None or not
known ? In fact you probably want to create it the new action_item only
after that test too! Or given that you delete obsolete action items later,
you might want to do this check first (or outside of the function).
> + url = "https://reproducible.debian.net/rb-pkg/{pkg}.html".format(pkg=package.name)
> + action_item.short_description = self.ITEM_DESCRIPTION[status].format(url=url)
> + action_item.save()
> +
> + def execute(self):
> + reproducibilities = self.get_build_reproducibility()
> + if reproducibilities is None:
> + return
get_build_reproducibility() can never return None with your current
implementation. To follow the pattern used in other tasks, it should
do so when the underlying URL hasn't changed. Instead of using
get_resource_content, you must use the HttpCache() directly to be able
to retrieve the result of cache.update().
See for example RetrieveLowThresholdNmuTask._retrieve_emails(). (That said
this whole pattern should really be factorized in a new helper function)
> + with transaction.atomic():
> + PackageExtractedInfo.objects.filter(key='reproducibility').delete()
> +
> + packages = []
> + extracted_info = []
> +
> + for name, status in reproducibilities.items():
> + try:
> + package = SourcePackageName.objects.get(name=name)
> + packages.append(package)
> + self.update_action_item(package, status)
> + except SourcePackageName.DoesNotExist:
> + continue
> +
> + try:
> + reproducibility_info = package.packageextractedinfo_set.get(key='reproducibility')
This will always raise the exception since you dropped them all at the
start of the transaction. So you can just keep the content of your except
clause AIUI.
> + reproducibility_info.value['reproducibility'] = status
> + except PackageExtractedInfo.DoesNotExist:
> + reproducibility_info = PackageExtractedInfo(
> + key='reproducibility',
> + package=package,
> + value={'reproducibility': status})
> + extracted_info.append(reproducibility_info)
> +
> + ActionItem.objects.delete_obsolete_items([self.action_item_type], packages)
> + PackageExtractedInfo.objects.bulk_create(extracted_info)
Except those little points, it looks mostly good.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
Reply to: