Hi Luciano, On Thu, Jun 09, 2016 at 12:01:09PM +0200, Luciano Bello wrote: > In the security team we would like to give information about which packages > you should update when we release a DSA (currently, we give to the user the > source package name). It would be easier for us if we have a way to get the > binaries for packages in (old)stable. Sources.d.n is the way to go, I think. > For that, this bug needs to be done :)
I don't mind at all you using sources.d.n for that purpose, but why not tracker.d.o? Code comments below: > diff --git a/debsources/app/infobox.py b/debsources/app/infobox.py > index dd6585c..3f75131 100644 > --- a/debsources/app/infobox.py > +++ b/debsources/app/infobox.py > @@ -14,7 +14,7 @@ from __future__ import absolute_import > from flask import url_for, current_app > > from debsources.models import ( > - PackageName, Package, Suite, SlocCount, Metric, Ctag) > + PackageName, Package, Suite, SlocCount, Metric, Ctag, Binary, BinaryName) > from debsources.excepts import Http500Error, Http404Error > > PTS_PREFIX = "https://tracker.debian.org/pkg/" > @@ -63,6 +63,22 @@ class Infobox(object): > > return [x[0] for x in suites] > > + def _get_binary_names(self): > + """ binary package names """ > + try: > + print self.package, self.version unnecessary debug line? > + binaries = (self.session.query(BinaryName.name) > + .filter(PackageName.name == self.package, > + Package.version == self.version, > + Package.name_id == PackageName.id, > + Binary.package_id == Package.id, > + Binary.name_id == BinaryName.id) > + .all()) > + except Exception as e: # pragma: no cover > + raise Http500Error(e) > + > + return [x[0] for x in binaries] > + > def _get_sloc(self): > """ sloccount """ > try: > @@ -160,3 +176,9 @@ class Infobox(object): > pkg_infos['copyright'] = False > > return pkg_infos > + > + def get_binaries(self): > + """ > + Retrieves the binary package names, in a list. > + """ > + return self._get_binary_names() I don't understand why you don't store the binary names in pkg_infos, instead of creating a different method? > diff --git a/debsources/app/views.py b/debsources/app/views.py > index e241f83..8ede294 100644 > --- a/debsources/app/views.py > +++ b/debsources/app/views.py > @@ -510,7 +510,9 @@ class PackageVersionsView(GeneralView): > # INFO PAGES # > class InfoPackageView(GeneralView): > def get_objects(self, package, version): > - pkg_infos = Infobox(session, package, version).get_infos() > + ib = Infobox(session, package, version) > + pkg_infos = ib.get_infos() > + binaries = ib.get_binaries() Same remark as above, having everything in pkg_infos.get_infos would be simpler IMHO. Unless I'm missing something? > return dict(pkg_infos=pkg_infos, > package=package, > - version=version) > + version=version,binaries=binaries) > diff --git a/debsources/db_storage.py b/debsources/db_storage.py > index 7f58c10..e6ed437 100644 > --- a/debsources/db_storage.py > +++ b/debsources/db_storage.py > @@ -14,7 +14,7 @@ from __future__ import absolute_import > import logging > > from debsources import fs_storage > -from debsources.models import File, Package, PackageName, SuiteInfo, Suite > +from debsources.models import File, Package, PackageName, SuiteInfo, Suite, > BinaryName, Binary > from debsources.models import VCS_TYPES > > > @@ -65,6 +65,21 @@ def add_package(session, pkg, pkgdir, sticky=False): > session.flush() > file_table[relpath] = file_.id > > + for binpkg in pkg['binary'].split(', '): > + logging.debug('add to db %s (binary) ...' % binpkg) > + > + binary_name = session.query(BinaryName) \ > + .filter_by(name=binpkg) \ > + .first() > + > + if not binary_name: > + binary_name = BinaryName(binpkg) > + session.add(binary_name) > + session.flush() > + > + > session.add(Binary(name_id=binary_name.id,package_id=db_package.id)) > + > + session.flush() > return file_table > > > diff --git a/debsources/models.py b/debsources/models.py > index c0e4583..468d28b 100644 > --- a/debsources/models.py > +++ b/debsources/models.py > @@ -207,8 +207,10 @@ class Binary(Base): > ForeignKey('packages.id', ondelete="CASCADE"), > index=True, nullable=False) > > - def __init__(self, version, area="main"): > - self.version = version > + def __init__(self, name_id, package_id, version=None): > + self.version = version > + self.name_id = name_id > + self.package_id = package_id > > def __repr__(self): > return self.version Overall, looks really good! If you fix the (very small) comments and add a test case I'll happily merge it. Thanks for your work! Cheers, -- Matthieu