Launchpad itself

Merge lp:~gmb/launchpad/no-sync-from-dupes-bug-484609 into lp:launchpad

  1. no-sync-from-dupes-bug-484609
  2. Merge into devel
Proposed by Graham Binns
Status: Merged
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/no-sync-from-dupes-bug-484609
Merge into: lp:launchpad
Diff against target: 131 lines (+107/-6)
2 files modified
lib/lp/bugs/doc/checkwatches.txt (+97/-0)
lib/lp/bugs/scripts/checkwatches.py (+10/-6)
To merge this branch: bzr merge lp:~gmb/launchpad/no-sync-from-dupes-bug-484609
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+15036@code.launchpad.net

Commit message

Comments will be synced and bugs back-linked with a remote bug only if the Launchpad bug that refers to it isn't a duplicate.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes 484609 by making checkwatches only call the comment syncing and back-linking code for a remote bug if the local bug isn't a duplicate.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Thanks Graham - a lot of people will be thanking you for fixing this!

It looks like it would have been the perfect opportunity to try mocker to create your test double, but I can understand that you want to get this landed ASAP.

-Michael

<gmb> noodles775: Can I get a review of https://code.edge.launchpad.net/~gmb/launchpad/no-sync-from-dupes-bug-484609/+merge/15036 please?
<noodles775> gmb: sure.
<gmb> Ta
* noodles775 has changed the topic to: on call: noodles || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
<gmb> noodles775: I've just spotted some errors circa lines 84 and 85 of the diff - references to multiple bug watches when there's only one. I'll fix it.
<noodles775> gmb: ok, it's just comments in your doctest right? So I'll still start it now.
<gmb> noodles775: Right.
<noodles775> gmb: s/bug_watche/bugwatch on line 80 of diff?
<gmb> noodles775: Yes.
<noodles775> gmb: also, the import of transaction on line 87 is unnecessary.
<gmb> noodles775: Okidoke. I'll remove it.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/doc/checkwatches.txt'
2--- lib/lp/bugs/doc/checkwatches.txt 2009-10-07 22:54:42 +0000
3+++ lib/lp/bugs/doc/checkwatches.txt 2009-11-19 14:43:16 +0000
4@@ -372,3 +372,100 @@
5 INFO 2 watches left to check on bug tracker 'auto-example.com'
6 INFO 1 watches left to check on bug tracker 'auto-example.com'
7 INFO 0 watches left to check on bug tracker 'auto-example.com'
8+
9+
10+Comment syncing for duplicate bugs
11+----------------------------------
12+
13+checkwatches won't try to sync comments for bugs which are duplicates of
14+other bugs in Launchpad. This is to avoid spamming both the upstream bug
15+tracker and Launchpad users with comments from the duplicate bugs. It
16+also side-steps the issue of Launchpad syncing with itself via an
17+external bug tracker (bug 484712).
18+
19+We'll create a non-functioning ExternalBugtracker to demonstrate this.
20+
21+ >>> import pytz
22+ >>> from datetime import datetime
23+ >>> from zope.interface import implements
24+ >>> from lp.bugs.interfaces.bugtask import (
25+ ... BugTaskStatus, BugTaskImportance)
26+ >>> from lp.bugs.interfaces.externalbugtracker import (
27+ ... ISupportsCommentImport, ISupportsCommentPushing,
28+ ... ISupportsBackLinking)
29+ >>> from lp.bugs.externalbugtracker.base import ExternalBugTracker
30+
31+ >>> nowish = datetime.now(pytz.timezone('UTC'))
32+ >>> class UselessExternalBugTracker(ExternalBugTracker):
33+ ...
34+ ... implements(
35+ ... ISupportsBackLinking, ISupportsCommentImport,
36+ ... ISupportsCommentPushing)
37+ ...
38+ ... def initializeRemoteBugDB(self, bug_ids):
39+ ... # This just exists to stop errors from being raised.
40+ ... pass
41+ ...
42+ ... def getCurrentDBTime(self):
43+ ... return nowish
44+ ...
45+ ... def getRemoteStatus(self, id):
46+ ... return 'NEW'
47+ ...
48+ ... def convertRemoteStatus(self, status):
49+ ... return BugTaskStatus.NEW
50+ ...
51+ ... def getRemoteImportance(self, id):
52+ ... return 'NONE'
53+ ...
54+ ... def convertRemoteImportance(self, importance):
55+ ... return BugTaskImportance.UNKNOWN
56+ ...
57+ ... def getCommentIds(self, bug_watch):
58+ ... print "getCommentIds() called"
59+ ... return []
60+ ...
61+ ... def fetchComments(self, bug_watch, comment_ids):
62+ ... return []
63+ ...
64+ ... def addRemoteComment(self, bug_watch, comment):
65+ ... print "addRemoteComment() called."
66+ ... return 0
67+ ...
68+ ... def getLaunchpadBugId(self, bug_id):
69+ ... print "getLaunchpadBugId() called"
70+ ... return None
71+ ...
72+ ... def setLaunchpadBugId(self, bug_id, lp_bug_id):
73+ ... print "setLaunchpadBugId() called"
74+
75+We'll generate a bug watch with which to test this.
76+
77+ >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
78+ >>> login('foo.bar@canonical.com')
79+ >>> bug_tracker = factory.makeBugTracker()
80+ >>> bug_watch = factory.makeBugWatch(bugtracker=bug_tracker)
81+ >>> transaction.commit()
82+ >>> LaunchpadZopelessLayer.switchDbUser(config.checkwatches.dbuser)
83+
84+If we pass our UselessExternalBugTracker and the bug watch we just
85+generated to updateBugWatches we can see that its comments will be
86+synced and it will be linked to the remote bug.
87+
88+ >>> updater = BugWatchUpdater(transaction)
89+ >>> remote_system = UselessExternalBugTracker('http://example.com')
90+
91+ >>> updater.updateBugWatches(remote_system, [bug_watch], now=nowish)
92+ getCommentIds() called
93+ getLaunchpadBugId() called
94+ setLaunchpadBugId() called
95+
96+If we mark the bug to which our bug watch is attached as a duplicate of
97+another bug, comments won't be synced and the bug won't be linked back
98+to the remote bug.
99+
100+ >>> bug_15 = getUtility(IBugSet).get(15)
101+ >>> bug_watch.bug.duplicateof = bug_15
102+ >>> transaction.commit()
103+
104+ >>> updater.updateBugWatches(remote_system, [bug_watch], now=nowish)
105
106=== modified file 'lib/lp/bugs/scripts/checkwatches.py'
107--- lib/lp/bugs/scripts/checkwatches.py 2009-10-06 14:03:03 +0000
108+++ lib/lp/bugs/scripts/checkwatches.py 2009-11-19 14:43:16 +0000
109@@ -777,12 +777,16 @@
110 if new_malone_importance is not None:
111 bug_watch.updateImportance(new_remote_importance,
112 new_malone_importance)
113- if can_import_comments:
114- self.importBugComments(remotesystem, bug_watch)
115- if can_push_comments:
116- self.pushBugComments(remotesystem, bug_watch)
117- if ISupportsBackLinking.providedBy(remotesystem):
118- self.linkLaunchpadBug(remotesystem, bug_watch)
119+ if bug_watch.bug.duplicateof is None:
120+ # Only sync comments and backlink if the local
121+ # bug isn't a duplicate. This helps us to avoid
122+ # spamming upstream.
123+ if can_import_comments:
124+ self.importBugComments(remotesystem, bug_watch)
125+ if can_push_comments:
126+ self.pushBugComments(remotesystem, bug_watch)
127+ if ISupportsBackLinking.providedBy(remotesystem):
128+ self.linkLaunchpadBug(remotesystem, bug_watch)
129
130 except (KeyboardInterrupt, SystemExit):
131 # We should never catch KeyboardInterrupt or SystemExit.