launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/unnecessary-remove-subscriptions-job-1009356 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/unnecessary-remove-subscriptions-job-1009356/+merge/122605

A simple change to the sharing service deletePillarGrantee method - only create a RemoveArtifactSubscriptionsJob if any existing access policy or access artifact grants are revoked. This avoids creating unnecessary jobs for noops.
-- 
https://code.launchpad.net/~wallyworld/launchpad/unnecessary-remove-subscriptions-job-1009356/+merge/122605
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/unnecessary-remove-subscriptions-job-1009356 into lp:launchpad.

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-09-03 11:22:25 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-09-04 01:09:22 +0000
@@ -462,25 +462,26 @@
         # First delete any access policy grants.
         policy_grant_source = getUtility(IAccessPolicyGrantSource)
         policy_grants = [(policy, grantee) for policy in pillar_policies]
-        grants = [
+        grants_to_revoke = [
             (grant.policy, grant.grantee)
             for grant in policy_grant_source.find(policy_grants)]
-        if len(grants) > 0:
-            policy_grant_source.revoke(grants)
+        if len(grants_to_revoke) > 0:
+            policy_grant_source.revoke(grants_to_revoke)
 
         # Second delete any access artifact grants.
         ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
-        to_delete = list(ap_grant_flat.findArtifactsByGrantee(
+        artifacts_to_revoke = list(ap_grant_flat.findArtifactsByGrantee(
             grantee, pillar_policies))
-        if len(to_delete) > 0:
+        if len(artifacts_to_revoke) > 0:
             getUtility(IAccessArtifactGrantSource).revokeByArtifact(
-                to_delete, [grantee])
+                artifacts_to_revoke, [grantee])
 
         # Create a job to remove subscriptions for artifacts the grantee can no
         # longer see.
-        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
-            user, artifacts=None, grantee=grantee, pillar=pillar,
-            information_types=information_types)
+        if grants_to_revoke or artifacts_to_revoke:
+            getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+                user, artifacts=None, grantee=grantee, pillar=pillar,
+                information_types=information_types)
 
         grant_counts = list(self.getAccessPolicyGrantCounts(pillar))
         invisible_types = [