-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Am 2014-08-19 11:13, schrieb Raphael Hertzog: > Hello Simon, > > On Mon, 18 Aug 2014, Simon Kainz wrote: >> this adds support for data from duck.debian.net. > > Thanks for the patch! Here are a few things to fix: > > First you should respect PEP8 for the coding style, notably: - use > only spaces to indent, not tabs - spaces around assignation > operatior > > You can use "pep8" to check your code (distro-tracker is not fully > PEP8 compliant yet, but we try to pay attention for new code).
ok, i did and fixed the areas where it complains about code written by me
>
>>> From a0a3810608a306ab77eee214a9be0854e39e3367 Mon Sep 17
>>> 00:00:00 2001
>> From: Simon Kainz <si...@familiekainz.at> Date: Mon, 18 Aug 2014
>> 10:42:28 +0200 Subject: [PATCH 1/2] add new template for duck
>> extra info.
>
> There's no point in adding the template in a separate commit.
> Please squash both commits together.
ok, done, see the attachment.
>
>> --- /dev/null +++
>> b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
>>
>>
@@ -0,0 +1,8 @@
>> +{% with duck = item.extra_data.duck_link %} +{% with issues =
>> item.extra_data.issues_link %} +<a href="{{ duck }}">DUCK</a>
>> reports some <a href="{{ issues }}">issues</a> concerning
>> upstream URLs defined for this package. +{% endwith %} +{%
>> endwith %}
>
> The "with" tag doesn't bring anything as you use each variable only
> once. Please inline the duck and issues variables.
fixed.
>
>> From 11e3918f8f9b99b96e0d77c11ddf686dc36d20f1 Mon Sep 17 00:00:00
>> 2001 From: Simon Kainz <si...@familiekainz.at> Date: Mon, 18 Aug
>> 2014 10:43:14 +0200 Subject: [PATCH 2/2] add data from
>> duck.debian.net into
>>
>
>> +class UpdateDebianDuckTask(BaseTask): + """ + A task for
>> updating upstream url issue information on all packages. +
>> """ + + DUCK_LINK = 'http://duck.debian.net' + # URL of the
>> list of source packages with issues. + DUCK_SP_LIST_URL =
>> 'http://duck.debian.net/static/sourcepackages.txt' + +
>
> Single empty line here.
fixed.
>
>> + ACTION_ITEM_TYPE_NAME = 'debian-duck' +
>> ACTION_ITEM_TEMPLATE = 'debian/duck-action-item.html' +
>> ITEM_DESCRIPTION = 'The URL(s) for this package had some recent
>> persistent <a href="{issues_link}">issues</a>' + + def
>> __init__(self, force_update=False, *args, **kwargs): +
>> super(UpdateDebianDuckTask, self).__init__(*args, **kwargs) +
>> self.force_update = force_update + self.action_item_type =
>> ActionItemType.objects.create_or_update( +
>> type_name=self.ACTION_ITEM_TYPE_NAME, +
>> full_description_template = self.ACTION_ITEM_TEMPLATE)
>
> Please do not use "tabs" but real spaces with 4-space
done.
>
>
>> + def _get_duck_urls_content(self): + """ + Gets the list of
>> packages with URL issues from + duck.debian.net + + :returns: A
>> array if source package names. + """ + + ducklist =
>> get_resource_content(self.DUCK_SP_LIST_URL) + + packages = [] +
>> for package_name in ducklist.splitlines(): +
>> package_name = package_name.strip() +
>> packages.append(package_name) + return packages
>
> Eek, the mix of spaces and tabs gives a really bad output here
> when quoted...
also fixed.
>
> Also get_resource_content will return None when the content of the
> URL hasn't changed since last time, you should deal with that
> case.
>
>> + +
>
> A single empty line here.
>
>> + def update_action_item(self,package):
>
> Space after the comma.
fixed.
>
>> + action_item =
>> package.get_action_item_for_type(self.action_item_type) + if not
>> action_item: + action_item = ActionItem( + package =
>> package, + item_type = self.action_item_type, + ) + +
>> issues_link = self.DUCK_LINK + "/static/sp/" + package.name[0] +
>> "/" + package.name + ".html"
>
> Are you really using a directory split based on the first character
> only? I ask because the debian pool has a special case for "lib*"
> packages where it uses the four first letters and many services
> have been mimicking this choice.
ok, fixed, i now use the lib* approach.
>
>> + action_item.short_description=
>> self.ITEM_DESCRIPTION.format(issues_link = issues_link)
>
> one space around the first "=" (in the assignation) but no spaces
> around the second one (named parameter).
>
>> + action_item.save() + + def execute(self): + ducklings=
>> self._get_duck_urls_content()
>
> One space around the equal.
fixed.
>
>> + if ducklings is None: + return
>
> Your current version of _get_duck_urls_content() will never return
> None but this is something to fix (cf another comment above).
>
fixed (at least to my understanding).
>> + ActionItem.objects.delete_obsolete_items( +
>> item_types=[self.action_item_type], +
>> non_obsolete_packages=ducklings) + + packages =
>> SourcePackageName.objects.filter(name__in=ducklings) + + for
>> package in packages: + self.update_action_item(package) +
>> class UpdateWnppStatsTask(BaseTask):
>
> Please put two empty lines between two class definitions.
done.
>
> It would also be nicer if you could include unit tests for your
> code. It should relatively easy to adapt from other similar tasks
> in distro_tracker/vendor/debian/tests.py (UpdateDebciStatusTask for
> example).
>
I added 3 testcases for duck to distro_tracker/vendor/debian/tests.py
which behave as expected.
> Cheers,
>
Sorry for the noise in the first place. My python-foo is rather
non-existent, but will get better.
So please take another look at my attached patch.
Bye & thanks for your help.
Simon
- --
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQIcBAEBCgAGBQJT9KJgAAoJEBy08PeN7K/pfLIP/3Zg92bTFvBUIn84slPtcxAo
uiCamgSOQKAuOXPPOcwEPyvhCH2P3NvD35m0O58l9oXm6bdSN/u8Yi/3PC7fuQ1K
f223IuPdezCol6VRoUHrLfiSWRBk3I/Nt3m8QQkQMQzNAbYTXHFrsidRbGfFna9R
d3u5t/RM84Q+onI4O70RHFivRslmRS2AfEXgf1zflmajP6jfyeq8+ST9n+wAiUwz
6iuV6PSGOqsr8hFqiMjtVEJ7/T887cCol9pJBCmD9uaH30I2Dzi4UzjhFfoWzOsW
R27w1Spoy3Vb/IvJxJCcsGNxryIZ1lvX5x6iRODqrC8478FJDGiDBo0pJJ9Uw50I
L+zoJWCwB47rb2Xij2YvSUwWTIpVNnnmwuJVHNj1gMUV2zcOVF6yCnksPvVgNBZF
1SCRno//mq/oZzbihzct7fTtZPhWheYSqd/x36Q19qI60s5yk2IVOHjksn8ADwjn
Rls71N9E2cspaKLpWELu/JMI1uk9CJf3XqyGDMW0RmYcYwNsHF3ZyanBsmEVMUYQ
kznE0aQBM539H/Kl4kd0lwOm4c2VuAiOgeyeYct0uRchjI1AAGx0Mm0sxIFpfKm2
xQgAgAjvep5mgGXsxeW6Uy9mWM91TAUszwuIQY0I44fDhd4Z5Cif2g5LHygEn4Cq
PFTa9EpTdV22gZmrVZgD
=JVLm
-----END PGP SIGNATURE-----
>From 0ce427d42900d99c4a148296545bff625d99e3fc Mon Sep 17 00:00:00 2001
From: Simon Kainz <si...@familiekainz.at>
Date: Wed, 20 Aug 2014 15:14:56 +0200
Subject: [PATCH] add support for data from duck.debian.net
---
.../debian/templates/debian/duck-action-item.html | 4 ++
distro_tracker/vendor/debian/tests.py | 57 +++++++++++++++
distro_tracker/vendor/debian/tracker_tasks.py | 81 ++++++++++++++++++++++
3 files changed, 142 insertions(+)
create mode 100644 distro_tracker/vendor/debian/templates/debian/duck-action-item.html
diff --git a/distro_tracker/vendor/debian/templates/debian/duck-action-item.html b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
new file mode 100644
index 0000000..ecb4b56
--- /dev/null
+++ b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
@@ -0,0 +1,4 @@
+<a href="{{ item.extra_data.duck_link }}">DUCK</a> reports some <a href="{{ item.extra_data.issues_link }}">issues</a> concerning upstream URLs defined for this package.
+
+
+
diff --git a/distro_tracker/vendor/debian/tests.py b/distro_tracker/vendor/debian/tests.py
index 3bd5a5d..2e657e2 100644
--- a/distro_tracker/vendor/debian/tests.py
+++ b/distro_tracker/vendor/debian/tests.py
@@ -60,6 +60,7 @@ from distro_tracker.vendor.debian.tracker_tasks import RetrieveLowThresholdNmuTa
from distro_tracker.vendor.debian.tracker_tasks import DebianWatchFileScannerUpdate
from distro_tracker.vendor.debian.tracker_tasks import UpdateExcusesTask
from distro_tracker.vendor.debian.tracker_tasks import UpdateDebciStatusTask
+from distro_tracker.vendor.debian.tracker_tasks import UpdateDebianDuckTask
from distro_tracker.vendor.debian.models import DebianContributor
from distro_tracker.vendor.debian.models import UbuntuPackage
from distro_tracker.vendor.debian.tracker_tasks import UpdateLintianStatsTask
@@ -4500,6 +4501,62 @@ class DebianSsoLoginTests(TestCase):
@mock.patch('distro_tracker.core.utils.http.requests')
+class UpdateDebianDuckTaskTest(TestCase):
+ """
+ Tests for the :class:`distro_tracker.vendor.debian.tracker_tasks.UpdateDebianDuckTask` task.
+ """
+ def setUp(self):
+ self.dummy_package = SourcePackageName.objects.create(name='dummy-package')
+ self.other_package = SourcePackageName.objects.create(name='other-package')
+ self.duck_data = """
+ dummy-package
+ dummy-package2
+ """
+
+ def run_task(self):
+ """
+ Runs the Duck status update task.
+ """
+ task = UpdateDebianDuckTask()
+ task.execute()
+
+ def test_action_item_when_in_list(self, mock_requests):
+ """
+ Tests that an ActionItem is created for a package reported by duck.
+ """
+ set_mock_response(mock_requests, text=self.duck_data)
+
+ self.run_task()
+ self.assertEqual(1, self.dummy_package.action_items.count())
+
+ def test_no_action_item_when_not_in_list(self, mock_requests):
+ """
+ Tests that no ActionItem is created for a package not reported by duck.
+ """
+ set_mock_response(mock_requests, text=self.duck_data)
+
+ self.run_task()
+ self.assertEqual(0, self.other_package.action_items.count())
+
+ def test_action_item_is_dropped_when_duck_reports_nothing_again(self, mock_requests):
+ """
+ Tests that ActionItems are dropped when a package was previousy reported
+ but is now not reported anymore.
+ """
+ set_mock_response(mock_requests, text=self.duck_data)
+ self.run_task()
+ self.assertEqual(1, self.dummy_package.action_items.count())
+
+ duck_data = """
+ yet-another-package
+ """
+ set_mock_response(mock_requests, text=duck_data)
+
+ self.run_task()
+ self.assertEqual(0, self.dummy_package.action_items.count())
+
+
+@mock.patch('distro_tracker.core.utils.http.requests')
class UpdateDebciStatusTaskTest(TestCase):
"""
Tests for the :class:`distro_tracker.vendor.debian.tracker_tasks.UpdateDebciStatusTask` task.
diff --git a/distro_tracker/vendor/debian/tracker_tasks.py b/distro_tracker/vendor/debian/tracker_tasks.py
index 920ac62..e815359 100644
--- a/distro_tracker/vendor/debian/tracker_tasks.py
+++ b/distro_tracker/vendor/debian/tracker_tasks.py
@@ -1686,6 +1686,87 @@ class UpdateUbuntuStatsTask(BaseTask):
patch_diff=diff)
+class UpdateDebianDuckTask(BaseTask):
+ """
+ A task for updating upstream url issue information on all packages.
+ """
+
+ DUCK_LINK = 'http://duck.debian.net'
+ # URL of the list of source packages with issues.
+ DUCK_SP_LIST_URL = 'http://duck.debian.net/static/sourcepackages.txt'
+
+ ACTION_ITEM_TYPE_NAME = 'debian-duck'
+ ACTION_ITEM_TEMPLATE = 'debian/duck-action-item.html'
+ ITEM_DESCRIPTION = 'The URL(s) for this package had some \
+ recent persistent <a href="{issues_link}">issues</a>'
+
+ def __init__(self, force_update=False, *args, **kwargs):
+ super(UpdateDebianDuckTask, self).__init__(*args, **kwargs)
+ self.force_update = force_update
+ self.action_item_type = ActionItemType.objects.create_or_update(
+ type_name=self.ACTION_ITEM_TYPE_NAME,
+ full_description_template=self.ACTION_ITEM_TEMPLATE)
+
+ def set_parameters(self, parameters):
+ if 'force_update' in parameters:
+ self.force_update = parameters['force_update']
+
+ def prefix(self, pname):
+ if pname.startswith("lib") and len(pname) > 3:
+ return pname[0:4]
+ return pname[0:1]
+
+ def _get_duck_urls_content(self):
+ """
+ Gets the list of packages with URL issues from
+ duck.debian.net
+
+ :returns: A array if source package names.
+ """
+
+ ducklist = get_resource_content(self.DUCK_SP_LIST_URL)
+ if ducklist is None:
+ return None
+
+ packages = []
+ for package_name in ducklist.splitlines():
+ package_name = package_name.strip()
+ packages.append(package_name)
+ return packages
+
+ def update_action_item(self, package):
+ action_item = package.get_action_item_for_type(self.action_item_type)
+ if not action_item:
+ action_item = ActionItem(
+ package=package,
+ item_type=self.action_item_type,
+ )
+
+ issues_link = self.DUCK_LINK + "/static/sp/" \
+ + self.prefix(package.name) + "/" + package.name + ".html"
+ action_item.short_description = self.ITEM_DESCRIPTION.format(issues_link=issues_link)
+
+ action_item.extra_data = {
+ 'duck_link': self.DUCK_LINK,
+ 'issues_link': issues_link
+ }
+ action_item.save()
+
+ def execute(self):
+ ducklings = self._get_duck_urls_content()
+ if ducklings is None:
+ return
+
+ ActionItem.objects.delete_obsolete_items(
+ item_types=[self.action_item_type],
+ non_obsolete_packages=ducklings)
+
+ packages = SourcePackageName.objects.filter(name__in=ducklings)
+
+ for package in packages:
+ self.update_action_item(package)
+
+
class UpdateWnppStatsTask(BaseTask):
"""
The task updates the WNPP bugs for all packages.
--
2.0.1
0001-add-support-for-data-from-duck.debian.net.patch.sig
Description: PGP signature
