launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> 
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py	2019-01-09 13:07:24 +0000
> +++ lib/lp/code/model/gitrepository.py	2019-01-28 17:34:23 +0000
> @@ -746,6 +746,17 @@
>          """
>          return set()
>  
> +    def getLatestScanJob(self):
> +        """See `IGitRepository`."""
> +        from lp.code.model.gitjob import GitJob, GitRefScanJob
> +        latest_job = IStore(GitJob).find(
> +            GitJob,
> +            GitJob.repository == self,
> +            Job.date_finished != None,

Hm.  Won't this mean you see the rescan link even if a rescan is already in progress (because the last scan with non-NULL date_finished, and the rescan has NULL date_finished because it hasn't finished yet)?  I think you should probably drop the date_finished condition here, but keep the order_by; nulls will be returned first, so you'll get any in-progress scans before any finished scans, and this problem will go away.

Branch.getLatestScanJob has the same problem; sorry for not spotting it there.

> +            GitJob.job_type == GitRefScanJob.class_job_type).order_by(
> +                Desc(Job.date_finished)).first()
> +        return latest_job
> +
>      def visibleByUser(self, user):
>          """See `IGitRepository`."""
>          if self.information_type in PUBLIC_INFORMATION_TYPES:


-- 
https://code.launchpad.net/~twom/launchpad/manually-rescan-git-link/+merge/362341
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.