Code review comment for lp:~jameinel/maas/1.2-kernel-option-tags

On Thursday 08 November 2012 05:49:18 you wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/8/2012 9:28 AM, Julian Edwards wrote:
> > On Thursday 08 November 2012 05:05:24 you wrote:
> >>> + tags = tags.order_by('name')[:1] + tag = get_one(tags) + if
> >>> tag is not None:
> >>>
> >>> I trust this is sensible ORM magic.
> >>
> >> The order_by is obvious, the [:1] is how you do a LIMIT 1
> >> request, get_one is a helper that checks that we get either None
> >> or exactly 1 object (there is also a get_first helper, though I'm
> >> not sure if that gets the LIMIT passed down into the DB. It uses
> >> islice instead of [:1], which makes me think it is iterating the
> >> cursor/queryset. Which means the DB might have to do all the work
> >> just to return 1 item.
> >
> > You might want to fix get_first() if you think it's not DTRT, and
> > then use it (so we're consistent across the code base).
> >
> > Cheers.
>
> So I think get_first is intending to support more than one use case.
> Namely it is trying to allow you to pass any iterable, not just
> something that can be passed a slice argument.
>
> Specifically, the test suite explicitly passes in generators, etc.
>
> So while I'm pretty sure it isn't doing a LIMIT in SQL, the
> infrastructure as defined is intentionally not doing so.
>
> Though I will say the only place that uses it in 'maasserver' that
> isn't a test would be happy to LIMIT.
>
> John
> =:->

I think we need to work out if it is causing a LIMIT when it generates the
SQL. If not, I think we should make it do what your code does and fix the
tests.

I'd add a comment about get_one near your code so it can be grepped and fixed
later if needs be, don't worry about it in this branch.

Cheers.