Code review comment for lp:~allenap/launchpad/dynamic-batch-size-bug-546085

Revision history for this message
Gavin Panella (allenap) wrote :

Post-review diff, constructed from revisions 9177, 9178 and 9180.

1--- lib/lp/bugs/externalbugtracker/__init__.py 2009-06-25 00:40:31 +0000
2+++ lib/lp/bugs/externalbugtracker/__init__.py 2010-04-07 14:25:18 +0000
3@@ -7,7 +7,7 @@
4
5 __metaclass__ = type
6 __all__ = [
7- 'get_external_bugtracker',
8+ 'BATCH_SIZE_UNLIMITED',
9 'BugNotFound',
10 'BugTrackerConnectError',
11 'BugWatchUpdateError',
12@@ -30,6 +30,7 @@
13 'UnparseableBugData',
14 'UnparseableBugTrackerVersion',
15 'UnsupportedBugTrackerVersion',
16+ 'get_external_bugtracker',
17 ]
18
19 from lp.bugs.externalbugtracker.base import *
20@@ -41,6 +42,8 @@
21 from lp.bugs.externalbugtracker.rt import *
22 from lp.bugs.externalbugtracker.trac import *
23 from lp.bugs.interfaces.bugtracker import BugTrackerType
24+
25+
26 BUG_TRACKER_CLASSES = {
27 BugTrackerType.BUGZILLA: Bugzilla,
28 BugTrackerType.DEBBUGS: DebBugs,
29--- lib/lp/bugs/externalbugtracker/base.py 2010-03-30 12:19:11 +0000
30+++ lib/lp/bugs/externalbugtracker/base.py 2010-04-07 14:25:18 +0000
31@@ -5,6 +5,7 @@
32
33 __metaclass__ = type
34 __all__ = [
35+ 'BATCH_SIZE_UNLIMITED',
36 'BugNotFound',
37 'BugTrackerAuthenticationError',
38 'BugTrackerConnectError',
39@@ -39,6 +40,9 @@
40 # The user agent we send in our requests
41 LP_USER_AGENT = "Launchpad Bugscraper/0.2 (https://bugs.launchpad.net/)"
42
43+# To signify that all bug watches should be checked in a single run.
44+BATCH_SIZE_UNLIMITED = 0
45+
46
47 #
48 # Errors.
49--- lib/lp/bugs/externalbugtracker/debbugs.py 2010-03-26 17:32:38 +0000
50+++ lib/lp/bugs/externalbugtracker/debbugs.py 2010-04-07 14:25:18 +0000
51@@ -22,18 +22,19 @@
52
53 from canonical.config import config
54 from canonical.database.sqlbase import commit
55+from canonical.launchpad.interfaces.message import IMessageSet
56+from canonical.launchpad.mail import simple_sendmail
57+from canonical.launchpad.webapp import urlsplit
58+
59 from lp.bugs.externalbugtracker import (
60- BugNotFound, BugTrackerConnectError, ExternalBugTracker,
61- InvalidBugId, UnknownRemoteStatusError)
62-from canonical.launchpad.interfaces.message import IMessageSet
63+ BATCH_SIZE_UNLIMITED, BugNotFound, BugTrackerConnectError,
64+ ExternalBugTracker, InvalidBugId, UnknownRemoteStatusError)
65+from lp.bugs.externalbugtracker.isolation import ensure_no_transaction
66 from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
67 from lp.bugs.interfaces.externalbugtracker import (
68 ISupportsBugImport, ISupportsCommentImport, ISupportsCommentPushing,
69 UNKNOWN_REMOTE_IMPORTANCE)
70-from canonical.launchpad.mail import simple_sendmail
71 from lp.bugs.scripts import debbugs
72-from canonical.launchpad.webapp import urlsplit
73-from lp.bugs.externalbugtracker.isolation import ensure_no_transaction
74
75
76 debbugsstatusmap = {'open': BugTaskStatus.NEW,
77@@ -61,7 +62,7 @@
78 # Because we keep a local copy of debbugs, we remove the batch_size
79 # limit so that all debbugs watches that need checking will be
80 # checked each time checkwatches runs.
81- batch_size = 0
82+ batch_size = BATCH_SIZE_UNLIMITED
83
84 def __init__(self, baseurl, db_location=None):
85 super(DebBugs, self).__init__(baseurl)
86--- lib/lp/bugs/scripts/checkwatches/updater.py 2010-03-31 10:51:17 +0000
87+++ lib/lp/bugs/scripts/checkwatches/updater.py 2010-04-07 14:25:18 +0000
88@@ -61,16 +61,15 @@
89
90 from lp.bugs import externalbugtracker
91 from lp.bugs.externalbugtracker import (
92- BugNotFound, BugTrackerConnectError, BugWatchUpdateError,
93- BugWatchUpdateWarning, InvalidBugId, PrivateRemoteBug,
94- UnknownBugTrackerTypeError, UnknownRemoteStatusError, UnparseableBugData,
95- UnparseableBugTrackerVersion, UnsupportedBugTrackerVersion)
96-from lp.bugs.externalbugtracker.bugzilla import (
97- BugzillaAPI)
98+ BATCH_SIZE_UNLIMITED, BugNotFound, BugTrackerConnectError,
99+ BugWatchUpdateError, BugWatchUpdateWarning, InvalidBugId,
100+ PrivateRemoteBug, UnknownBugTrackerTypeError, UnknownRemoteStatusError,
101+ UnparseableBugData, UnparseableBugTrackerVersion,
102+ UnsupportedBugTrackerVersion)
103+from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI
104 from lp.bugs.externalbugtracker.isolation import check_no_transaction
105 from lp.bugs.interfaces.bug import IBugSet
106-from lp.bugs.interfaces.externalbugtracker import (
107- ISupportsBackLinking)
108+from lp.bugs.interfaces.externalbugtracker import ISupportsBackLinking
109 from lp.services.limitedlist import LimitedList
110 from lp.services.scripts.base import LaunchpadCronScript
111
112@@ -250,17 +249,17 @@
113 def suggest_batch_size(remote_system, num_watches):
114 """Suggest a value for batch_size if it's not set.
115
116- Givend the number of bug watches for an `remote_system`, this sets
117- a suggested batch size on it. If `remote_system` already has a
118- batch size set, this does not override it.
119+ Given the number of bug watches for a `remote_system`, this sets a
120+ suggested batch size on it. If `remote_system` already has a batch
121+ size set, this does not override it.
122
123 :param remote_system: An `ExternalBugTracker`.
124 :param num_watches: The number of watches for `remote_system`.
125 """
126 if remote_system.batch_size is None:
127 remote_system.batch_size = max(
128- SUGGESTED_BATCH_SIZE_MIN, int(
129- SUGGESTED_BATCH_SIZE_PROPORTION * num_watches))
130+ SUGGESTED_BATCH_SIZE_MIN,
131+ int(SUGGESTED_BATCH_SIZE_PROPORTION * num_watches))
132
133
134 class BugWatchUpdater(object):
135@@ -402,7 +401,12 @@
136 `BaseScheduler`. If no scheduler is given, `SerialScheduler`
137 will be used, which simply runs the jobs in order.
138 """
139- self.log.debug("Using a global batch size of %s" % batch_size)
140+ if batch_size is None:
141+ self.log.debug("No global batch size specified.")
142+ elif batch_size == BATCH_SIZE_UNLIMITED:
143+ self.log.debug("Using an unlimited global batch size.")
144+ else:
145+ self.log.debug("Using a global batch size of %s" % batch_size)
146
147 # Default to using the very simple SerialScheduler.
148 if scheduler is None:
149@@ -681,11 +685,6 @@
150 # by the ExternalBugTracker.
151 batch_size = remotesystem.batch_size
152
153- if batch_size == 0:
154- # A batch_size of 0 means that there's no batch size limit
155- # for this bug tracker.
156- batch_size = None
157-
158 with self.transaction:
159 old_bug_watches = set(
160 bug_watch for bug_watch in bug_watches
161@@ -718,7 +717,7 @@
162 # are actually some bugs that we're interested in so as to
163 # avoid unnecessary network traffic.
164 if server_time is not None and len(remote_old_ids) > 0:
165- if batch_size is None:
166+ if batch_size == BATCH_SIZE_UNLIMITED:
167 remote_old_ids_to_check = (
168 remotesystem.getModifiedRemoteBugs(
169 remote_old_ids, oldest_lastchecked))
170@@ -746,7 +745,7 @@
171 remote_ids_to_check = chain(
172 remote_ids_with_comments, remote_new_ids, remote_old_ids_to_check)
173
174- if batch_size is not None:
175+ if batch_size != BATCH_SIZE_UNLIMITED:
176 # Some remote bug IDs may appear in more than one list so
177 # we must filter the list before slicing.
178 remote_ids_to_check = islice(
179--- lib/lp/bugs/doc/checkwatches.txt 2010-03-30 17:25:52 +0000
180+++ lib/lp/bugs/doc/checkwatches.txt 2010-04-07 16:08:16 +0000
181@@ -45,7 +45,7 @@
182
183 >>> print err
184 INFO creating lockfile
185- DEBUG Using a global batch size of None
186+ DEBUG No global batch size specified.
187 DEBUG Skipping updating Ubuntu Bugzilla watches.
188 DEBUG No watches to update on http://bugs.debian.org
189 DEBUG No watches to update on mailto:bugs@example.com
190@@ -274,7 +274,7 @@
191 ... updater.log = original_log
192 ... externalbugtracker.Roundup.batch_size = batch_size
193 ... urllib2.urlopen = urlopen
194- DEBUG Using a global batch size of None
195+ DEBUG No global batch size specified.
196 INFO Updating 5 watches for 5 bugs on http://bugs.example.com
197 ERROR Connection timed out when updating ... (OOPS-...)
198
199@@ -412,7 +412,8 @@
200 >>> from lp.bugs.interfaces.externalbugtracker import (
201 ... ISupportsCommentImport, ISupportsCommentPushing,
202 ... ISupportsBackLinking)
203- >>> from lp.bugs.externalbugtracker.base import ExternalBugTracker
204+ >>> from lp.bugs.externalbugtracker.base import (
205+ ... BATCH_SIZE_UNLIMITED, ExternalBugTracker)
206
207 >>> nowish = datetime.now(utc)
208 >>> class UselessExternalBugTracker(ExternalBugTracker):
209@@ -421,6 +422,8 @@
210 ... ISupportsBackLinking, ISupportsCommentImport,
211 ... ISupportsCommentPushing)
212 ...
213+ ... batch_size = BATCH_SIZE_UNLIMITED
214+ ...
215 ... def initializeRemoteBugDB(self, bug_ids):
216 ... # This just exists to stop errors from being raised.
217 ... pass
218--- lib/lp/bugs/doc/externalbugtracker-debbugs.txt 2010-03-26 13:48:53 +0000
219+++ lib/lp/bugs/doc/externalbugtracker-debbugs.txt 2010-04-07 16:08:16 +0000
220@@ -14,7 +14,7 @@
221 You can specify the db_location explicitly:
222
223 >>> from lp.bugs.externalbugtracker import (
224- ... DebBugs)
225+ ... BATCH_SIZE_UNLIMITED, DebBugs)
226 >>> from canonical.testing import LaunchpadZopelessLayer
227 >>> txn = LaunchpadZopelessLayer.txn
228 >>> external_debbugs = DebBugs(
229@@ -42,8 +42,8 @@
230 worry about batching bug watch updates for performance reasons, so
231 DebBugs instances don't have a batch_size limit.
232
233- >>> print external_debbugs.batch_size
234- None
235+ >>> external_debbugs.batch_size == BATCH_SIZE_UNLIMITED
236+ True
237
238
239 == Retrieving bug status from the debbugs database ==
240--- lib/lp/bugs/tests/externalbugtracker.py 2009-12-23 12:14:59 +0000
241+++ lib/lp/bugs/tests/externalbugtracker.py 2010-04-07 16:08:16 +0000
242@@ -24,8 +24,8 @@
243 from canonical.config import config
244 from canonical.database.sqlbase import commit, ZopelessTransactionManager
245 from lp.bugs.externalbugtracker import (
246- BugNotFound, BugTrackerConnectError, Bugzilla, DebBugs,
247- ExternalBugTracker, Mantis, RequestTracker, Roundup, SourceForge,
248+ BATCH_SIZE_UNLIMITED, BugNotFound, BugTrackerConnectError, Bugzilla,
249+ DebBugs, ExternalBugTracker, Mantis, RequestTracker, Roundup, SourceForge,
250 Trac)
251 from lp.bugs.externalbugtracker.trac import (
252 FAULT_TICKET_NOT_FOUND, LP_PLUGIN_BUG_IDS_ONLY, LP_PLUGIN_FULL,
253@@ -152,6 +152,8 @@
254 implementation, though it doesn't actually do anything.
255 """
256
257+ batch_size = BATCH_SIZE_UNLIMITED
258+
259 def __init__(self, baseurl='http://example.com/'):
260 super(TestExternalBugTracker, self).__init__(baseurl)
261

« Back to merge proposal