Re: [dak/master] Use distinct() to avoid listing duplicated claimed overrides

[Ansgar Burchardt <ansgar@debian.org>]

On 04/26/2013 12:56, Luca Falavigna wrote:
> --- a/dak/process_new.py
> +++ b/dak/process_new.py
> @@ -126,11 +126,16 @@ def claimed_overrides(upload, missing, session):
>      for m in missing:
>          if m['type'] != 'dsc':
>              binaries.remove(m['package'])
> -    return session.query(DBBinary).filter(DBBinary.package.in_(binaries)). \
> -                         join(DBBinary.source). \
> -                         filter(not_(DBSource.source.in_(source))). \
> -                         join(DBBinary.suites). \
> -                         filter(Suite.suite_name.in_(suites))
> +    if binaries:
> +        return session.query(DBBinary.package, DBSource.source).distinct(). \
> +                             filter(DBBinary.package.in_(binaries)). \
> +                             join(DBBinary.source). \
> +                             filter(not_(DBSource.source.in_(source))). \
> +                             join(DBBinary.suites). \
> +                             filter(Suite.suite_name.in_(suites)). \
> +                             order_by(DBSource.source, DBBinary.package)
> +    else:
> +        return None

Why not just return an empty list instead of None?

> @@ -154,10 +159,10 @@ def print_new (upload, missing, indexed, session, file=sys.stdout):
>              line = line + ' [!]'
>          print >>file, line
>      claimed = claimed_overrides(upload, missing, session)
> -    if claimed.count():
> +    if claimed and claimed.count():

I think this does two SQL queries: one to check count() > 0 and another
one to iterate over the packages. Just use

  claimed = claimed_overrides(...).all()

to get a list then check len(claimed) > 0. Or include the .all() in the
claimed_overrides function.

Ansgar