launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-837893 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #837893 in Launchpad itself: "distroseriesdifferencejob needs SELECT on PackagesetSources"
  https://bugs.launchpad.net/launchpad/+bug/837893

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-837893/+merge/73502

= Summary =

The DistroSeriesDifferenceJob runner was failing in a scenario that burrowed deeploy through layers of conditions.  It was lacking some select privileges on the database.  Annoyingly, the data in question should be completely public so I question once again the extent of our fine-grained SELECT privileges.


== Proposed fix ==

Test for the missing privileges.  Add them.  Done.

Well actually, there is one bit of fuzziness: William says we also need PackagesetGroup which didn't show up in the test failures I reproduced.  But to tickle that particular case in a test would involve digging even further into unrelated layers of abstraction and enough is enough.  There's nothing secret in that table, so I just added it.


== Pre-implementation notes ==

Based on production failure spotted by William earlier today.


== Implementation details ==

The package I'm generating a DSDJ for is not in the packageset I'm creating.  To tickle the problem case, it is sufficient that the parent series have a packageset at all.


== Tests ==

{{{
./bin/test -vvc lp.soyuz.tests.test_distroseriesdifferencejob -t passesPackagesetFilter
}}}


== Demo and Q/A ==

Run DSDJs on dogfood with these additional privileges; it should succeed.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  database/schema/security.cfg
  lib/lp/soyuz/tests/test_distroseriesdifferencejob.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-837893/+merge/73502
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-837893 into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-08-19 16:03:54 +0000
+++ database/schema/security.cfg	2011-08-31 09:55:36 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 #
 # Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -1076,12 +1076,15 @@
 public.distroseries                             = SELECT
 public.distroseriesdifference                   = SELECT, INSERT, UPDATE
 public.distroseriesparent                       = SELECT
+public.flatpackagesetinclusion                  = SELECT
 public.job                                      = SELECT, UPDATE
 public.libraryfilealias                         = SELECT
 public.libraryfilecontent                       = SELECT
 public.packagecopyjob                           = SELECT
 public.packagediff                              = SELECT
 public.packageset                               = SELECT
+public.packagesetgroup                          = SELECT
+public.packagesetsources                        = SELECT
 public.sourcepackagename                        = SELECT
 public.sourcepackagepublishinghistory           = SELECT
 public.sourcepackagerelease                     = SELECT

=== modified file 'lib/lp/soyuz/tests/test_distroseriesdifferencejob.py'
--- lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-08-26 09:17:08 +0000
+++ lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-08-31 09:55:36 +0000
@@ -953,3 +953,22 @@
         for user in script_users:
             self.layer.switchDbUser(user)
             list(dsp.parent_series.getDerivedSeries())
+
+    def test_passesPackagesetFilter(self):
+        dsp = self.factory.makeDistroSeriesParent()
+        self.factory.makePackageset(distroseries=dsp.parent_series)
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=dsp.parent_series,
+            archive=dsp.parent_series.main_archive,
+            pocket=PackagePublishingPocket.RELEASE)
+        dsdj = create_job(
+            dsp.derived_series, spph.sourcepackagerelease.sourcepackagename,
+            dsp.parent_series)
+        transaction.commit()
+
+        self.layer.switchDbUser('distroseriesdifferencejob')
+
+        dsdj.passesPackagesetFilter()
+
+        # The test is that we get here without exceptions.
+        pass