On Wed, Aug 11, 2010 at 7:54 PM, Edwin Grubbs
<[email protected]> wrote:
> Review: Approve code
> Hi Michael,
>
> This is a nice improvement. Since the TeamParticipation table won't be used
> if the user is anonymous or an admin, it would be a small but easy
> optimization to not join it into those queries. For example:
>
> origin = [
> BuildFarmJob,
> LeftJoin(PackageBuild, ...),
> LeftJoin(Archive, ...),
> ]
>
> if ANONYMOUS:
> ...
> elif ADMIN:
> ...
> else:
> origin.append(LeftJoin(TeamParticipation, ...))
>
> filtered_builds = IStore(BuildFarmJob).using(*origin).find(...)
>
> I guess if an admin is running the query, there is no reason to use any
> tables besides BuildFarmJob.
Yes... good point. I've attached an incremental (pushed as r9651) that
only uses the joins when necessary. Thanks!
--
https://code.launchpad.net/~michael.nelson/launchpad/616331-private-builds-in-builder-history/+merge/32355
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~michael.nelson/launchpad/616331-private-builds-in-builder-history into
lp:launchpad.
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2010-08-11 17:09:45 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-08-11 21:26:07 +0000
@@ -365,15 +365,18 @@
# Currently only package builds can be private (via their
# related archive), but not all build farm jobs will have a
# related package build - hence the left join.
- left_join_pkg_builds = LeftJoin(
- PackageBuild, PackageBuild.build_farm_job == BuildFarmJob.id)
- left_join_archive = LeftJoin(
- Archive, PackageBuild.archive == Archive.id)
- left_join_participation = LeftJoin(
- TeamParticipation, TeamParticipation.teamID == Archive.ownerID)
+ origin = [BuildFarmJob]
+ left_join_archive = [
+ LeftJoin(
+ PackageBuild,
+ PackageBuild.build_farm_job == BuildFarmJob.id),
+ LeftJoin(
+ Archive, PackageBuild.archive == Archive.id),
+ ]
if user is None:
# Anonymous requests don't get to see private builds at all.
+ origin.extend(left_join_archive)
extra_clauses.append(Coalesce(Archive.private, False) == False)
elif user.inTeam(getUtility(ILaunchpadCelebrities).admin):
@@ -382,16 +385,15 @@
else:
# Everyone else sees all public builds and the
# specific private builds to which they have access.
+ origin.extend(left_join_archive)
+ origin.append(LeftJoin(
+ TeamParticipation, TeamParticipation.teamID == Archive.ownerID))
extra_clauses.append(
Or(Coalesce(Archive.private, False) == False,
TeamParticipation.person == user))
- filtered_builds = IStore(BuildFarmJob).using(
- BuildFarmJob,
- left_join_pkg_builds,
- left_join_archive,
- left_join_participation,
- ).find(BuildFarmJob, *extra_clauses)
+ filtered_builds = IStore(BuildFarmJob).using(*origin).find(
+ BuildFarmJob, *extra_clauses)
filtered_builds.order_by(
Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
