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
=== modified file 'lib/lp/bugs/doc/checkwatches.txt'
--- lib/lp/bugs/doc/checkwatches.txt 2009-10-07 22:54:42 +0000
+++ lib/lp/bugs/doc/checkwatches.txt 2009-11-19 14:43:16 +0000
@@ -372,3 +372,100 @@
372 INFO 2 watches left to check on bug tracker 'auto-example.com'372 INFO 2 watches left to check on bug tracker 'auto-example.com'
373 INFO 1 watches left to check on bug tracker 'auto-example.com'373 INFO 1 watches left to check on bug tracker 'auto-example.com'
374 INFO 0 watches left to check on bug tracker 'auto-example.com'374 INFO 0 watches left to check on bug tracker 'auto-example.com'
375
376
377Comment syncing for duplicate bugs
378----------------------------------
379
380checkwatches won't try to sync comments for bugs which are duplicates of
381other bugs in Launchpad. This is to avoid spamming both the upstream bug
382tracker and Launchpad users with comments from the duplicate bugs. It
383also side-steps the issue of Launchpad syncing with itself via an
384external bug tracker (bug 484712).
385
386We'll create a non-functioning ExternalBugtracker to demonstrate this.
387
388 >>> import pytz
389 >>> from datetime import datetime
390 >>> from zope.interface import implements
391 >>> from lp.bugs.interfaces.bugtask import (
392 ... BugTaskStatus, BugTaskImportance)
393 >>> from lp.bugs.interfaces.externalbugtracker import (
394 ... ISupportsCommentImport, ISupportsCommentPushing,
395 ... ISupportsBackLinking)
396 >>> from lp.bugs.externalbugtracker.base import ExternalBugTracker
397
398 >>> nowish = datetime.now(pytz.timezone('UTC'))
399 >>> class UselessExternalBugTracker(ExternalBugTracker):
400 ...
401 ... implements(
402 ... ISupportsBackLinking, ISupportsCommentImport,
403 ... ISupportsCommentPushing)
404 ...
405 ... def initializeRemoteBugDB(self, bug_ids):
406 ... # This just exists to stop errors from being raised.
407 ... pass
408 ...
409 ... def getCurrentDBTime(self):
410 ... return nowish
411 ...
412 ... def getRemoteStatus(self, id):
413 ... return 'NEW'
414 ...
415 ... def convertRemoteStatus(self, status):
416 ... return BugTaskStatus.NEW
417 ...
418 ... def getRemoteImportance(self, id):
419 ... return 'NONE'
420 ...
421 ... def convertRemoteImportance(self, importance):
422 ... return BugTaskImportance.UNKNOWN
423 ...
424 ... def getCommentIds(self, bug_watch):
425 ... print "getCommentIds() called"
426 ... return []
427 ...
428 ... def fetchComments(self, bug_watch, comment_ids):
429 ... return []
430 ...
431 ... def addRemoteComment(self, bug_watch, comment):
432 ... print "addRemoteComment() called."
433 ... return 0
434 ...
435 ... def getLaunchpadBugId(self, bug_id):
436 ... print "getLaunchpadBugId() called"
437 ... return None
438 ...
439 ... def setLaunchpadBugId(self, bug_id, lp_bug_id):
440 ... print "setLaunchpadBugId() called"
441
442We'll generate a bug watch with which to test this.
443
444 >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
445 >>> login('foo.bar@canonical.com')
446 >>> bug_tracker = factory.makeBugTracker()
447 >>> bug_watch = factory.makeBugWatch(bugtracker=bug_tracker)
448 >>> transaction.commit()
449 >>> LaunchpadZopelessLayer.switchDbUser(config.checkwatches.dbuser)
450
451If we pass our UselessExternalBugTracker and the bug watch we just
452generated to updateBugWatches we can see that its comments will be
453synced and it will be linked to the remote bug.
454
455 >>> updater = BugWatchUpdater(transaction)
456 >>> remote_system = UselessExternalBugTracker('http://example.com')
457
458 >>> updater.updateBugWatches(remote_system, [bug_watch], now=nowish)
459 getCommentIds() called
460 getLaunchpadBugId() called
461 setLaunchpadBugId() called
462
463If we mark the bug to which our bug watch is attached as a duplicate of
464another bug, comments won't be synced and the bug won't be linked back
465to the remote bug.
466
467 >>> bug_15 = getUtility(IBugSet).get(15)
468 >>> bug_watch.bug.duplicateof = bug_15
469 >>> transaction.commit()
470
471 >>> updater.updateBugWatches(remote_system, [bug_watch], now=nowish)
375472
=== modified file 'lib/lp/bugs/scripts/checkwatches.py'
--- lib/lp/bugs/scripts/checkwatches.py 2009-10-06 14:03:03 +0000
+++ lib/lp/bugs/scripts/checkwatches.py 2009-11-19 14:43:16 +0000
@@ -777,12 +777,16 @@
777 if new_malone_importance is not None:777 if new_malone_importance is not None:
778 bug_watch.updateImportance(new_remote_importance,778 bug_watch.updateImportance(new_remote_importance,
779 new_malone_importance)779 new_malone_importance)
780 if can_import_comments:780 if bug_watch.bug.duplicateof is None:
781 self.importBugComments(remotesystem, bug_watch)781 # Only sync comments and backlink if the local
782 if can_push_comments:782 # bug isn't a duplicate. This helps us to avoid
783 self.pushBugComments(remotesystem, bug_watch)783 # spamming upstream.
784 if ISupportsBackLinking.providedBy(remotesystem):784 if can_import_comments:
785 self.linkLaunchpadBug(remotesystem, bug_watch)785 self.importBugComments(remotesystem, bug_watch)
786 if can_push_comments:
787 self.pushBugComments(remotesystem, bug_watch)
788 if ISupportsBackLinking.providedBy(remotesystem):
789 self.linkLaunchpadBug(remotesystem, bug_watch)
786790
787 except (KeyboardInterrupt, SystemExit):791 except (KeyboardInterrupt, SystemExit):
788 # We should never catch KeyboardInterrupt or SystemExit.792 # We should never catch KeyboardInterrupt or SystemExit.