launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/drop-populate-specification-aag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/drop-populate-specification-aag/+merge/143627

Destroy PopulateSpecificationAccessPolicy, it has completed on qastaging, staging, dogfood and production. I saw no point against running it against sampledata, there are no private blueprints in either of sampledata's databases.
-- 
https://code.launchpad.net/~stevenk/launchpad/drop-populate-specification-aag/+merge/143627
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/drop-populate-specification-aag into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2013-01-16 05:19:03 +0000
+++ lib/lp/scripts/garbo.py	2013-01-17 01:04:22 +0000
@@ -48,7 +48,6 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.model.answercontact import AnswerContact
-from lp.blueprints.model.specification import Specification
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
@@ -105,7 +104,6 @@
 from lp.services.librarian.model import TimeLimitedToken
 from lp.services.log.logger import PrefixFilter
 from lp.services.looptuner import TunableLoop
-from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.oauth.model import OAuthNonce
 from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
 from lp.services.propertycache import cachedproperty
@@ -1337,38 +1335,6 @@
         transaction.commit()
 
 
-class PopulateSpecificationAccessPolicy(TunableLoop):
-
-    maximum_chunk_size = 5000
-
-    def __init__(self, log, abort_time=None):
-        super(PopulateSpecificationAccessPolicy, self).__init__(
-            log, abort_time)
-        self.memcache_key = '%s:spec-populate-ap' % config.instance_name
-        watermark = getUtility(IMemcacheClient).get(self.memcache_key)
-        self.start_at = watermark or 0
-
-    def findSpecifications(self):
-        return IMasterStore(Specification).find(
-            Specification,
-            Specification.id >= self.start_at).order_by(Specification.id)
-
-    def isDone(self):
-        return self.findSpecifications().is_empty()
-
-    def __call__(self, chunk_size):
-        spec_ids = [spec.id for spec in self.findSpecifications()[:chunk_size]]
-        IMasterStore(Specification).execute(
-            'SELECT specification_denorm_access(id) FROM specification '
-            'WHERE id IN %s' % sqlvalues(spec_ids))
-        self.start_at = spec_ids[-1] + 1
-        result = getUtility(IMemcacheClient).set(
-            self.memcache_key, self.start_at)
-        if not result:
-            self.log.warning('Failed to set start_at in memcache.')
-        transaction.commit()
-
-
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1624,7 +1590,6 @@
         UnusedSessionPruner,
         DuplicateSessionPruner,
         BugHeatUpdater,
-        PopulateSpecificationAccessPolicy,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2013-01-16 05:46:20 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2013-01-17 01:04:22 +0000
@@ -38,7 +38,6 @@
 
 from lp.answers.model.answercontact import AnswerContact
 from lp.app.enums import InformationType
-from lp.blueprints.model.specification import Specification
 from lp.bugs.model.bugnotification import (
     BugNotification,
     BugNotificationRecipient,
@@ -58,12 +57,8 @@
 from lp.registry.enums import (
     BranchSharingPolicy,
     BugSharingPolicy,
-    SpecificationSharingPolicy,
-    )
-from lp.registry.interfaces.accesspolicy import (
-    IAccessPolicyArtifactSource,
-    IAccessPolicySource,
-    )
+    )
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.model.commercialsubscription import CommercialSubscription
@@ -102,7 +97,6 @@
 from lp.services.job.model.job import Job
 from lp.services.librarian.model import TimeLimitedToken
 from lp.services.log.logger import NullHandler
-from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.messages.model.message import Message
 from lp.services.oauth.model import (
     OAuthAccessToken,
@@ -127,10 +121,7 @@
     TestCase,
     TestCaseWithFactory,
     )
-from lp.testing.dbuser import (
-    dbuser,
-    switch_dbuser,
-    )
+from lp.testing.dbuser import switch_dbuser
 from lp.testing.layers import (
     DatabaseLayer,
     LaunchpadScriptLayer,
@@ -1282,42 +1273,6 @@
             'PopulateLatestPersonSourcePackageReleaseCache')
         self.assertEqual(spph_2.id, job_data['last_spph_id'])
 
-    def test_PopulateSpecificationAccessPolicy(self):
-        # Specifications without a access_policy have one set by the job.
-        with dbuser('testadmin'):
-            specification = self.factory.makeSpecification()
-            product = removeSecurityProxy(specification.product)
-            product.specification_sharing_policy = (
-                SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY)
-            self.factory.makeAccessPolicy(pillar=product)
-
-        def get_access_policy():
-            return IMasterStore(Specification).execute(
-                'SELECT access_policy FROM specification WHERE id = ?',
-                (specification.id,)).get_one()[0]
-
-        # The specification is public, so running the garbo job will have no
-        # effect.
-        self.runHourly()
-        self.assertIs(None, get_access_policy())
-
-        with dbuser('testadmin'):
-            specification.transitionToInformationType(
-                InformationType.PROPRIETARY, specification.owner)
-            IMasterStore(Specification).execute(
-                'UPDATE specification SET access_policy = NULL WHERE id = ?',
-                (specification.id,))
-            getUtility(IMemcacheClient).set(
-                '%s:spec-populate-ap' % config.instance_name, 0)
-
-        access_artifact = self.factory.makeAccessArtifact(
-            concrete=specification)
-        apas = getUtility(IAccessPolicyArtifactSource).findByArtifact(
-            (access_artifact,))
-        # Now it will be set.
-        self.runHourly()
-        self.assertEqual(apas[0].policy.id, get_access_policy())
-
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer