launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/snappy/browser/snapbuild.py'
> --- lib/lp/snappy/browser/snapbuild.py	2016-07-16 07:46:23 +0000
> +++ lib/lp/snappy/browser/snapbuild.py	2016-07-19 17:18:24 +0000
> @@ -84,25 +87,40 @@
>      def has_files(self):
>          return bool(self.files)
>  
> -    @cachedproperty
> -    def store_upload_status(self):
> -        job = self.context.store_upload_jobs.first()
> -        if job is None:
> -            return None
> -        elif job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
> -            return "Store upload in progress"
> -        elif job.job.status == JobStatus.COMPLETED:
> -            return structured(
> -                '<a href="%s">Manage this package in the store</a>',
> -                job.store_url).escapedtext
> -        elif job.store_url:
> -            return structured(
> -                '<a href="%s">Manage this package in the store</a><br />'
> -                'Releasing package to channels failed: %s',
> -                job.store_url, job.error_message).escapedtext
> -        else:
> -            return structured(
> -                "Store upload failed: %s", job.error_message).escapedtext
> +    @property
> +    def last_upload_job(self):
> +        return self.context.store_upload_jobs.first()
> +
> +    @property
> +    def next_url(self):
> +        return canonical_url(self.context)
> +
> +    @action('Upload build to store', name='upload')
> +    def upload_action(self, action, data):
> +        """Schedule an upload of this build to the store."""
> +        if not self.context.snap.can_upload_to_store:
> +            self.request.response.addWarningNotification(
> +                "Cannot upload this package to the store because it is not "
> +                "properly configured.")
> +            return
> +        if not self.has_files:
> +            self.request.response.addWarningNotification(
> +                "Cannot upload this package because it has no files.")
> +            return
> +        job = self.last_upload_job
> +        if job is not None:
> +            if job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
> +                self.request.response.addWarningNotification(
> +                    "An upload of this package is already in progress.")
> +                return
> +            if job.job.status == JobStatus.COMPLETED:
> +                self.request.response.addWarningNotification(
> +                    "Cannot upload this package because it has already "
> +                    "been uploaded.")
> +                return
> +        getUtility(ISnapStoreUploadJobSource).create(self.context)
> +        self.request.response.addInfoNotification(
> +            "An upload has been scheduled and will run as soon as possible.")

Should this be encapsulated in the model and exposed on the API? It seems most common that a CI system might want to ask LP to build the package, run some quick tests on the binary, then poke LP to upload and release it.

>  
>  
>  class SnapBuildCancelView(LaunchpadFormView):


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-build-manual-upload/+merge/300503
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.