From 74d2a5df5ca7ee4b6497a6e7609491d72cdbb309 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 17:18:13 -0500 Subject: Refactor more package signoff stuff This sets up some shared utility code for use in a later package signoff email report command. Signed-off-by: Dan McGee --- packages/utils.py | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 125 insertions(+), 9 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index c8c1f8a6..42cfbe0f 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -1,12 +1,11 @@ -from collections import defaultdict from operator import itemgetter from django.db import connection from django.db.models import Count, Max -from main.models import Package -from main.utils import cache_function -from .models import PackageGroup, PackageRelation, Signoff +from main.models import Package, Repo +from main.utils import cache_function, groupby_preserve_order, PackageStandin +from .models import PackageGroup, PackageRelation, SignoffSpecification, Signoff @cache_function(300) def get_group_info(include_arches=None): @@ -148,8 +147,90 @@ def get_wrong_permissions(): id__in=to_fetch) return relations -def get_current_signoffs(): - '''Returns a mapping of pkgbase -> signoff objects.''' + +DEFAULT_SIGNOFF_SPEC = SignoffSpecification() + +def approved_by_signoffs(signoffs, spec=DEFAULT_SIGNOFF_SPEC): + if signoffs: + good_signoffs = sum(1 for s in signoffs if not s.revoked) + return good_signoffs >= spec.required + return False + +class PackageSignoffGroup(object): + '''Encompasses all packages in testing with the same pkgbase.''' + def __init__(self, packages, user=None): + if len(packages) == 0: + raise Exception + self.packages = packages + self.user = user + self.target_repo = None + self.signoffs = set() + self.specification = DEFAULT_SIGNOFF_SPEC + + first = packages[0] + self.pkgbase = first.pkgbase + self.arch = first.arch + self.repo = first.repo + self.version = '' + self.last_update = first.last_update + self.packager = first.packager + + version = first.full_version + if all(version == pkg.full_version for pkg in packages): + self.version = version + + @property + def package(self): + '''Try and return a relevant single package object representing this + group. Start by seeing if there is only one package, then look for the + matching package by name, finally falling back to a standin package + object.''' + if len(self.packages) == 1: + return self.packages[0] + + same_pkgs = [p for p in self.packages if p.pkgname == p.pkgbase] + if same_pkgs: + return same_pkgs[0] + + return PackageStandin(self.packages[0]) + + def find_signoffs(self, all_signoffs): + '''Look through a list of Signoff objects for ones matching this + particular group and store them on the object.''' + for s in all_signoffs: + if s.pkgbase != self.pkgbase: + continue + if self.version and not s.full_version == self.version: + continue + if s.arch_id == self.arch.id and s.repo_id == self.repo.id: + self.signoffs.add(s) + + def approved(self): + return approved_by_signoffs(self.signoffs, self.specification) + + @property + def completed(self): + return sum(1 for s in self.signoffs if not s.revoked) + + @property + def required(self): + return self.specification.required + + def user_signed_off(self, user=None): + '''Did a given user signoff on this package? user can be passed as an + argument, or attached to the group object itself so this can be called + from a template.''' + if user is None: + user = self.user + return user in (s.user for s in self.signoffs if not s.revoked) + + def __unicode__(self): + return u'%s-%s (%s): %d' % ( + self.pkgbase, self.version, self.arch, len(self.signoffs)) + +def get_current_signoffs(repos): + '''Returns a mapping of pkgbase -> signoff objects for the given repos.''' + cursor = connection.cursor() sql = """ SELECT DISTINCT s.id FROM packages_signoff s @@ -162,14 +243,49 @@ def get_current_signoffs(): AND s.repo_id = p.repo_id ) JOIN repos r ON p.repo_id = r.id - WHERE r.testing = %s + WHERE r.id IN ( """ - cursor = connection.cursor() - cursor.execute(sql, [True]) + sql += ", ".join("%s" for r in repos) + sql += ")" + cursor.execute(sql, [r.id for r in repos]) + results = cursor.fetchall() # fetch all of the returned signoffs by ID to_fetch = [row[0] for row in results] signoffs = Signoff.objects.select_related('user').in_bulk(to_fetch) return signoffs.values() +def get_target_repo_map(pkgbases): + package_repos = Package.objects.order_by().values_list( + 'pkgbase', 'repo__name').filter( + repo__testing=False, repo__staging=False, + pkgbase__in=pkgbases).distinct() + return dict(package_repos) + +def get_signoff_groups(repos=None): + if repos is None: + repos = Repo.objects.filter(testing=True) + + test_pkgs = Package.objects.normal().filter(repo__in=repos) + packages = test_pkgs.order_by('pkgname') + + # Collect all pkgbase values in testing repos + q_pkgbase = test_pkgs.values('pkgbase') + pkgtorepo = get_target_repo_map(q_pkgbase) + + # Collect all existing signoffs for these packages + signoffs = get_current_signoffs(repos) + + same_pkgbase_key = lambda x: (x.repo.name, x.arch.name, x.pkgbase) + grouped = groupby_preserve_order(packages, same_pkgbase_key) + signoff_groups = [] + for group in grouped: + signoff_group = PackageSignoffGroup(group) + signoff_group.target_repo = pkgtorepo.get(signoff_group.pkgbase, + "Unknown") + signoff_group.find_signoffs(signoffs) + signoff_groups.append(signoff_group) + + return signoff_groups + # vim: set ts=4 sw=4 et: -- cgit v1.2.3-54-g00ecf From 5f2c3bf98baabf919681525e600639643aa2c119 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 20:39:59 -0500 Subject: Signoffs changes and improvements * Better signoff report with more detail * Show signoff specification in signoffs view * Honor disabled/bad flags and display in approval column * Various other small bugfixes and tweaks Signed-off-by: Dan McGee --- media/archweb.css | 8 +++++-- media/archweb.js | 30 ++++++++++++++++---------- packages/management/commands/signoff_report.py | 13 ++++++++++- packages/models.py | 28 +++++++++++++++++------- packages/utils.py | 16 ++++++++------ packages/views.py | 15 +++++++++---- templates/packages/signoff_cell.html | 2 ++ templates/packages/signoff_report.txt | 13 +++++++++++ templates/packages/signoffs.html | 19 ++++++++++++++-- 9 files changed, 109 insertions(+), 35 deletions(-) (limited to 'packages/utils.py') diff --git a/media/archweb.css b/media/archweb.css index f817e18d..303173f2 100644 --- a/media/archweb.css +++ b/media/archweb.css @@ -914,8 +914,12 @@ ul.admin-actions { color: red; } -#dev-signoffs .signed-username { - color: #888; +#dev-signoffs .signoff-bad { + color: darkorange; +} + +#dev-signoffs .signoff-disabled { + color: gray; } /* iso testing feedback form */ diff --git a/media/archweb.js b/media/archweb.js index 43812b33..a9f4e0c9 100644 --- a/media/archweb.js +++ b/media/archweb.js @@ -215,28 +215,33 @@ function signoff_package() { $.getJSON(link.href, function(data) { link = $(link); var signoff = null; + var cell = link.closest('td'); if (data.created) { signoff = $('
  • ').addClass('signed-username').text(data.user); - link.closest('td').children('ul').append(signoff); + var list = cell.children('ul'); + if (list.size() == 0) { + list = $('
      ').prependTo(cell); + } + list.append(signoff); } else if(data.user) { signoff = link.closest('td').find('li').filter(function(index) { return $(this).text() == data.user; }); } - console.log(signoff, data.revoked, data.user); if (signoff && data.revoked) { signoff.text(signoff.text() + ' (revoked)'); } /* update the approved column to reflect reality */ - var approved; - if (data.approved) { - approved = link.closest('tr').children('.signoff-no'); - approved.text('Yes').addClass( - 'signoff-yes').removeClass('signoff-no'); + var approved = link.closest('tr').children('.approval'); + approved.attr('class', ''); + if (data.known_bad) { + approved.text('Bad').addClass('signoff-bad'); + } else if (!data.enabled) { + approved.text('Disabled').addClass('signoff-disabled'); + } else if (data.approved) { + approved.text('Yes').addClass('signoff-yes'); } else { - approved = link.closest('tr').children('.signoff-yes'); - approved.text('No').addClass( - 'signoff-no').removeClass('signoff-yes'); + approved.text('No').addClass('signoff-no'); } link.removeAttr('title'); /* Form our new link. The current will be something like @@ -245,6 +250,10 @@ function signoff_package() { if (data.revoked) { link.text('Signoff'); link.attr('href', base_href + '/signoff/'); + /* should we be hiding the link? */ + if (data.known_bad || !data.enabled) { + link.remove(); + } } else { link.text('Revoke Signoff'); link.attr('href', base_href + '/signoff/revoke/'); @@ -260,7 +269,6 @@ function filter_signoffs() { var all_rows = rows; $('#signoffs_filter .arch_filter').each(function() { if (!$(this).is(':checked')) { - console.log($(this).val()); rows = rows.not('.' + $(this).val()); } }); diff --git a/packages/management/commands/signoff_report.py b/packages/management/commands/signoff_report.py index 02f3d985..3431dada 100644 --- a/packages/management/commands/signoff_report.py +++ b/packages/management/commands/signoff_report.py @@ -56,6 +56,8 @@ def generate_report(email, repo_name): # Collect all existing signoffs for these packages signoff_groups = sorted(get_signoff_groups([repo]), key=attrgetter('target_repo', 'arch', 'pkgbase')) + disabled = [] + bad = [] complete = [] incomplete = [] new = [] @@ -68,10 +70,16 @@ def generate_report(email, repo_name): old_cutoff = now - timedelta(days=old_days) for group in signoff_groups: - if group.approved(): + spec = group.specification + if spec.known_bad: + bad.append(group) + elif not spec.enabled: + disabled.append(group) + elif group.approved(): complete.append(group) else: incomplete.append(group) + if group.package.last_update > new_cutoff: new.append(group) if group.package.last_update < old_cutoff: @@ -96,6 +104,9 @@ def generate_report(email, repo_name): c = Context({ 'repo': repo, 'signoffs_url': signoffs_url, + 'disabled': disabled, + 'bad': bad, + 'all': signoff_groups, 'incomplete': incomplete, 'complete': complete, 'new': new, diff --git a/packages/models.py b/packages/models.py index a2b53a06..b70c21bf 100644 --- a/packages/models.py +++ b/packages/models.py @@ -1,3 +1,5 @@ +from collections import namedtuple + from django.db import models from django.db.models.signals import pre_save, post_save from django.contrib.auth.models import User @@ -42,22 +44,26 @@ class Meta: class SignoffSpecificationManager(models.Manager): def get_from_package(self, pkg): '''Utility method to pull all relevant name-version fields from a - package and get a matching specification.''' + package and get a matching signoff specification.''' return self.get( pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel, epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo) - def get_or_create_from_package(self, pkg): - '''Utility method to pull all relevant name-version fields from a - package and get or create a matching specification.''' - return self.get_or_create( - pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel, - epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo) + def get_or_default_from_package(self, pkg): + '''utility method to pull all relevant name-version fields from a + package and get a matching signoff specification, or return the default + base case.''' + try: + return self.get( + pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel, + epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo) + except SignoffSpecification.DoesNotExist: + return DEFAULT_SIGNOFF_SPEC class SignoffSpecification(models.Model): ''' A specification for the signoff policy for this particular revision of a - pakcage. The default is requiring two signoffs for a given package. These + package. The default is requiring two signoffs for a given package. These are created only if necessary; e.g., if one wanted to override the required=2 attribute, otherwise a sane default object is used. ''' @@ -89,6 +95,12 @@ def __unicode__(self): return u'%s-%s' % (self.pkgbase, self.full_version) +# fake default signoff spec when we don't have a persisted one in the database +FakeSignoffSpecification = namedtuple('FakeSignoffSpecification', + ('required', 'enabled', 'known_bad', 'comments')) +DEFAULT_SIGNOFF_SPEC = FakeSignoffSpecification(2, True, False, u'') + + class SignoffManager(models.Manager): def get_from_package(self, pkg, user, revoked=False): '''Utility method to pull all relevant name-version fields from a diff --git a/packages/utils.py b/packages/utils.py index 42cfbe0f..60b95e21 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -5,7 +5,8 @@ from main.models import Package, Repo from main.utils import cache_function, groupby_preserve_order, PackageStandin -from .models import PackageGroup, PackageRelation, SignoffSpecification, Signoff +from .models import (PackageGroup, PackageRelation, + SignoffSpecification, Signoff, DEFAULT_SIGNOFF_SPEC) @cache_function(300) def get_group_info(include_arches=None): @@ -148,9 +149,7 @@ def get_wrong_permissions(): return relations -DEFAULT_SIGNOFF_SPEC = SignoffSpecification() - -def approved_by_signoffs(signoffs, spec=DEFAULT_SIGNOFF_SPEC): +def approved_by_signoffs(signoffs, spec): if signoffs: good_signoffs = sum(1 for s in signoffs if not s.revoked) return good_signoffs >= spec.required @@ -158,14 +157,13 @@ def approved_by_signoffs(signoffs, spec=DEFAULT_SIGNOFF_SPEC): class PackageSignoffGroup(object): '''Encompasses all packages in testing with the same pkgbase.''' - def __init__(self, packages, user=None): + def __init__(self, packages): if len(packages) == 0: raise Exception self.packages = packages - self.user = user + self.user = None self.target_repo = None self.signoffs = set() - self.specification = DEFAULT_SIGNOFF_SPEC first = packages[0] self.pkgbase = first.pkgbase @@ -175,6 +173,10 @@ def __init__(self, packages, user=None): self.last_update = first.last_update self.packager = first.packager + self.specification = \ + SignoffSpecification.objects.get_or_default_from_package(first) + self.default_spec = self.specification is DEFAULT_SIGNOFF_SPEC + version = first.full_version if all(version == pkg.full_version for pkg in packages): self.version = version diff --git a/packages/views.py b/packages/views.py index 66bcd3fc..307691e2 100644 --- a/packages/views.py +++ b/packages/views.py @@ -7,8 +7,9 @@ from django.core.mail import send_mail from django.core.serializers.json import DjangoJSONEncoder from django.db.models import Q -from django.http import HttpResponse, Http404 -from django.shortcuts import get_object_or_404, get_list_or_404, redirect +from django.http import HttpResponse, Http404, HttpResponseForbidden +from django.shortcuts import (get_object_or_404, get_list_or_404, + redirect, render) from django.template import loader, Context from django.utils import simplejson from django.views.decorators.cache import never_cache @@ -404,12 +405,16 @@ def signoff_package(request, name, repo, arch, revoke=False): package, request.user) all_signoffs = Signoff.objects.for_package(package) + spec = SignoffSpecification.objects.get_or_default_from_package(package) if request.is_ajax(): data = { 'created': created, 'revoked': bool(signoff.revoked), - 'approved': approved_by_signoffs(all_signoffs), + 'approved': approved_by_signoffs(all_signoffs, spec), + 'required': spec.required, + 'enabled': spec.enabled, + 'known_bad': spec.known_bad, 'user': str(request.user), } return HttpResponse(simplejson.dumps(data, ensure_ascii=False), @@ -429,7 +434,9 @@ def signoff_options(request, name, repo, arch): arch__name=arch, repo__name__iexact=repo, repo__testing=True) package = packages[0] - # TODO ensure submitter is maintainer and/or packager + if request.user != package.packager and \ + request.user not in package.maintainers: + return render(request, '403.html', status=403) try: spec = SignoffSpecification.objects.get_from_package(package) diff --git a/templates/packages/signoff_cell.html b/templates/packages/signoff_cell.html index 0a630119..6c705b4e 100644 --- a/templates/packages/signoff_cell.html +++ b/templates/packages/signoff_cell.html @@ -11,10 +11,12 @@ Revoke Signoff {% else %} +{% if not group.specification.known_bad and group.specification.enabled %} {% endif %} +{% endif %} {% if group.packager == user %}
      Packager Options diff --git a/templates/packages/signoff_report.txt b/templates/packages/signoff_report.txt index 84e3fc6b..81020c8f 100644 --- a/templates/packages/signoff_report.txt +++ b/templates/packages/signoff_report.txt @@ -1,6 +1,19 @@ === {% autoescape off %}Signoff report for [{{ repo|lower }}] === {{ signoffs_url }} +There are currently: +* {{ new|length }} new package{{ new|length|pluralize }} in last {{ new_hours }} hours +* {{ bad|length }} known bad package{{ bad|length|pluralize }} +* {{ disabled|length }} package{{ disabled|length|pluralize }} not accepting signoffs +* {{ complete|length }} fully signed off package{{ complete|length|pluralize }} +* {{ incomplete|length }} package{{ incomplete|length|pluralize }} missing signoffs +* {{ old|length }} package{{ old|length|pluralize }} older than {{ old_days }} days + +(Note: the word 'package' as used here refers to packages as grouped by +pkgbase, architecture, and repository; e.g., one PKGBUILD produces one +package per architecture, even if it is a split package.) + + == New packages in [{{ repo|lower}}] in last {{ new_hours }} hours ({{ new|length }} total) == {% for group in new %} * {{ group.pkgbase }}-{{ group.version }} ({{ group.arch }}){% endfor %} diff --git a/templates/packages/signoffs.html b/templates/packages/signoffs.html index 9bc7fd74..d517e5e3 100644 --- a/templates/packages/signoffs.html +++ b/templates/packages/signoffs.html @@ -39,6 +39,7 @@

      Filter Displayed Signoffs

      Last Updated Approved Signoffs + Notes @@ -50,8 +51,22 @@

      Filter Displayed Signoffs

      {{ group.packager|default:"Unknown" }} {{ group.packages|length }} {{ group.last_update|date }} - {{ group.approved|yesno|capfirst }} + {% if group.specification.known_bad %} + Bad + {% else %} + {% if not group.specification.enabled %} + Disabled + {% else %} + {{ group.approved|yesno|capfirst }} + {% endif %} + {% endif %} {% include "packages/signoff_cell.html" %} + {% if not group.default_spec %}{% with group.specification as spec %} + {% if spec.required != 2 %}Required signoffs: {{ spec.required }}
      {% endif %} + {% if not spec.enabled %}Signoffs are not currently enabled
      {% endif %} + {% if spec.known_bad %}Package is known to be bad
      {% endif %} + {{ spec.comments|default:""|linebreaks }} + {% endwith %}{% endif %} {% endfor %} @@ -64,7 +79,7 @@

      Filter Displayed Signoffs

      $(document).ready(function() { $('a.signoff-link').click(signoff_package); $(".results").tablesorter({widgets: ['zebra'], sortList: [[0,0]], - headers: { 7: { sorter: false } } }); + headers: { 7: { sorter: false }, 8: {sorter: false } } }); $('#signoffs_filter input').change(filter_signoffs); $('#criteria_reset').click(filter_signoffs_reset); // fire function on page load to ensure the current form selections take effect -- cgit v1.2.3-54-g00ecf From 0aa42e2c01df2bf1c9e425994420f5ae10252597 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 21:32:30 -0500 Subject: Allow signoff manipulation if you are a maintainer This is a more expensive and not-yet-optimized way of doing this, but we can fix that later as needed. Signed-off-by: Dan McGee --- packages/utils.py | 4 ++++ templates/packages/signoff_cell.html | 2 +- templates/todolists/view.html | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index 60b95e21..1a2c0de0 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -2,6 +2,7 @@ from django.db import connection from django.db.models import Count, Max +from django.contrib.auth.models import User from main.models import Package, Repo from main.utils import cache_function, groupby_preserve_order, PackageStandin @@ -172,6 +173,9 @@ def __init__(self, packages): self.version = '' self.last_update = first.last_update self.packager = first.packager + self.maintainers = User.objects.filter( + package_relations__type=PackageRelation.MAINTAINER, + package_relations__pkgbase=self.pkgbase) self.specification = \ SignoffSpecification.objects.get_or_default_from_package(first) diff --git a/templates/packages/signoff_cell.html b/templates/packages/signoff_cell.html index 4f9f726b..0bf44ca2 100644 --- a/templates/packages/signoff_cell.html +++ b/templates/packages/signoff_cell.html @@ -17,7 +17,7 @@ title="Signoff {{ group.pkgbase }} for {{ group.arch }}">Signoff
      {% endif %} {% endif %} -{% if group.packager == user %} +{% if user == group.packager or user in group.maintainers %} diff --git a/templates/todolists/view.html b/templates/todolists/view.html index 8f515c9b..c9ea919a 100644 --- a/templates/todolists/view.html +++ b/templates/todolists/view.html @@ -29,7 +29,7 @@

      Todo List: {{ list.name }}

      Name Arch Repo - Maintainer + Maintainers Status -- cgit v1.2.3-54-g00ecf From 278d74b1d12568d4c9b6d5533e57e820d038ae64 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 22:05:35 -0500 Subject: Minor signoff query tweaks/optimizations Signed-off-by: Dan McGee --- packages/utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index 1a2c0de0..65769baf 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -248,8 +248,7 @@ def get_current_signoffs(repos): AND s.arch_id = p.arch_id AND s.repo_id = p.repo_id ) - JOIN repos r ON p.repo_id = r.id - WHERE r.id IN ( + WHERE p.repo_id IN ( """ sql += ", ".join("%s" for r in repos) sql += ")" @@ -264,15 +263,16 @@ def get_current_signoffs(repos): def get_target_repo_map(pkgbases): package_repos = Package.objects.order_by().values_list( 'pkgbase', 'repo__name').filter( - repo__testing=False, repo__staging=False, pkgbase__in=pkgbases).distinct() return dict(package_repos) def get_signoff_groups(repos=None): if repos is None: repos = Repo.objects.filter(testing=True) + repo_ids = [r.pk for r in repos] - test_pkgs = Package.objects.normal().filter(repo__in=repos) + test_pkgs = Package.objects.select_related( + 'arch', 'repo', 'packager').filter(repo__in=repo_ids) packages = test_pkgs.order_by('pkgname') # Collect all pkgbase values in testing repos -- cgit v1.2.3-54-g00ecf From 19c2841f20653fd3c59f73fdb16f7f7b1ea15434 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 22:46:36 -0500 Subject: Add new attach_maintainers() utility method This allows us to alleviate the N+1 query problem when we want maintainer data for a queryset of packages. We use it on signoffs here; we should also be able to apply this to the todolist section where this problem has existed for some time. Signed-off-by: Dan McGee --- main/models.py | 14 +++++++++++--- packages/utils.py | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) (limited to 'packages/utils.py') diff --git a/main/models.py b/main/models.py index 972b098c..db456c20 100644 --- a/main/models.py +++ b/main/models.py @@ -206,11 +206,19 @@ def get_full_url(self, proto='https'): def is_signed(self): return bool(self.pgp_signature) + _maintainers = None + @property def maintainers(self): - return User.objects.filter( - package_relations__pkgbase=self.pkgbase, - package_relations__type=PackageRelation.MAINTAINER) + if self._maintainers is None: + self._maintainers = User.objects.filter( + package_relations__pkgbase=self.pkgbase, + package_relations__type=PackageRelation.MAINTAINER) + return self._maintainers + + @maintainers.setter + def maintainers(self, maintainers): + self._maintainers = maintainers @cache_function(300) def applicable_arches(self): diff --git a/packages/utils.py b/packages/utils.py index 65769baf..0d756a85 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -1,3 +1,4 @@ +from collections import defaultdict from operator import itemgetter from django.db import connection @@ -127,6 +128,7 @@ def get_differences_info(arch_a, arch_b): differences.sort(key=lambda a: (a.repo.name, a.pkgname)) return differences + def get_wrong_permissions(): sql = """ SELECT DISTINCT id @@ -150,6 +152,32 @@ def get_wrong_permissions(): return relations +def attach_maintainers(packages): + '''Given a queryset or something resembling it of package objects, find all + the maintainers and attach them to the packages to prevent N+1 query + cascading.''' + pkgbases = set(p.pkgbase for p in packages) + rels = PackageRelation.objects.filter(type=PackageRelation.MAINTAINER, + pkgbase__in=pkgbases).values_list('pkgbase', 'user_id').distinct() + + # get all the user objects we will need + user_ids = set(rel[1] for rel in rels) + users = User.objects.in_bulk(user_ids) + + # now build a pkgbase -> [maintainers...] map + maintainers = defaultdict(list) + for rel in rels: + maintainers[rel[0]].append(users[rel[1]]) + + annotated = [] + # and finally, attach the maintainer lists on the original packages + for package in packages: + package.maintainers = maintainers[package.pkgbase] + annotated.append(package) + + return annotated + + def approved_by_signoffs(signoffs, spec): if signoffs: good_signoffs = sum(1 for s in signoffs if not s.revoked) @@ -173,9 +201,7 @@ def __init__(self, packages): self.version = '' self.last_update = first.last_update self.packager = first.packager - self.maintainers = User.objects.filter( - package_relations__type=PackageRelation.MAINTAINER, - package_relations__pkgbase=self.pkgbase) + self.maintainers = first.maintainers self.specification = \ SignoffSpecification.objects.get_or_default_from_package(first) @@ -236,6 +262,7 @@ def __unicode__(self): def get_current_signoffs(repos): '''Returns a mapping of pkgbase -> signoff objects for the given repos.''' + # TODO this isn't current at all- this is every single signoff... cursor = connection.cursor() sql = """ SELECT DISTINCT s.id @@ -274,6 +301,7 @@ def get_signoff_groups(repos=None): test_pkgs = Package.objects.select_related( 'arch', 'repo', 'packager').filter(repo__in=repo_ids) packages = test_pkgs.order_by('pkgname') + packages = attach_maintainers(packages) # Collect all pkgbase values in testing repos q_pkgbase = test_pkgs.values('pkgbase') -- cgit v1.2.3-54-g00ecf From 0db2830b8fda4d898a184a31f3375c10f3cc4083 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 23:30:16 -0500 Subject: Make maintainer lookup on todo lists fast This is rather sick to look at. Sorry, Django gives me no other choice. Signed-off-by: Dan McGee --- main/models.py | 13 +++++++++---- packages/utils.py | 1 + todolists/views.py | 8 +++++++- 3 files changed, 17 insertions(+), 5 deletions(-) (limited to 'packages/utils.py') diff --git a/main/models.py b/main/models.py index db456c20..caf36be0 100644 --- a/main/models.py +++ b/main/models.py @@ -460,12 +460,17 @@ class Todolist(models.Model): def __unicode__(self): return self.name + _packages = None + @property def packages(self): - # select_related() does not use LEFT OUTER JOIN for nullable ForeignKey - # fields. That is why we need to explicitly list the ones we want. - return TodolistPkg.objects.select_related( - 'pkg__repo', 'pkg__arch').filter(list=self).order_by('pkg') + if not self._packages: + # select_related() does not use LEFT OUTER JOIN for nullable + # ForeignKey fields. That is why we need to explicitly list the + # ones we want. + self._packages = TodolistPkg.objects.select_related( + 'pkg__repo', 'pkg__arch').filter(list=self).order_by('pkg') + return self._packages @property def package_names(self): diff --git a/packages/utils.py b/packages/utils.py index 0d756a85..4af0f67d 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -156,6 +156,7 @@ def attach_maintainers(packages): '''Given a queryset or something resembling it of package objects, find all the maintainers and attach them to the packages to prevent N+1 query cascading.''' + packages = list(packages) pkgbases = set(p.pkgbase for p in packages) rels = PackageRelation.objects.filter(type=PackageRelation.MAINTAINER, pkgbase__in=pkgbases).values_list('pkgbase', 'user_id').distinct() diff --git a/todolists/views.py b/todolists/views.py index 8ad7be56..585cefd0 100644 --- a/todolists/views.py +++ b/todolists/views.py @@ -12,6 +12,7 @@ from django.utils import simplejson from main.models import Todolist, TodolistPkg, Package +from packages.utils import attach_maintainers from .utils import get_annotated_todolists class TodoListForm(forms.ModelForm): @@ -49,6 +50,9 @@ def flag(request, listid, pkgid): @never_cache def view(request, listid): todolist = get_object_or_404(Todolist, id=listid) + # we don't hold onto the result, but the objects are the same here, + # so accessing maintainers in the template is now cheap + attach_maintainers(tp.pkg for tp in todolist.packages) return direct_to_template(request, 'todolists/view.html', {'list': todolist}) @login_required @@ -163,8 +167,10 @@ def send_todolist_emails(todo_list, new_packages): def public_list(request): todo_lists = Todolist.objects.incomplete() + # total hackjob, but it makes this a lot less query-intensive. + all_pkgs = [tp for tl in todo_lists for tp in tl.packages] + attach_maintainers([tp.pkg for tp in all_pkgs]) return direct_to_template(request, "todolists/public_list.html", {"todo_lists": todo_lists}) - # vim: set ts=4 sw=4 et: -- cgit v1.2.3-54-g00ecf From 94f46acebf03652d7ad2ed504d4ce863d5cbd913 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 4 Nov 2011 00:01:51 -0500 Subject: Find all potential package signoff specifications upfront This should save a significant amount of time in the case where there are a lot of signups to look up; at least one query per signoff row. Signed-off-by: Dan McGee --- packages/utils.py | 54 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 15 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index 4af0f67d..5240ae23 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -194,6 +194,8 @@ def __init__(self, packages): self.user = None self.target_repo = None self.signoffs = set() + self.specification = DEFAULT_SIGNOFF_SPEC + self.default_spec = True first = packages[0] self.pkgbase = first.pkgbase @@ -204,10 +206,6 @@ def __init__(self, packages): self.packager = first.packager self.maintainers = first.maintainers - self.specification = \ - SignoffSpecification.objects.get_or_default_from_package(first) - self.default_spec = self.specification is DEFAULT_SIGNOFF_SPEC - version = first.full_version if all(version == pkg.full_version for pkg in packages): self.version = version @@ -238,6 +236,17 @@ def find_signoffs(self, all_signoffs): if s.arch_id == self.arch.id and s.repo_id == self.repo.id: self.signoffs.add(s) + def find_specification(self, specifications): + for spec in specifications: + if spec.pkgbase != self.pkgbase: + continue + if self.version and not spec.full_version == self.version: + continue + if spec.arch_id == self.arch.id and spec.repo_id == self.repo.id: + self.specification = spec + self.default_spec = False + return + def approved(self): return approved_by_signoffs(self.signoffs, self.specification) @@ -261,13 +270,9 @@ def __unicode__(self): return u'%s-%s (%s): %d' % ( self.pkgbase, self.version, self.arch, len(self.signoffs)) -def get_current_signoffs(repos): - '''Returns a mapping of pkgbase -> signoff objects for the given repos.''' - # TODO this isn't current at all- this is every single signoff... - cursor = connection.cursor() - sql = """ +_SQL_SPEC_OR_SIGNOFF = """ SELECT DISTINCT s.id - FROM packages_signoff s + FROM %s s JOIN packages p ON ( s.pkgbase = p.pkgbase AND s.pkgver = p.pkgver @@ -276,11 +281,16 @@ def get_current_signoffs(repos): AND s.arch_id = p.arch_id AND s.repo_id = p.repo_id ) - WHERE p.repo_id IN ( + AND p.repo_id IN (%s) """ - sql += ", ".join("%s" for r in repos) - sql += ")" - cursor.execute(sql, [r.id for r in repos]) + +def get_current_signoffs(repos): + '''Returns a mapping of pkgbase -> signoff objects for the given repos.''' + cursor = connection.cursor() + # query pre-process- fill in table name and placeholders for IN + sql = _SQL_SPEC_OR_SIGNOFF % ('packages_signoff', + ','.join(['%s' for r in repos])) + cursor.execute(sql, [r.pk for r in repos]) results = cursor.fetchall() # fetch all of the returned signoffs by ID @@ -288,6 +298,18 @@ def get_current_signoffs(repos): signoffs = Signoff.objects.select_related('user').in_bulk(to_fetch) return signoffs.values() +def get_current_specifications(repos): + '''Returns a mapping of pkgbase -> signoff specification objects for the + given repos.''' + cursor = connection.cursor() + sql = _SQL_SPEC_OR_SIGNOFF % ('packages_signoffspecification', + ','.join(['%s' for r in repos])) + cursor.execute(sql, [r.pk for r in repos]) + + results = cursor.fetchall() + to_fetch = [row[0] for row in results] + return SignoffSpecification.objects.in_bulk(to_fetch).values() + def get_target_repo_map(pkgbases): package_repos = Package.objects.order_by().values_list( 'pkgbase', 'repo__name').filter( @@ -308,8 +330,9 @@ def get_signoff_groups(repos=None): q_pkgbase = test_pkgs.values('pkgbase') pkgtorepo = get_target_repo_map(q_pkgbase) - # Collect all existing signoffs for these packages + # Collect all possible signoffs and specifications for these packages signoffs = get_current_signoffs(repos) + specs = get_current_specifications(repos) same_pkgbase_key = lambda x: (x.repo.name, x.arch.name, x.pkgbase) grouped = groupby_preserve_order(packages, same_pkgbase_key) @@ -319,6 +342,7 @@ def get_signoff_groups(repos=None): signoff_group.target_repo = pkgtorepo.get(signoff_group.pkgbase, "Unknown") signoff_group.find_signoffs(signoffs) + signoff_group.find_specification(specs) signoff_groups.append(signoff_group) return signoff_groups -- cgit v1.2.3-54-g00ecf From 20ebe658921c2ce78bf9a05116de045ee38f0820 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 4 Nov 2011 10:43:02 -0500 Subject: Fix signoff target repo mapping I clearly should not have removed this code yesterday, otherwise packages have their target repo matched to a testing one. Signed-off-by: Dan McGee --- packages/utils.py | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index 5240ae23..ddd822e4 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -313,6 +313,7 @@ def get_current_specifications(repos): def get_target_repo_map(pkgbases): package_repos = Package.objects.order_by().values_list( 'pkgbase', 'repo__name').filter( + repo__testing=False, repo__staging=False, pkgbase__in=pkgbases).distinct() return dict(package_repos) -- cgit v1.2.3-54-g00ecf From 28f72db7be7bf2f54d734c78422e6179f0ce29f1 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 4 Nov 2011 12:38:57 -0500 Subject: Rewrite get_target_repo_map() using raw SQL This improves the shitty query plan brought upon us by MySQL by rewriting it to use JOINs only and no dependent subqueries. Signed-off-by: Dan McGee --- packages/utils.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index ddd822e4..b21ac557 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -310,12 +310,25 @@ def get_current_specifications(repos): to_fetch = [row[0] for row in results] return SignoffSpecification.objects.in_bulk(to_fetch).values() -def get_target_repo_map(pkgbases): - package_repos = Package.objects.order_by().values_list( - 'pkgbase', 'repo__name').filter( - repo__testing=False, repo__staging=False, - pkgbase__in=pkgbases).distinct() - return dict(package_repos) +def get_target_repo_map(repos): + sql = """ +SELECT DISTINCT p1.pkgbase, r.name + FROM packages p1 + JOIN repos r ON p1.repo_id = r.id + JOIN packages p2 ON p1.pkgbase = p2.pkgbase + WHERE r.staging = %s + AND r.testing = %s + AND p2.repo_id IN ( + """ + sql += ','.join(['%s' for r in repos]) + sql += ")" + + params = [False, False] + params.extend(r.pk for r in repos) + + cursor = connection.cursor() + cursor.execute(sql, params) + return dict(cursor.fetchall()) def get_signoff_groups(repos=None): if repos is None: @@ -328,8 +341,7 @@ def get_signoff_groups(repos=None): packages = attach_maintainers(packages) # Collect all pkgbase values in testing repos - q_pkgbase = test_pkgs.values('pkgbase') - pkgtorepo = get_target_repo_map(q_pkgbase) + pkgtorepo = get_target_repo_map(repos) # Collect all possible signoffs and specifications for these packages signoffs = get_current_signoffs(repos) -- cgit v1.2.3-54-g00ecf From 022692b3f33de8c45741d3cb27fa95f9f6facdea Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 11 Nov 2011 10:43:18 -0600 Subject: Show relevant signoffs on dashboard Signed-off-by: Dan McGee --- devel/views.py | 5 +++++ packages/utils.py | 7 ++++++- templates/devel/index.html | 50 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 3 deletions(-) (limited to 'packages/utils.py') diff --git a/devel/views.py b/devel/views.py index 694ed6dc..0002df04 100644 --- a/devel/views.py +++ b/devel/views.py @@ -18,6 +18,7 @@ from main.models import Arch, Repo from main.models import UserProfile from packages.models import PackageRelation +from packages.utils import get_signoff_groups from todolists.utils import get_annotated_todolists from .utils import get_annotated_maintainers @@ -48,6 +49,9 @@ def index(request): todolists = get_annotated_todolists() todolists = [todolist for todolist in todolists if todolist.incomplete_count > 0] + signoffs = sorted(get_signoff_groups(user=request.user), + key=operator.attrgetter('pkgbase')) + maintainers = get_annotated_maintainers() maintained = PackageRelation.objects.filter( @@ -70,6 +74,7 @@ def index(request): 'orphan': orphan, 'flagged' : flagged, 'todopkgs' : todopkgs, + 'signoffs': signoffs } return direct_to_template(request, 'devel/index.html', page_dict) diff --git a/packages/utils.py b/packages/utils.py index b21ac557..0df0e382 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -330,7 +330,7 @@ def get_target_repo_map(repos): cursor.execute(sql, params) return dict(cursor.fetchall()) -def get_signoff_groups(repos=None): +def get_signoff_groups(repos=None, user=None): if repos is None: repos = Repo.objects.filter(testing=True) repo_ids = [r.pk for r in repos] @@ -340,6 +340,11 @@ def get_signoff_groups(repos=None): packages = test_pkgs.order_by('pkgname') packages = attach_maintainers(packages) + # Filter by user if asked to do so + if user is not None: + packages = [p for p in packages if user == p.packager + or user in p.maintainers] + # Collect all pkgbase values in testing repos pkgtorepo = get_target_repo_map(repos) diff --git a/templates/devel/index.html b/templates/devel/index.html index d3f7ec3b..06cf10ab 100644 --- a/templates/devel/index.html +++ b/templates/devel/index.html @@ -15,8 +15,8 @@

      My Flagged Packages

      Name - Repo Version + Repo Arch Flagged Last Updated @@ -26,8 +26,8 @@

      My Flagged Packages

      {% for pkg in flagged %} {% pkg_details_link pkg %} - {{ pkg.repo.name }} {{ pkg.full_version }} + {{ pkg.repo.name }} {{ pkg.arch.name }} {{ pkg.flag_date|date }} {{ pkg.last_update|date }} @@ -96,6 +96,47 @@

      Package Todo Lists

      +

      Signoff Status

      + + + + + + + + + + + + + + + {% for group in signoffs %} + + + + + + + {% if group.specification.known_bad %} + + {% else %} + {% if not group.specification.enabled %} + + {% else %} + + {% endif %} + {% endif %} + + + {% endfor %} + +
      NameVersionArchTarget RepoLast UpdatedApprovedSignoffs
      {% pkg_details_link group.package %}{{ group.version }}{{ group.arch.name }}{{ group.target_repo }}{{ group.last_update|date }}BadDisabled{{ group.approved|yesno|capfirst }}
        + {% for signoff in group.signoffs %} +
      • {{ signoff.user }}{% if signoff.revoked %} (revoked){% endif %}
        • + {% endfor %} +
      +

      Developer Reports

      • Big: @@ -255,6 +296,11 @@

        Stats by Developer

        {widgets: ['zebra'], sortList: [[0,0], [1,0]]}); $("#dash-todo:not(:has(tbody tr.empty))").tablesorter( {widgets: ['zebra'], sortList: [[1,1]]}); + $("#dash-signoffs:not(:has(tbody tr.empty))").tablesorter({ + widgets: ['zebra'], + sortList: [[0,0]], + headers: { 6: {sorter: false } } + }); $(".dash-stats").tablesorter({ widgets: ['zebra'], sortList: [[0,0]], -- cgit v1.2.3-54-g00ecf From a883b0af23143364ab0724fda2ecdef9aba8191f Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 11 Nov 2011 11:57:04 -0600 Subject: Add a split packages sitemap With very low priority, but this should at least give a few more cross-linking pages to any crawlers using sitemaps. Signed-off-by: Dan McGee --- packages/utils.py | 18 ++++++++++++++++-- sitemaps.py | 17 ++++++++++++++++- urls.py | 1 + 3 files changed, 33 insertions(+), 3 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index 0df0e382..f8e1f2a1 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -2,10 +2,10 @@ from operator import itemgetter from django.db import connection -from django.db.models import Count, Max +from django.db.models import Count, Max, F from django.contrib.auth.models import User -from main.models import Package, Repo +from main.models import Package, Arch, Repo from main.utils import cache_function, groupby_preserve_order, PackageStandin from .models import (PackageGroup, PackageRelation, SignoffSpecification, Signoff, DEFAULT_SIGNOFF_SPEC) @@ -49,6 +49,20 @@ def get_group_info(include_arches=None): groups.extend(val.itervalues()) return sorted(groups, key=itemgetter('name', 'arch')) +def get_split_packages_info(): + '''Return info on split packages that do not have an actual package name + matching the split pkgbase.''' + pkgnames = Package.objects.values('pkgname') + split_pkgs = Package.objects.exclude(pkgname=F('pkgbase')).exclude( + pkgbase__in=pkgnames).values('pkgbase', 'repo', 'arch').annotate( + last_update=Max('last_update')) + all_arches = Arch.objects.in_bulk(set(s['arch'] for s in split_pkgs)) + all_repos = Repo.objects.in_bulk(set(s['repo'] for s in split_pkgs)) + for split in split_pkgs: + split['arch'] = all_arches[split['arch']] + split['repo'] = all_repos[split['repo']] + return split_pkgs + class Difference(object): def __init__(self, pkgname, repo, pkg_a, pkg_b): self.pkgname = pkgname diff --git a/sitemaps.py b/sitemaps.py index 8ac5bc4f..7718002d 100644 --- a/sitemaps.py +++ b/sitemaps.py @@ -3,7 +3,7 @@ from main.models import Package from news.models import News -from packages.utils import get_group_info +from packages.utils import get_group_info, get_split_packages_info class PackagesSitemap(Sitemap): changefreq = "weekly" @@ -41,6 +41,21 @@ def location(self, obj): return '/groups/%s/%s/' % (obj['arch'], obj['name']) +class SplitPackagesSitemap(Sitemap): + changefreq = "weekly" + priority = "0.3" + + def items(self): + return get_split_packages_info() + + def lastmod(self, obj): + return obj['last_update'] + + def location(self, obj): + return '/packages/%s/%s/%s/' % ( + obj['repo'].name.lower(), obj['arch'], obj['pkgbase']) + + class NewsSitemap(Sitemap): changefreq = "never" priority = "0.8" diff --git a/urls.py b/urls.py index adbc8870..1d06f0f2 100644 --- a/urls.py +++ b/urls.py @@ -18,6 +18,7 @@ 'packages': sitemaps.PackagesSitemap, 'package-files': sitemaps.PackageFilesSitemap, 'package-groups': sitemaps.PackageGroupsSitemap, + 'split-packages': sitemaps.SplitPackagesSitemap, } admin.autodiscover() -- cgit v1.2.3-54-g00ecf From f71f3e23eda8e1ff447340812025ab724aff14cb Mon Sep 17 00:00:00 2001 From: Johannes Krampf Date: Sat, 3 Dec 2011 14:18:07 +0100 Subject: Fix differences query if there are more than 2 architectures --- packages/utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'packages/utils.py') diff --git a/packages/utils.py b/packages/utils.py index f8e1f2a1..82d47bc7 100644 --- a/packages/utils.py +++ b/packages/utils.py @@ -106,7 +106,11 @@ def get_differences_info(arch_a, arch_b): AND p.arch_id != q.arch_id AND p.id != q.id ) - WHERE p.arch_id IN (%s, %s) + WHERE p.arch_id in (%s, %s) + AND ( + q.arch_id in (%s, %s) + OR q.id IS NULL + ) AND ( q.id IS NULL OR p.pkgver != q.pkgver @@ -115,7 +119,7 @@ def get_differences_info(arch_a, arch_b): ) """ cursor = connection.cursor() - cursor.execute(sql, [arch_a.id, arch_b.id]) + cursor.execute(sql, [arch_a.id, arch_b.id, arch_a.id, arch_b.id]) results = cursor.fetchall() # column A will always have a value, column B might be NULL to_fetch = [row[0] for row in results] -- cgit v1.2.3-54-g00ecf