launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

QueueItemsView is caching some extra attributes that are not beign cached on PackageUploadSet, but there was room for a small improvement anyway.

This should be ok to be reviewed, but should only be merged once the other MP is approved.

Diff comments:

> diff --git a/lib/lp/soyuz/browser/queue.py b/lib/lp/soyuz/browser/queue.py
> index a476e1d..02d5f54 100644
> --- a/lib/lp/soyuz/browser/queue.py
> +++ b/lib/lp/soyuz/browser/queue.py
> @@ -222,10 +223,8 @@ class QueueItemsView(LaunchpadView):
>              return None
>  
>          upload_ids = [upload.id for upload in uploads]
> -        puses = load_referencing(
> -            PackageUploadSource, uploads, ['packageuploadID'])
> -        pubs = load_referencing(
> -            PackageUploadBuild, uploads, ['packageuploadID'])
> +        puses = sum([u.sources for u in uploads], [])
> +        pubs = sum([u.builds for u in uploads], [])

Those attributes are already pre-cached by PackageUploadSet.getAll. This change saves 2 extra queries.

>  
>          source_sprs = load_related(
>              SourcePackageRelease, puses, ['sourcepackagereleaseID'])
> @@ -491,8 +490,6 @@ class CompletePackageUpload:
>      # (i.e. no proxying of __set__).
>      pocket = None
>      date_created = None
> -    sources = None
> -    builds = None

Sources and builds are pre-cached on `packageupload`. No need to have another list pre-cached here too

>      customfiles = None
>      contains_source = None
>      contains_build = None
> diff --git a/lib/lp/soyuz/interfaces/queue.py b/lib/lp/soyuz/interfaces/queue.py
> index c57da73..a3e915e 100644
> --- a/lib/lp/soyuz/interfaces/queue.py
> +++ b/lib/lp/soyuz/interfaces/queue.py
> @@ -503,9 +552,11 @@ class IPackageUploadBuild(Interface):
>              readonly=False,
>              )
>  
> -    build = Int(
> -            title=_("The related build"), required=True, readonly=False,
> -            )
> +    build = Int(title=_("The related build"), required=True, readonly=False)
> +
> +    buildID = Int(
> +        title=_("The Related build ID"), required=True,
> +        readonly=True)

Adding this here to avoid security problems when reading buildID to pre-fetching things on QueueItemsView.

>  
>      def binaries():
>          """Returns the properties of the binaries in this build.
> @@ -548,8 +599,11 @@ class IPackageUploadSource(Interface):
>  
>      sourcepackagerelease = Int(
>              title=_("The related source package release"), required=True,
> -            readonly=False,
> -            )
> +            readonly=False)
> +
> +    sourcepackagereleaseID = Int(
> +            title=_("The related source package release ID"), required=True,
> +            readonly=True)

Adding it here for the same reason buildID was added above

>  
>      def getSourceAncestryForDiffs():
>          """Return a suitable ancestry publication for this context.


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/377897
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:pkg-upload-log-api into launchpad:master.