launchpad-reviewers team mailing list archive

[Dimitri John Ledkov <mp+447665@xxxxxxxxxxxxxxxxxx>]

Dimitri John Ledkov has proposed merging ~xnox/launchpad:snap-request-builds-mimick-auto into launchpad:master.

Commit message:
snappy: Make requestbuild use auto-build archive and pocket defaults

Make request snap builds page for a snap, reuse auto-build archive & pocket, if set on the snap.

Proof of concept untested code, no tests added.

LP: #2028687

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2028687 in Launchpad itself: "+requestbuilds and auto_builds are different by default"
  https://bugs.launchpad.net/launchpad/+bug/2028687

For more details, see:
https://code.launchpad.net/~xnox/launchpad/+git/launchpad/+merge/447665
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~xnox/launchpad:snap-request-builds-mimick-auto into launchpad:master.

diff --git a/charm/launchpad-librarian/README.md b/charm/launchpad-librarian/README.md
index 99e7701..8c116c8 100644
--- a/charm/launchpad-librarian/README.md
+++ b/charm/launchpad-librarian/README.md
@@ -44,7 +44,8 @@ especially careful when redeploying.  The general procedure is as follows:
 1. Create a Ceph volume for locally-spooled librarian data.  On production
    this should be a 2 TiB volume to allow some breathing room if uploading
    to Swift is temporarily unavailable: `openstack volume create --size 2048
-   --description 'spooled production librarian data' librarian-data`.
+   --type Ceph_NVMe --description 'spooled production librarian data'
+   librarian-data`.
 
 1. Attach this Ceph volume to the new unit using `openstack server add
    volume`.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index c882f43..86bec58 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -729,7 +729,7 @@ public.bugsubscriptionfiltertag           = SELECT
 public.bugtag                             = SELECT
 public.charmrecipe                        = SELECT, UPDATE
 public.charmrecipebuild                   = SELECT
-public.cibuild                            = SELECT, INSERT
+public.cibuild                            = SELECT, INSERT, UPDATE
 public.codeimport                         = SELECT
 public.codereviewmessage                  = SELECT
 public.codereviewvote                     = SELECT
diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index 0634d50..edddef8 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -280,7 +280,7 @@ class CIBuild(PackageBuildMixin, StormBase):
         self.build_farm_job = build_farm_job
         self.git_repository = git_repository
         self.commit_sha1 = commit_sha1
-        self.git_refs = git_refs
+        self.git_refs = sorted(git_refs) if git_refs is not None else None
         self.distro_arch_series = distro_arch_series
         self.processor = processor
         self.virtualized = virtualized
@@ -754,7 +754,7 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
                 for cibuild in result:
                     if cibuild.git_refs is None:
                         cibuild.git_refs = []
-                    cibuild.git_refs.extend(git_refs)
+                    cibuild.git_refs = sorted(set(cibuild.git_refs + git_refs))
             raise CIBuildAlreadyRequested
 
         build = self.new(
diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
index bc3fd83..0847d23 100644
--- a/lib/lp/code/model/tests/test_cibuild.py
+++ b/lib/lp/code/model/tests/test_cibuild.py
@@ -815,7 +815,7 @@ class TestCIBuildSet(TestCaseWithFactory):
         self.assertTrue(build_queue.virtualized)
         self.assertIsNone(build_queue.builder_constraints)
         self.assertEqual(BuildQueueStatus.WAITING, build_queue.status)
-        self.assertEqual(git_refs, build.git_refs)
+        self.assertEqual(sorted(git_refs), build.git_refs)
 
         # Rescheduling a build for the same commit_sha1 raises an error, but
         # git_refs of the build are updated
@@ -826,10 +826,10 @@ class TestCIBuildSet(TestCaseWithFactory):
             commit_sha1,
             das,
             stages,
-            ["ref/bar"],
+            ["ref/test", "ref/bar"],
         )
         git_refs.append("ref/bar")
-        self.assertEqual(git_refs, build.git_refs)
+        self.assertEqual(sorted(git_refs), build.git_refs)
 
     def test_requestBuild_score(self):
         # CI builds have an initial queue score of 2600.
diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py
index 36b26a2..221e7e5 100644
--- a/lib/lp/code/model/tests/test_gitjob.py
+++ b/lib/lp/code/model/tests/test_gitjob.py
@@ -17,11 +17,13 @@ from testtools.matchers import (
     MatchesSetwise,
     MatchesStructure,
 )
+from zope.component import getUtility
 from zope.interface import providedBy
 from zope.security.proxy import removeSecurityProxy
 
 from lp.code.adapters.gitrepository import GitRepositoryDelta
 from lp.code.enums import GitGranteeType, GitObjectType
+from lp.code.interfaces.cibuild import CIBuildAlreadyRequested, ICIBuildSet
 from lp.code.interfaces.gitjob import (
     IGitJob,
     IGitRefScanJob,
@@ -38,6 +40,7 @@ from lp.code.model.gitjob import (
 from lp.code.tests.helpers import GitHostingFixture
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
 from lp.services.job.runner import JobRunner
 from lp.services.utils import seconds_since_epoch
 from lp.services.webapp import canonical_url
@@ -457,6 +460,36 @@ class TestGitRefScanJob(TestCaseWithFactory):
             payload,
         )
 
+    def test_branchscanner_job_can_requestBuild(self):
+        # During GitRefScanJobs, CIBuilds might be triggered by dbuser
+        # `branchscanner`. Ensure dbuser can create and update CIBuilds
+        repository = self.factory.makeGitRepository()
+        commit_sha1 = hashlib.sha1(self.factory.getUniqueBytes()).hexdigest()
+        das = self.factory.makeBuildableDistroArchSeries()
+
+        with dbuser("branchscanner"):
+            build = getUtility(ICIBuildSet).requestBuild(
+                repository,
+                commit_sha1,
+                das,
+                [[("test", 0)]],
+            )
+            IStore(repository).flush()
+
+            # Create duplicated build
+            self.assertRaises(
+                CIBuildAlreadyRequested,
+                getUtility(ICIBuildSet).requestBuild,
+                repository,
+                commit_sha1,
+                das,
+                [[("test", 0)]],
+                git_refs=["refs/heads/test"],
+            )
+            IStore(repository).flush()
+
+        self.assertEqual(["refs/heads/test"], build.git_refs)
+
 
 class TestReclaimGitRepositorySpaceJob(TestCaseWithFactory):
     """Tests for `ReclaimGitRepositorySpaceJob`."""
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index 5a74635..ea96830 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -477,7 +477,7 @@ class SnapRequestBuildsView(LaunchpadFormView):
     def initial_values(self):
         """See `LaunchpadFormView`."""
         return {
-            "archive": (
+            "archive": self.context.auto_build_archive or (
                 # XXX cjwatson 2019-02-04: In order to support non-Ubuntu
                 # bases, we'd need to store this as None and infer it based
                 # on the inferred distro series; but this will do for now.
@@ -486,7 +486,7 @@ class SnapRequestBuildsView(LaunchpadFormView):
                 else self.context.distro_series.main_archive
             ),
             "distro_arch_series": [],
-            "pocket": PackagePublishingPocket.UPDATES,
+            "pocket": self.context.auto_build_pocket or PackagePublishingPocket.UPDATES,
             "channels": self.context.auto_build_channels,
         }