launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15702
[Merge] lp:~stevenk/launchpad/export-pu-auditor into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/export-pu-auditor into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/export-pu-auditor/+merge/171460
Add two new exported properties -- IPackageUpload.{accept,reject}or that will query auditor (if enabled via a feature flag) to return who accepted or rejected a packageupload. IPackageUploadSet.getAll() will also cache the lot of them using one GET to auditor. I've also switched to auditorfixture 0.0.6, and changed AuditorLayer to be Zopeless so the naive packageupload tests continue to work, and it now resets the auditor database between each test.
--
https://code.launchpad.net/~stevenk/launchpad/export-pu-auditor/+merge/171460
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/export-pu-auditor into lp:launchpad.
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2013-06-06 06:40:50 +0000
+++ lib/lp/soyuz/configure.zcml 2013-06-26 04:56:32 +0000
@@ -194,7 +194,9 @@
components
searchable_names
searchable_versions
- changes_file_id"/>
+ changes_file_id
+ acceptor
+ rejector"/>
<require
permission="launchpad.Edit"
attributes="
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2013-05-28 01:24:33 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2013-06-26 04:56:32 +0000
@@ -52,6 +52,7 @@
from zope.security.interfaces import Unauthorized
from lp import _
+from lp.registry.interfaces.person import IPerson
from lp.soyuz.enums import PackageUploadStatus
from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJob
@@ -277,6 +278,18 @@
on all the binarypackagerelease records arising from the build.
""")
+ acceptor = exported(
+ Reference(
+ schema=IPerson,
+ description=_("The person who accepted this upload, if any."),
+ title=_("Acceptor"), required=False, readonly=True), as_of="devel")
+
+ rejector = exported(
+ Reference(
+ schema=IPerson,
+ description=_("The person who rejected this upload, if any."),
+ title=_("Rejector"), required=False, readonly=True), as_of="devel")
+
@export_read_operation()
@operation_for_version("devel")
def sourceFileUrls():
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/model/queue.py 2013-06-26 04:56:32 +0000
@@ -301,6 +301,24 @@
else:
return None
+ def _fetch_actor_from_auditor(self, operation):
+ client = AuditorClient()
+ data = client.receive(obj=self, operation=operation, limit=1)
+ if data:
+ return data[0]['actor']
+
+ @cachedproperty
+ def acceptor(self):
+ """See `IPackageUpload`."""
+ if bool(getFeatureFlag('auditor.enabled')):
+ return self._fetch_actor_from_auditor('packageupload-accepted')
+
+ @cachedproperty
+ def rejector(self):
+ """See `IPackageUpload`."""
+ if bool(getFeatureFlag('auditor.enabled')):
+ return self._fetch_actor_from_auditor('packageupload-rejected')
+
def getFileByName(self, filename):
"""See `IPackageUpload`."""
if (self.changesfile is not None and
@@ -599,6 +617,7 @@
if bool(getFeatureFlag('auditor.enabled')):
client = AuditorClient()
client.send(self, 'packageupload-accepted', user)
+ del get_property_cache(self).acceptor
def rejectFromQueue(self, user, logger=None, dry_run=False, comment=None):
"""See `IPackageUpload`."""
@@ -632,6 +651,7 @@
if bool(getFeatureFlag('auditor.enabled')):
client = AuditorClient()
client.send(self, 'packageupload-rejected', user)
+ del get_property_cache(self).rejector
def _isSingleSourceUpload(self):
"""Return True if this upload contains only a single source."""
@@ -1715,6 +1735,8 @@
cache.sources = []
cache.builds = []
cache.customfiles = []
+ cache.acceptor = None
+ cache.rejector = None
for pus in puses:
get_property_cache(pus.packageupload).sources.append(pus)
@@ -1754,3 +1776,15 @@
spr_cache.published_archives.append(publication.archive)
for diff in diffs:
get_property_cache(diff.to_source).package_diffs.append(diff)
+
+ if bool(getFeatureFlag('auditor.enabled')):
+ client = AuditorClient()
+ data = client.receive(
+ obj=uploads,
+ operation=('packageupload-accepted', 'packageupload-rejected'))
+ for entry in data:
+ prop_map = {'packageupload-accepted': 'acceptor',
+ 'packageupload-rejected': 'rejector'}
+ operation = prop_map[entry['operation']]
+ setattr(
+ get_property_cache(entry['object']), operation, entry['actor'])
=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py 2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py 2013-06-26 04:56:32 +0000
@@ -28,6 +28,7 @@
from lp.registry.interfaces.series import SeriesStatus
from lp.services.config import config
from lp.services.database.interfaces import IStore
+from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.mail import stub
@@ -57,6 +58,7 @@
TestCaseWithFactory,
)
from lp.testing.layers import (
+ AuditorLayer,
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
@@ -66,7 +68,36 @@
)
-class PackageUploadTestCase(TestCaseWithFactory):
+class PackageUploadTestBase:
+
+ def makeSourcePackageUpload(self, pocket=None, sourcepackagename=None,
+ section_name=None, changes_dict=None,
+ distroseries=None):
+ """Make a useful source package upload for queue tests."""
+ if distroseries is None:
+ distroseries = self.test_publisher.distroseries
+ distroseries.changeslist = "autotest_changes@xxxxxxxxxx"
+ uploader = self.factory.makePerson()
+ key = self.factory.makeGPGKey(owner=uploader)
+ changes = Changes({"Changed-By": uploader.preferredemail.email})
+ if changes_dict is not None:
+ changes.update(changes_dict)
+ upload = self.factory.makePackageUpload(
+ archive=distroseries.main_archive, distroseries=distroseries,
+ pocket=pocket, changes_file_content=changes.dump().encode("UTF-8"),
+ signing_key=key)
+ spr = self.factory.makeSourcePackageRelease(
+ sourcepackagename=sourcepackagename, distroseries=distroseries,
+ component="main", section_name=section_name,
+ changelog_entry="dummy")
+ upload.addSource(spr)
+ spr.addFile(self.factory.makeLibraryFileAlias(
+ filename="%s_%s.dsc" % (spr.name, spr.version)))
+ transaction.commit()
+ return upload, uploader
+
+
+class PackageUploadTestCase(TestCaseWithFactory, PackageUploadTestBase):
layer = LaunchpadZopelessLayer
dbuser = config.uploadqueue.dbuser
@@ -140,30 +171,6 @@
self.assertEqual(current_component, spr.component)
self.assertEqual(new_section, spr.section)
- def makeSourcePackageUpload(self, pocket=None, sourcepackagename=None,
- section_name=None, changes_dict=None):
- """Make a useful source package upload for queue tests."""
- distroseries = self.test_publisher.distroseries
- distroseries.changeslist = "autotest_changes@xxxxxxxxxx"
- uploader = self.factory.makePerson()
- key = self.factory.makeGPGKey(owner=uploader)
- changes = Changes({"Changed-By": uploader.preferredemail.email})
- if changes_dict is not None:
- changes.update(changes_dict)
- upload = self.factory.makePackageUpload(
- archive=distroseries.main_archive, distroseries=distroseries,
- pocket=pocket, changes_file_content=changes.dump().encode("UTF-8"),
- signing_key=key)
- spr = self.factory.makeSourcePackageRelease(
- sourcepackagename=sourcepackagename, distroseries=distroseries,
- component="main", section_name=section_name,
- changelog_entry="dummy")
- upload.addSource(spr)
- spr.addFile(self.factory.makeLibraryFileAlias(
- filename="%s_%s.dsc" % (spr.name, spr.version)))
- transaction.commit()
- return upload, uploader
-
def makeBuildPackageUpload(self):
"""Make a useful build package upload for queue tests."""
distroseries = self.test_publisher.distroseries
@@ -1342,3 +1349,44 @@
with StormStatementRecorder() as recorder:
ws_distroseries.getPackageUploads()
self.assertThat(recorder, HasQueryCount(Equals(32)))
+
+
+class PackageUploadAuditorTestCase(TestCaseWithFactory, PackageUploadTestBase):
+
+ layer = AuditorLayer
+
+ def setUp(self):
+ super(PackageUploadAuditorTestCase, self).setUp()
+ self.test_publisher = SoyuzTestPublisher()
+
+ def test_acceptFromQueue_with_auditor(self):
+ self.test_publisher.prepareBreezyAutotest()
+ person = self.factory.makePerson()
+ upload, uploader = self.makeSourcePackageUpload()
+ fixture = FeatureFixture({'auditor.enabled': 'true'})
+ self.useFixture(fixture)
+ upload.acceptFromQueue(user=person)
+ self.assertEqual(person, upload.acceptor)
+
+ def test_rejectFromQueue_source_with_auditor(self):
+ self.test_publisher.prepareBreezyAutotest()
+ person = self.factory.makePerson()
+ upload, uploader = self.makeSourcePackageUpload()
+ fixture = FeatureFixture({'auditor.enabled': 'true'})
+ self.useFixture(fixture)
+ upload.rejectFromQueue(user=person)
+ self.assertEqual(person, upload.rejector)
+
+ def test_getAll_auditor_preload(self):
+ self.test_publisher.prepareBreezyAutotest()
+ distroseries = self.factory.makeDistroSeries()
+ person = self.factory.makePerson()
+ upload, uploader = self.makeSourcePackageUpload(
+ distroseries=distroseries)
+ with FeatureFixture({'auditor.enabled': 'true'}):
+ upload.rejectFromQueue(user=person)
+ uploads = list(distroseries.getPackageUploads())
+ # Calling .rejector on the PU will return None if it isn't cached due
+ # to the lack of feature flag.
+ self.assertContentEqual([upload], uploads)
+ self.assertEqual(person, uploads[0].rejector)
=== modified file 'lib/lp/testing/layers.py'
--- lib/lp/testing/layers.py 2013-06-20 05:50:00 +0000
+++ lib/lp/testing/layers.py 2013-06-26 04:56:32 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Layers used by Launchpad tests.
@@ -1345,42 +1345,6 @@
disconnect_stores()
-class AuditorLayer(LaunchpadFunctionalLayer):
-
- auditor = AuditorServer()
-
- _is_setup = False
-
- @classmethod
- @profiled
- def setUp(cls):
- cls.auditor.setUp()
- cls.config_fixture.add_section(cls.auditor.service_config)
- cls.appserver_config_fixture.add_section(cls.auditor.service_config)
- cls._is_setup = True
-
- @classmethod
- @profiled
- def tearDown(cls):
- if not cls._is_setup:
- return
- cls.auditor.cleanUp()
- cls._is_setup = False
- # Can't pop the config above, so bail here and let the test runner
- # start a sub-process.
- raise NotImplementedError
-
- @classmethod
- @profiled
- def testSetUp(cls):
- pass
-
- @classmethod
- @profiled
- def testTearDown(cls):
- pass
-
-
class GoogleLaunchpadFunctionalLayer(LaunchpadFunctionalLayer,
GoogleServiceLayer):
"""Provides Google service in addition to LaunchpadFunctionalLayer."""
@@ -1528,6 +1492,42 @@
transaction.abort()
+class AuditorLayer(LaunchpadZopelessLayer):
+
+ auditor = AuditorServer()
+
+ _is_setup = False
+
+ @classmethod
+ @profiled
+ def setUp(cls):
+ cls.auditor.setUp()
+ cls.config_fixture.add_section(cls.auditor.service_config)
+ cls.appserver_config_fixture.add_section(cls.auditor.service_config)
+ cls._is_setup = True
+
+ @classmethod
+ @profiled
+ def tearDown(cls):
+ if not cls._is_setup:
+ return
+ cls.auditor.cleanUp()
+ cls._is_setup = False
+ # Can't pop the config above, so bail here and let the test runner
+ # start a sub-process.
+ raise NotImplementedError
+
+ @classmethod
+ @profiled
+ def testSetUp(cls):
+ pass
+
+ @classmethod
+ @profiled
+ def testTearDown(cls):
+ cls.auditor.resetdb()
+
+
class MockHTTPTask:
class MockHTTPRequestParser:
=== modified file 'versions.cfg'
--- versions.cfg 2013-06-18 01:10:41 +0000
+++ versions.cfg 2013-06-26 04:56:32 +0000
@@ -13,7 +13,7 @@
argparse = 1.2.1
auditor = 0.0.3
auditorclient = 0.0.3
-auditorfixture = 0.0.5
+auditorfixture = 0.0.6
BeautifulSoup = 3.2.1
bson = 0.3.3
bzr = 2.5.1
Follow ups
