Merge lp:~gmb/launchpad/cw-refactor-add-rbu-bug-568881 into lp:launchpad/db-devel
- cw-refactor-add-rbu-bug-568881
- Merge into db-devel
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 9337 |
Proposed branch: | lp:~gmb/launchpad/cw-refactor-add-rbu-bug-568881 |
Merge into: | lp:launchpad/db-devel |
Diff against target: |
905 lines (+433/-275) 8 files modified
lib/lp/bugs/doc/externalbugtracker.txt (+14/-10) lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py (+3/-4) lib/lp/bugs/scripts/checkwatches/core.py (+14/-225) lib/lp/bugs/scripts/checkwatches/remotebugupdater.py (+218/-0) lib/lp/bugs/scripts/checkwatches/tests/test_core.py (+30/-19) lib/lp/bugs/scripts/checkwatches/tests/test_remotebugupdater.py (+59/-0) lib/lp/bugs/scripts/checkwatches/utilities.py (+61/-0) lib/lp/bugs/scripts/tests/test_bugimport.py (+34/-17) |
To merge this branch: | bzr merge lp:~gmb/launchpad/cw-refactor-add-rbu-bug-568881 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | code | Approve | |
Canonical Launchpad Engineering | code | Pending | |
Review via email: mp+24404@code.launchpad.net |
Commit message
The remote bug-specific part of CheckwatchesMaster has been moved into a RemoteBugUpdater class.
Description of the change
This branch is the second in the mad-checkwatche
It moves the remote-bug-specific part of CheckwatchesMaster into a
RemoteBugUpdater class.
The majority of changes here are simple moves, though there are some
alterations to tests and callsites to make sure that everything works
properly.
Gavin Panella (allenap) : | # |
Graham Binns (gmb) wrote : | # |
On Thu, Apr 29, 2010 at 12:52:29PM -0000, Gavin Panella wrote:
> Review: Approve
> Hi Graham,
>
> This mad refactorage is looking good :) I have a few comments but
> nothing major, so r=me.
>
> Gavin.
>
>
> > @@ -168,6 +126,17 @@
> > else:
> > self._syncable_
> >
> > + def _makeRemoteBugU
> > + bug_watch_ids, unmodified_
> > + """Create and return a `RemoteBugUpdater` instance.
> > +
> > + This method exists purely for the sake of being able to
> > + overrride it in tests.
>
> Have you been listening to Christina Aguilera recently? s/rrr/rr/
What's really sad is that you're confident enough about your knowledge
of her album names to make that gag. Shame on you.
> Another way to do this with less boiler-plate is to have a
> remote_
> _makeRemoteBugU
Right. After discussion on IRC I've done this.
> > + @commit_before
> > + def updateRemoteBug
> > + can_push_
>
> The can_* arguments don't change for the life of the RemoteBugUpdater,
> so I think it makes sense to set them in __init__(). This could be
> done by just passing server_time in.
Agreed. Done.
> > + # Avoid circular imports
>
> Already avoided.
>
Oops, fixed.
> > + with self.transaction:
> > + bug_watches = self._getBugWat
> > + # If there aren't any bug watches for this remote bug,
> > + # just log a warning and carry on.
> > + if len(bug_watches) == 0:
> > + self.warning(
> > + "Spurious remote bug ID: No watches found for "
> > + "remote bug %s on %s" % (
> > + self.remote_bug, self.external_
> > + return
> > + # Mark them all as checked.
> > + for bug_watch in bug_watches:
> > + bug_watch.
> > + bug_watch.
> > + # Next if this one is definitely unmodified.
>
> s/Next/Return/
>
Fixed.
> > + if self.remote_bug in self.unmodified
> > + return
> > + # Save the remote bug URL for error reporting.
> > + remote_bug_url = bug_watches[0].url
> > + # Save the list of local bug IDs for error reporting.
> > + local_ids = ", ".join(
> > + str(bug_id) for bug_id in sorted(
> > + watch.bug.id for watch in bug_watches))
> > +
> > + try:
> > + new_remote_status = None
> > + new_malone_status = None
> > + new_remote_
> > + new_malone_
> > + error = None
> > + oops_id = None
> > +
> > + # XXX: 2007-10-17 Graham Binns
> > + # This nested set of try:excepts isn't really
> > + ...
Preview Diff
1 | === modified file 'lib/lp/bugs/doc/externalbugtracker.txt' |
2 | --- lib/lp/bugs/doc/externalbugtracker.txt 2010-04-23 11:19:49 +0000 |
3 | +++ lib/lp/bugs/doc/externalbugtracker.txt 2010-05-05 10:09:40 +0000 |
4 | @@ -391,12 +391,15 @@ |
5 | >>> from zope.interface import implements |
6 | >>> from canonical.launchpad.interfaces import ISupportsCommentImport |
7 | >>> class CommentImportExternalBugTracker(TimeUnknownExternalBugTracker): |
8 | + ... baseurl = 'http://whatever.com' |
9 | ... implements(ISupportsCommentImport) |
10 | ... sync_comments = True |
11 | - >>> bug_watch_updater.updateBugWatches( |
12 | - ... CommentImportExternalBugTracker(), [], now=utc_now) |
13 | - getCurrentDBTime() called |
14 | - initializeRemoteBugDB() called: [] |
15 | + |
16 | + >>> checkwatches_master = CheckwatchesMaster( |
17 | + ... transaction, syncable_gnome_products=[]) |
18 | + >>> remote_bug_updater = checkwatches_master.remote_bug_updater_factory( |
19 | + ... checkwatches_master, CommentImportExternalBugTracker(), '1', |
20 | + ... [], [], server_time=None) |
21 | WARNING:...:Comment importing supported, but server time can't be |
22 | trusted. No comments will be imported. (OOPS-...) |
23 | |
24 | @@ -641,7 +644,7 @@ |
25 | |
26 | === Converting statuses === |
27 | |
28 | -Once it has retrieved the bugs from the remote server, CheckwatchesMaster |
29 | +Once it has retrieved the bugs from the remote server, RemoteBugUpdater |
30 | attempts to convert their statuses into Launchpad BugTaskStatuses by |
31 | calling the convertRemoteStatus() method on the ExternalBugTracker via |
32 | its own _convertRemoteStatus() method. |
33 | @@ -660,17 +663,18 @@ |
34 | ... else: |
35 | ... raise UnknownRemoteStatusError(remote_status) |
36 | |
37 | -CheckwatchesMaster._convertRemoteStatus() will handle these errors and will |
38 | +RemoteBugUpdater._convertRemoteStatus() will handle these errors and will |
39 | return BugTaskStatus.UNKNOWN when they occur. It will also log a |
40 | warning. |
41 | |
42 | - >>> status = bug_watch_updater._convertRemoteStatus( |
43 | - ... StatusConvertingExternalBugTracker(), 'new') |
44 | + >>> remote_bug_updater = bug_watch_updater.remote_bug_updater_factory( |
45 | + ... bug_watch_updater, StatusConvertingExternalBugTracker(), |
46 | + ... '1', [1], [], utc_now) |
47 | + >>> status = remote_bug_updater._convertRemoteStatus('new') |
48 | >>> print status.title |
49 | New |
50 | |
51 | - >>> status = bug_watch_updater._convertRemoteStatus( |
52 | - ... StatusConvertingExternalBugTracker(), 'spam') |
53 | + >>> status = remote_bug_updater._convertRemoteStatus('spam') |
54 | WARNING...Unknown remote status 'spam'. (OOPS-...) |
55 | >>> print status.title |
56 | Unknown |
57 | |
58 | === modified file 'lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py' |
59 | --- lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py 2010-04-26 12:34:47 +0000 |
60 | +++ lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py 2010-05-05 10:09:40 +0000 |
61 | @@ -26,9 +26,12 @@ |
62 | from lp.bugs.interfaces.bug import IBugSet |
63 | from lp.bugs.scripts.checkwatches.base import ( |
64 | WorkingBase, commit_before) |
65 | +from lp.bugs.scripts.checkwatches.utilities import ( |
66 | + get_remote_system_oops_properties) |
67 | from lp.registry.interfaces.person import PersonCreationRationale |
68 | |
69 | |
70 | + |
71 | class BugWatchUpdater(WorkingBase): |
72 | """Handles the updating of a single BugWatch for checkwatches.""" |
73 | |
74 | @@ -81,10 +84,6 @@ |
75 | @commit_before |
76 | def importBugComments(self): |
77 | """Import all the comments from the remote bug.""" |
78 | - # Avoid circularity. |
79 | - from lp.bugs.scripts.checkwatches.core import ( |
80 | - get_remote_system_oops_properties) |
81 | - |
82 | with self.transaction: |
83 | local_bug_id = self.bug_watch.bug.id |
84 | remote_bug_id = self.bug_watch.remotebug |
85 | |
86 | === modified file 'lib/lp/bugs/scripts/checkwatches/core.py' |
87 | --- lib/lp/bugs/scripts/checkwatches/core.py 2010-04-30 03:10:17 +0000 |
88 | +++ lib/lp/bugs/scripts/checkwatches/core.py 2010-05-05 10:09:40 +0000 |
89 | @@ -37,22 +37,21 @@ |
90 | from canonical.database.constants import UTC_NOW |
91 | from canonical.database.sqlbase import flush_database_updates |
92 | from canonical.launchpad.interfaces import ( |
93 | - BugTaskStatus, BugWatchActivityStatus, CreateBugParams, |
94 | - IBugTrackerSet, IBugWatchSet, IDistribution, ILaunchpadCelebrities, |
95 | - IPersonSet, ISupportsCommentImport, ISupportsCommentPushing, |
96 | - PersonCreationRationale, UNKNOWN_REMOTE_STATUS) |
97 | + CreateBugParams, IBugTrackerSet, IBugWatchSet, IDistribution, |
98 | + ILaunchpadCelebrities, IPersonSet, ISupportsCommentImport, |
99 | + ISupportsCommentPushing, PersonCreationRationale) |
100 | from canonical.launchpad.scripts.logger import log as default_log |
101 | |
102 | from lp.bugs import externalbugtracker |
103 | from lp.bugs.externalbugtracker import ( |
104 | - BATCH_SIZE_UNLIMITED, BugNotFound, BugTrackerConnectError, |
105 | - BugWatchUpdateError, InvalidBugId, PrivateRemoteBug, |
106 | - UnknownBugTrackerTypeError, UnknownRemoteStatusError, UnparseableBugData, |
107 | - UnparseableBugTrackerVersion, UnsupportedBugTrackerVersion) |
108 | + BATCH_SIZE_UNLIMITED, BugWatchUpdateError, |
109 | + UnknownBugTrackerTypeError) |
110 | from lp.bugs.interfaces.externalbugtracker import ISupportsBackLinking |
111 | from lp.bugs.scripts.checkwatches.base import ( |
112 | WorkingBase, commit_before, with_interaction) |
113 | -from lp.bugs.scripts.checkwatches.bugwatchupdater import BugWatchUpdater |
114 | +from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater |
115 | +from lp.bugs.scripts.checkwatches.utilities import ( |
116 | + get_bugwatcherrortype_for_error) |
117 | from lp.services.scripts.base import LaunchpadCronScript |
118 | |
119 | |
120 | @@ -77,30 +76,6 @@ |
121 | """Time difference between ourselves and the remote server is too much.""" |
122 | |
123 | |
124 | -_exception_to_bugwatcherrortype = [ |
125 | - (BugTrackerConnectError, BugWatchActivityStatus.CONNECTION_ERROR), |
126 | - (PrivateRemoteBug, BugWatchActivityStatus.PRIVATE_REMOTE_BUG), |
127 | - (UnparseableBugData, BugWatchActivityStatus.UNPARSABLE_BUG), |
128 | - (UnparseableBugTrackerVersion, |
129 | - BugWatchActivityStatus.UNPARSABLE_BUG_TRACKER), |
130 | - (UnsupportedBugTrackerVersion, |
131 | - BugWatchActivityStatus.UNSUPPORTED_BUG_TRACKER), |
132 | - (UnknownBugTrackerTypeError, |
133 | - BugWatchActivityStatus.UNSUPPORTED_BUG_TRACKER), |
134 | - (InvalidBugId, BugWatchActivityStatus.INVALID_BUG_ID), |
135 | - (BugNotFound, BugWatchActivityStatus.BUG_NOT_FOUND), |
136 | - (PrivateRemoteBug, BugWatchActivityStatus.PRIVATE_REMOTE_BUG), |
137 | - (socket.timeout, BugWatchActivityStatus.TIMEOUT)] |
138 | - |
139 | -def get_bugwatcherrortype_for_error(error): |
140 | - """Return the correct `BugWatchActivityStatus` for a given error.""" |
141 | - for exc_type, bugwatcherrortype in _exception_to_bugwatcherrortype: |
142 | - if isinstance(error, exc_type): |
143 | - return bugwatcherrortype |
144 | - else: |
145 | - return BugWatchActivityStatus.UNKNOWN |
146 | - |
147 | - |
148 | def unique(iterator): |
149 | """Generate only unique items from an iterator.""" |
150 | seen = set() |
151 | @@ -126,26 +101,11 @@ |
152 | int(SUGGESTED_BATCH_SIZE_PROPORTION * num_watches)) |
153 | |
154 | |
155 | -def get_remote_system_oops_properties(remote_system): |
156 | - """Return (name, value) tuples describing a remote system. |
157 | - |
158 | - Each item in the list is intended for use as an OOPS property. |
159 | - |
160 | - :remote_system: The `ExternalBugTracker` instance from which the |
161 | - OOPS properties should be extracted. |
162 | - """ |
163 | - return [ |
164 | - ('batch_size', remote_system.batch_size), |
165 | - ('batch_query_threshold', remote_system.batch_query_threshold), |
166 | - ('sync_comments', remote_system.sync_comments), |
167 | - ('externalbugtracker', remote_system.__class__.__name__), |
168 | - ('baseurl', remote_system.baseurl) |
169 | - ] |
170 | - |
171 | - |
172 | class CheckwatchesMaster(WorkingBase): |
173 | """Takes responsibility for updating remote bug watches.""" |
174 | |
175 | + remote_bug_updater_factory = RemoteBugUpdater |
176 | + |
177 | def __init__(self, transaction_manager, logger=default_log, |
178 | syncable_gnome_products=None): |
179 | """Initialize a CheckwatchesMaster. |
180 | @@ -455,37 +415,6 @@ |
181 | self.logger.debug( |
182 | "No watches to update on %s" % bug_tracker.baseurl) |
183 | |
184 | - def _convertRemoteStatus(self, remotesystem, remote_status): |
185 | - """Convert a remote bug status to a Launchpad status and return it. |
186 | - |
187 | - :param remotesystem: The `IExternalBugTracker` instance |
188 | - representing the remote system. |
189 | - :param remote_status: The remote status to be converted into a |
190 | - Launchpad status. |
191 | - |
192 | - If the remote status cannot be mapped to a Launchpad status, |
193 | - BugTaskStatus.UNKNOWN will be returned and a warning will be |
194 | - logged. |
195 | - """ |
196 | - # We don't bother trying to convert UNKNOWN_REMOTE_STATUS. |
197 | - if remote_status == UNKNOWN_REMOTE_STATUS: |
198 | - return BugTaskStatus.UNKNOWN |
199 | - |
200 | - try: |
201 | - launchpad_status = remotesystem.convertRemoteStatus( |
202 | - remote_status) |
203 | - except UnknownRemoteStatusError: |
204 | - # We log the warning, since we need to know about statuses |
205 | - # that we don't handle correctly. |
206 | - self.warning( |
207 | - "Unknown remote status '%s'." % remote_status, |
208 | - get_remote_system_oops_properties(remotesystem), |
209 | - sys.exc_info()) |
210 | - |
211 | - launchpad_status = BugTaskStatus.UNKNOWN |
212 | - |
213 | - return launchpad_status |
214 | - |
215 | def _getRemoteIdsToCheck(self, remotesystem, bug_watches, |
216 | server_time=None, now=None, batch_size=None): |
217 | """Return the remote bug IDs to check for a set of bug watches. |
218 | @@ -607,17 +536,6 @@ |
219 | 'unmodified_remote_ids': sorted(unmodified_remote_ids), |
220 | } |
221 | |
222 | - def _getBugWatchesForRemoteBug(self, remote_bug_id, bug_watch_ids): |
223 | - """Return a list of bug watches for the given remote bug. |
224 | - |
225 | - The returned watches will all be members of `bug_watch_ids`. |
226 | - |
227 | - This method exists primarily to be overridden during testing. |
228 | - """ |
229 | - return list( |
230 | - getUtility(IBugWatchSet).getBugWatchesForRemoteBug( |
231 | - remote_bug_id, bug_watch_ids)) |
232 | - |
233 | # XXX gmb 2008-11-07 [bug=295319] |
234 | # This method is 186 lines long. It needs to be shorter. |
235 | @commit_before |
236 | @@ -678,140 +596,11 @@ |
237 | bug_watch_ids, get_bugwatcherrortype_for_error(error)) |
238 | raise |
239 | |
240 | - # Whether we can import and / or push comments is determined |
241 | - # on a per-bugtracker-type level. |
242 | - can_import_comments = ( |
243 | - ISupportsCommentImport.providedBy(remotesystem) and |
244 | - remotesystem.sync_comments) |
245 | - can_push_comments = ( |
246 | - ISupportsCommentPushing.providedBy(remotesystem) and |
247 | - remotesystem.sync_comments) |
248 | - can_back_link = ( |
249 | - ISupportsBackLinking.providedBy(remotesystem) and |
250 | - remotesystem.sync_comments) |
251 | - |
252 | - if can_import_comments and server_time is None: |
253 | - can_import_comments = False |
254 | - self.warning( |
255 | - "Comment importing supported, but server time can't be" |
256 | - " trusted. No comments will be imported.") |
257 | - |
258 | - error_type_messages = { |
259 | - BugWatchActivityStatus.INVALID_BUG_ID: |
260 | - ("Invalid bug %(bug_id)r on %(base_url)s " |
261 | - "(local bugs: %(local_ids)s)."), |
262 | - BugWatchActivityStatus.BUG_NOT_FOUND: |
263 | - ("Didn't find bug %(bug_id)r on %(base_url)s " |
264 | - "(local bugs: %(local_ids)s)."), |
265 | - BugWatchActivityStatus.PRIVATE_REMOTE_BUG: |
266 | - ("Remote bug %(bug_id)r on %(base_url)s is private " |
267 | - "(local bugs: %(local_ids)s)."), |
268 | - } |
269 | - error_type_message_default = ( |
270 | - "remote bug: %(bug_id)r; " |
271 | - "base url: %(base_url)s; " |
272 | - "local bugs: %(local_ids)s" |
273 | - ) |
274 | - |
275 | for remote_bug_id in all_remote_ids: |
276 | - with self.transaction: |
277 | - bug_watches = self._getBugWatchesForRemoteBug( |
278 | - remote_bug_id, bug_watch_ids) |
279 | - # If there aren't any bug watches for this remote bug, |
280 | - # just log a warning and carry on. |
281 | - if len(bug_watches) == 0: |
282 | - self.warning( |
283 | - "Spurious remote bug ID: No watches found for " |
284 | - "remote bug %s on %s" % ( |
285 | - remote_bug_id, remotesystem.baseurl)) |
286 | - continue |
287 | - # Mark them all as checked. |
288 | - for bug_watch in bug_watches: |
289 | - bug_watch.lastchecked = UTC_NOW |
290 | - bug_watch.next_check = None |
291 | - # Next if this one is definitely unmodified. |
292 | - if remote_bug_id in unmodified_remote_ids: |
293 | - continue |
294 | - # Save the remote bug URL for error reporting. |
295 | - remote_bug_url = bug_watches[0].url |
296 | - # Save the list of local bug IDs for error reporting. |
297 | - local_ids = ", ".join( |
298 | - str(bug_id) for bug_id in sorted( |
299 | - watch.bug.id for watch in bug_watches)) |
300 | - |
301 | - try: |
302 | - new_remote_status = None |
303 | - new_malone_status = None |
304 | - new_remote_importance = None |
305 | - new_malone_importance = None |
306 | - error = None |
307 | - oops_id = None |
308 | - |
309 | - # XXX: 2007-10-17 Graham Binns |
310 | - # This nested set of try:excepts isn't really |
311 | - # necessary and can be refactored out when bug |
312 | - # 136391 is dealt with. |
313 | - try: |
314 | - new_remote_status = ( |
315 | - remotesystem.getRemoteStatus(remote_bug_id)) |
316 | - new_malone_status = self._convertRemoteStatus( |
317 | - remotesystem, new_remote_status) |
318 | - new_remote_importance = ( |
319 | - remotesystem.getRemoteImportance(remote_bug_id)) |
320 | - new_malone_importance = ( |
321 | - remotesystem.convertRemoteImportance( |
322 | - new_remote_importance)) |
323 | - except (InvalidBugId, BugNotFound, PrivateRemoteBug), ex: |
324 | - error = get_bugwatcherrortype_for_error(ex) |
325 | - message = error_type_messages.get( |
326 | - error, error_type_message_default) |
327 | - oops_id = self.warning( |
328 | - message % { |
329 | - 'bug_id': remote_bug_id, |
330 | - 'base_url': remotesystem.baseurl, |
331 | - 'local_ids': local_ids, |
332 | - }, |
333 | - properties=[ |
334 | - ('URL', remote_bug_url), |
335 | - ('bug_id', remote_bug_id), |
336 | - ('local_ids', local_ids), |
337 | - ] + get_remote_system_oops_properties( |
338 | - remotesystem), |
339 | - info=sys.exc_info()) |
340 | - |
341 | - for bug_watch in bug_watches: |
342 | - bug_watch_updater = BugWatchUpdater( |
343 | - self, bug_watch, remotesystem) |
344 | - |
345 | - bug_watch_updater.updateBugWatch( |
346 | - new_remote_status, new_malone_status, |
347 | - new_remote_importance, new_malone_importance, |
348 | - can_import_comments, can_push_comments, |
349 | - can_back_link, error, oops_id) |
350 | - |
351 | - except (KeyboardInterrupt, SystemExit): |
352 | - # We should never catch KeyboardInterrupt or SystemExit. |
353 | - raise |
354 | - |
355 | - except Exception, error: |
356 | - # Send the error to the log. |
357 | - oops_id = self.error( |
358 | - "Failure updating bug %r on %s (local bugs: %s)." % |
359 | - (remote_bug_id, bug_tracker_url, local_ids), |
360 | - properties=[ |
361 | - ('URL', remote_bug_url), |
362 | - ('bug_id', remote_bug_id), |
363 | - ('local_ids', local_ids)] + |
364 | - get_remote_system_oops_properties(remotesystem)) |
365 | - # We record errors against the bug watches and update |
366 | - # their lastchecked dates so that we don't try to |
367 | - # re-check them every time checkwatches runs. |
368 | - error_type = get_bugwatcherrortype_for_error(error) |
369 | - with self.transaction: |
370 | - getUtility(IBugWatchSet).bulkSetError( |
371 | - bug_watches, error_type) |
372 | - getUtility(IBugWatchSet).bulkAddActivity( |
373 | - bug_watches, result=error_type, oops_id=oops_id) |
374 | + remote_bug_updater = self.remote_bug_updater_factory( |
375 | + self, remotesystem, remote_bug_id, bug_watch_ids, |
376 | + unmodified_remote_ids, server_time) |
377 | + remote_bug_updater.updateRemoteBug() |
378 | |
379 | def importBug(self, external_bugtracker, bugtracker, bug_target, |
380 | remote_bug): |
381 | |
382 | === added file 'lib/lp/bugs/scripts/checkwatches/remotebugupdater.py' |
383 | --- lib/lp/bugs/scripts/checkwatches/remotebugupdater.py 1970-01-01 00:00:00 +0000 |
384 | +++ lib/lp/bugs/scripts/checkwatches/remotebugupdater.py 2010-05-05 10:09:40 +0000 |
385 | @@ -0,0 +1,218 @@ |
386 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
387 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
388 | + |
389 | +"""Classes and logic for the remote bug updater.""" |
390 | + |
391 | +from __future__ import with_statement |
392 | + |
393 | +__metaclass__ = type |
394 | +__all__ = [ |
395 | + 'RemoteBugUpdater', |
396 | + ] |
397 | + |
398 | +import sys |
399 | + |
400 | +from zope.component import getUtility |
401 | + |
402 | +from canonical.database.constants import UTC_NOW |
403 | + |
404 | +from lp.bugs.externalbugtracker import ( |
405 | + BugNotFound, InvalidBugId, PrivateRemoteBug, UnknownRemoteStatusError) |
406 | + |
407 | +from lp.bugs.interfaces.bugtask import BugTaskStatus |
408 | +from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus, IBugWatchSet |
409 | +from lp.bugs.interfaces.externalbugtracker import ( |
410 | + ISupportsBackLinking, ISupportsCommentImport, |
411 | + ISupportsCommentPushing, UNKNOWN_REMOTE_STATUS) |
412 | +from lp.bugs.scripts.checkwatches.base import WorkingBase, commit_before |
413 | +from lp.bugs.scripts.checkwatches.bugwatchupdater import BugWatchUpdater |
414 | +from lp.bugs.scripts.checkwatches.utilities import ( |
415 | + get_bugwatcherrortype_for_error, get_remote_system_oops_properties) |
416 | + |
417 | + |
418 | +class RemoteBugUpdater(WorkingBase): |
419 | + |
420 | + def __init__(self, parent, external_bugtracker, remote_bug, |
421 | + bug_watch_ids, unmodified_remote_ids, server_time): |
422 | + self.initFromParent(parent) |
423 | + self.external_bugtracker = external_bugtracker |
424 | + self.bug_tracker_url = external_bugtracker.baseurl |
425 | + self.remote_bug = remote_bug |
426 | + self.bug_watch_ids = bug_watch_ids |
427 | + self.unmodified_remote_ids = unmodified_remote_ids |
428 | + |
429 | + self.error_type_messages = { |
430 | + BugWatchActivityStatus.INVALID_BUG_ID: |
431 | + ("Invalid bug %(bug_id)r on %(base_url)s " |
432 | + "(local bugs: %(local_ids)s)."), |
433 | + BugWatchActivityStatus.BUG_NOT_FOUND: |
434 | + ("Didn't find bug %(bug_id)r on %(base_url)s " |
435 | + "(local bugs: %(local_ids)s)."), |
436 | + BugWatchActivityStatus.PRIVATE_REMOTE_BUG: |
437 | + ("Remote bug %(bug_id)r on %(base_url)s is private " |
438 | + "(local bugs: %(local_ids)s)."), |
439 | + } |
440 | + self.error_type_message_default = ( |
441 | + "remote bug: %(bug_id)r; " |
442 | + "base url: %(base_url)s; " |
443 | + "local bugs: %(local_ids)s" |
444 | + ) |
445 | + |
446 | + # Whether we can import and / or push comments is determined |
447 | + # on a per-bugtracker-type level. |
448 | + self.can_import_comments = ( |
449 | + ISupportsCommentImport.providedBy(external_bugtracker) and |
450 | + external_bugtracker.sync_comments) |
451 | + self.can_push_comments = ( |
452 | + ISupportsCommentPushing.providedBy(external_bugtracker) and |
453 | + external_bugtracker.sync_comments) |
454 | + self.can_back_link = ( |
455 | + ISupportsBackLinking.providedBy(external_bugtracker) and |
456 | + external_bugtracker.sync_comments) |
457 | + |
458 | + if self.can_import_comments and server_time is None: |
459 | + self.can_import_comments = False |
460 | + self.warning( |
461 | + "Comment importing supported, but server time can't be " |
462 | + "trusted. No comments will be imported.") |
463 | + |
464 | + def _getBugWatchesForRemoteBug(self): |
465 | + """Return a list of bug watches for the current remote bug. |
466 | + |
467 | + The returned watches will all be members of `self.bug_watch_ids`. |
468 | + |
469 | + This method exists primarily to be overridden during testing. |
470 | + """ |
471 | + return list( |
472 | + getUtility(IBugWatchSet).getBugWatchesForRemoteBug( |
473 | + self.remote_bug, self.bug_watch_ids)) |
474 | + |
475 | + @commit_before |
476 | + def updateRemoteBug(self): |
477 | + with self.transaction: |
478 | + bug_watches = self._getBugWatchesForRemoteBug() |
479 | + # If there aren't any bug watches for this remote bug, |
480 | + # just log a warning and carry on. |
481 | + if len(bug_watches) == 0: |
482 | + self.warning( |
483 | + "Spurious remote bug ID: No watches found for " |
484 | + "remote bug %s on %s" % ( |
485 | + self.remote_bug, self.external_bugtracker.baseurl)) |
486 | + return |
487 | + # Mark them all as checked. |
488 | + for bug_watch in bug_watches: |
489 | + bug_watch.lastchecked = UTC_NOW |
490 | + bug_watch.next_check = None |
491 | + # Return if this one is definitely unmodified. |
492 | + if self.remote_bug in self.unmodified_remote_ids: |
493 | + return |
494 | + # Save the remote bug URL for error reporting. |
495 | + remote_bug_url = bug_watches[0].url |
496 | + # Save the list of local bug IDs for error reporting. |
497 | + local_ids = ", ".join( |
498 | + str(bug_id) for bug_id in sorted( |
499 | + watch.bug.id for watch in bug_watches)) |
500 | + |
501 | + try: |
502 | + new_remote_status = None |
503 | + new_malone_status = None |
504 | + new_remote_importance = None |
505 | + new_malone_importance = None |
506 | + error = None |
507 | + oops_id = None |
508 | + |
509 | + # XXX: 2007-10-17 Graham Binns |
510 | + # This nested set of try:excepts isn't really |
511 | + # necessary and can be refactored out when bug |
512 | + # 136391 is dealt with. |
513 | + try: |
514 | + new_remote_status = ( |
515 | + self.external_bugtracker.getRemoteStatus( |
516 | + self.remote_bug)) |
517 | + new_malone_status = self._convertRemoteStatus( |
518 | + new_remote_status) |
519 | + new_remote_importance = ( |
520 | + self.external_bugtracker.getRemoteImportance( |
521 | + self.remote_bug)) |
522 | + new_malone_importance = ( |
523 | + self.external_bugtracker.convertRemoteImportance( |
524 | + new_remote_importance)) |
525 | + except (InvalidBugId, BugNotFound, PrivateRemoteBug), ex: |
526 | + error = get_bugwatcherrortype_for_error(ex) |
527 | + message = self.error_type_messages.get( |
528 | + error, self.error_type_message_default) |
529 | + oops_id = self.warning( |
530 | + message % { |
531 | + 'bug_id': self.remote_bug, |
532 | + 'base_url': self.external_bugtracker.baseurl, |
533 | + 'local_ids': local_ids, |
534 | + }, |
535 | + properties=[ |
536 | + ('URL', remote_bug_url), |
537 | + ('bug_id', self.remote_bug), |
538 | + ('local_ids', local_ids), |
539 | + ] + get_remote_system_oops_properties( |
540 | + self.external_bugtracker), |
541 | + info=sys.exc_info()) |
542 | + |
543 | + for bug_watch in bug_watches: |
544 | + bug_watch_updater = BugWatchUpdater( |
545 | + self, bug_watch, self.external_bugtracker) |
546 | + |
547 | + bug_watch_updater.updateBugWatch( |
548 | + new_remote_status, new_malone_status, |
549 | + new_remote_importance, new_malone_importance, |
550 | + self.can_import_comments, self.can_push_comments, |
551 | + self.can_back_link, error, oops_id) |
552 | + |
553 | + except Exception, error: |
554 | + # Send the error to the log. |
555 | + oops_id = self.error( |
556 | + "Failure updating bug %r on %s (local bugs: %s)." % |
557 | + (self.remote_bug, self.bug_tracker_url, local_ids), |
558 | + properties=[ |
559 | + ('URL', remote_bug_url), |
560 | + ('bug_id', self.remote_bug), |
561 | + ('local_ids', local_ids)] + |
562 | + get_remote_system_oops_properties( |
563 | + self.external_bugtracker)) |
564 | + # We record errors against the bug watches and update |
565 | + # their lastchecked dates so that we don't try to |
566 | + # re-check them every time checkwatches runs. |
567 | + error_type = get_bugwatcherrortype_for_error(error) |
568 | + with self.transaction: |
569 | + getUtility(IBugWatchSet).bulkSetError( |
570 | + bug_watches, error_type) |
571 | + getUtility(IBugWatchSet).bulkAddActivity( |
572 | + bug_watches, result=error_type, oops_id=oops_id) |
573 | + |
574 | + def _convertRemoteStatus(self, remote_status): |
575 | + """Convert a remote bug status to a Launchpad status and return it. |
576 | + |
577 | + :param remote_status: The remote status to be converted into a |
578 | + Launchpad status. |
579 | + |
580 | + If the remote status cannot be mapped to a Launchpad status, |
581 | + BugTaskStatus.UNKNOWN will be returned and a warning will be |
582 | + logged. |
583 | + """ |
584 | + # We don't bother trying to convert UNKNOWN_REMOTE_STATUS. |
585 | + if remote_status == UNKNOWN_REMOTE_STATUS: |
586 | + return BugTaskStatus.UNKNOWN |
587 | + |
588 | + try: |
589 | + launchpad_status = self.external_bugtracker.convertRemoteStatus( |
590 | + remote_status) |
591 | + except UnknownRemoteStatusError: |
592 | + # We log the warning, since we need to know about statuses |
593 | + # that we don't handle correctly. |
594 | + self.warning( |
595 | + "Unknown remote status '%s'." % remote_status, |
596 | + get_remote_system_oops_properties( |
597 | + self.external_bugtracker), |
598 | + sys.exc_info()) |
599 | + |
600 | + launchpad_status = BugTaskStatus.UNKNOWN |
601 | + |
602 | + return launchpad_status |
603 | + |
604 | |
605 | === modified file 'lib/lp/bugs/scripts/checkwatches/tests/test_core.py' |
606 | --- lib/lp/bugs/scripts/checkwatches/tests/test_core.py 2010-04-23 11:19:49 +0000 |
607 | +++ lib/lp/bugs/scripts/checkwatches/tests/test_core.py 2010-05-05 10:09:40 +0000 |
608 | @@ -11,6 +11,7 @@ |
609 | |
610 | import transaction |
611 | |
612 | +from datetime import datetime |
613 | from zope.component import getUtility |
614 | |
615 | from canonical.config import config |
616 | @@ -24,9 +25,11 @@ |
617 | from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI |
618 | from lp.bugs.interfaces.bugtracker import IBugTrackerSet |
619 | from lp.bugs.scripts import checkwatches |
620 | +from lp.bugs.scripts.checkwatches.base import ( |
621 | + CheckWatchesErrorUtility, WorkingBase) |
622 | from lp.bugs.scripts.checkwatches.core import ( |
623 | - CheckwatchesMaster, TwistedThreadScheduler) |
624 | -from lp.bugs.scripts.checkwatches.base import CheckWatchesErrorUtility |
625 | + CheckwatchesMaster, LOGIN, TwistedThreadScheduler) |
626 | +from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater |
627 | from lp.bugs.tests.externalbugtracker import ( |
628 | TestBugzillaAPIXMLRPCTransport, TestExternalBugTracker, new_bugtracker) |
629 | from lp.testing import TestCaseWithFactory, ZopeTestInSubProcess |
630 | @@ -57,10 +60,10 @@ |
631 | return self |
632 | |
633 | |
634 | -class NoBugWatchesByRemoteBugUpdater(checkwatches.CheckwatchesMaster): |
635 | - """A subclass of CheckwatchesMaster with methods overridden for testing.""" |
636 | +class NoBugWatchesByRemoteBugUpdater(RemoteBugUpdater): |
637 | + """A subclass of RemoteBugUpdater with methods overridden for testing.""" |
638 | |
639 | - def _getBugWatchesForRemoteBug(self, remote_bug_id, bug_watch_ids): |
640 | + def _getBugWatchesForRemoteBug(self): |
641 | """Return an empty list. |
642 | |
643 | This method overrides _getBugWatchesForRemoteBug() so that bug |
644 | @@ -154,10 +157,8 @@ |
645 | |
646 | def test_bug_497141(self): |
647 | # Regression test for bug 497141. KeyErrors raised in |
648 | - # CheckwatchesMaster.updateBugWatches() shouldn't cause |
649 | + # RemoteBugUpdater.updateRemoteBug() shouldn't cause |
650 | # checkwatches to abort. |
651 | - updater = NoBugWatchesByRemoteBugUpdater( |
652 | - transaction.manager, QuietFakeLogger()) |
653 | |
654 | # Create a couple of bug watches for testing purposes. |
655 | bug_tracker = self.factory.makeBugTracker() |
656 | @@ -170,17 +171,27 @@ |
657 | remote_system = NonConnectingBugzillaAPI( |
658 | bug_tracker.baseurl, xmlrpc_transport=test_transport) |
659 | |
660 | - # Calling updateBugWatches() shouldn't raise a KeyError, even |
661 | - # though with our broken updater _getExternalBugTrackersAndWatches() |
662 | - # will return an empty dict. |
663 | - updater.updateBugWatches(remote_system, bug_watches) |
664 | - |
665 | - # An error will have been logged instead of the KeyError being |
666 | - # raised. |
667 | - error_utility = CheckWatchesErrorUtility() |
668 | - last_oops = error_utility.getLastOopsReport() |
669 | - self.assertTrue( |
670 | - last_oops.value.startswith('Spurious remote bug ID')) |
671 | + working_base = WorkingBase() |
672 | + working_base.init(LOGIN, transaction.manager, QuietFakeLogger()) |
673 | + |
674 | + for bug_watch in bug_watches: |
675 | + updater = NoBugWatchesByRemoteBugUpdater( |
676 | + working_base, remote_system, bug_watch.remotebug, |
677 | + [bug_watch.id], [], datetime.now()) |
678 | + |
679 | + # Calling updateRemoteBug() shouldn't raise a KeyError, |
680 | + # even though with our broken updater |
681 | + # _getExternalBugTrackersAndWatches() will return an empty |
682 | + # dict. |
683 | + updater.updateRemoteBug() |
684 | + |
685 | + # An error will have been logged instead of the KeyError being |
686 | + # raised. |
687 | + error_utility = CheckWatchesErrorUtility() |
688 | + last_oops = error_utility.getLastOopsReport() |
689 | + self.assertTrue( |
690 | + last_oops.value.startswith('Spurious remote bug ID'), |
691 | + "Unexpected last OOPS value: %s" % last_oops.value) |
692 | |
693 | def test_suggest_batch_size(self): |
694 | class RemoteSystem: pass |
695 | |
696 | === added file 'lib/lp/bugs/scripts/checkwatches/tests/test_remotebugupdater.py' |
697 | --- lib/lp/bugs/scripts/checkwatches/tests/test_remotebugupdater.py 1970-01-01 00:00:00 +0000 |
698 | +++ lib/lp/bugs/scripts/checkwatches/tests/test_remotebugupdater.py 2010-05-05 10:09:40 +0000 |
699 | @@ -0,0 +1,59 @@ |
700 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
701 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
702 | + |
703 | +"""Tests for the checkwatches remotebugupdater module.""" |
704 | + |
705 | +__metaclass__ = type |
706 | + |
707 | +import transaction |
708 | +import unittest |
709 | + |
710 | +from canonical.testing import LaunchpadZopelessLayer |
711 | + |
712 | +from lp.bugs.externalbugtracker.bugzilla import Bugzilla |
713 | +from lp.bugs.scripts.checkwatches.core import CheckwatchesMaster |
714 | +from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater |
715 | +from lp.testing import TestCaseWithFactory |
716 | + |
717 | + |
718 | +class RemoteBugUpdaterTestCase(TestCaseWithFactory): |
719 | + |
720 | + layer = LaunchpadZopelessLayer |
721 | + |
722 | + def test_create(self): |
723 | + # CheckwatchesMaster.remote_bug_updater_factory points to the |
724 | + # RemoteBugUpdater class, so it can be used to create |
725 | + # RemoteBugUpdaters. |
726 | + remote_system = Bugzilla('http://example.com') |
727 | + remote_bug_id = '42' |
728 | + bug_watch_ids = [1, 2] |
729 | + unmodified_remote_ids = ['76'] |
730 | + |
731 | + checkwatches_master = CheckwatchesMaster(transaction) |
732 | + updater = checkwatches_master.remote_bug_updater_factory( |
733 | + checkwatches_master, remote_system, remote_bug_id, |
734 | + bug_watch_ids, unmodified_remote_ids, None) |
735 | + |
736 | + self.assertTrue( |
737 | + isinstance(updater, RemoteBugUpdater), |
738 | + "updater should be an instance of RemoteBugUpdater.") |
739 | + self.assertEqual( |
740 | + remote_system, updater.external_bugtracker, |
741 | + "Unexpected external_bugtracker for RemoteBugUpdater.") |
742 | + self.assertEqual( |
743 | + remote_bug_id, updater.remote_bug, |
744 | + "RemoteBugUpdater's remote_bug should be '%s', was '%s'" % |
745 | + (remote_bug_id, updater.remote_bug)) |
746 | + self.assertEqual( |
747 | + bug_watch_ids, updater.bug_watch_ids, |
748 | + "RemoteBugUpdater's bug_watch_ids should be '%s', were '%s'" % |
749 | + (bug_watch_ids, updater.bug_watch_ids)) |
750 | + self.assertEqual( |
751 | + unmodified_remote_ids, updater.unmodified_remote_ids, |
752 | + "RemoteBugUpdater's unmodified_remote_ids should be '%s', " |
753 | + "were '%s'" % |
754 | + (unmodified_remote_ids, updater.unmodified_remote_ids)) |
755 | + |
756 | + |
757 | +def test_suite(): |
758 | + return unittest.TestLoader().loadTestsFromName(__name__) |
759 | |
760 | === added file 'lib/lp/bugs/scripts/checkwatches/utilities.py' |
761 | --- lib/lp/bugs/scripts/checkwatches/utilities.py 1970-01-01 00:00:00 +0000 |
762 | +++ lib/lp/bugs/scripts/checkwatches/utilities.py 2010-05-05 10:09:40 +0000 |
763 | @@ -0,0 +1,61 @@ |
764 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
765 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
766 | + |
767 | +"""Utility functions for checkwatches.""" |
768 | + |
769 | +__metaclass__ = type |
770 | +__all__ = [ |
771 | + 'get_bugwatcherrortype_for_error', |
772 | + 'get_remote_system_oops_properties', |
773 | + ] |
774 | + |
775 | +import socket |
776 | + |
777 | +from lp.bugs.externalbugtracker import ( |
778 | + BugNotFound, BugTrackerConnectError, InvalidBugId, PrivateRemoteBug, |
779 | + UnknownBugTrackerTypeError, UnparseableBugData, |
780 | + UnparseableBugTrackerVersion, UnsupportedBugTrackerVersion) |
781 | + |
782 | +from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus |
783 | + |
784 | + |
785 | +_exception_to_bugwatcherrortype = [ |
786 | + (BugTrackerConnectError, BugWatchActivityStatus.CONNECTION_ERROR), |
787 | + (PrivateRemoteBug, BugWatchActivityStatus.PRIVATE_REMOTE_BUG), |
788 | + (UnparseableBugData, BugWatchActivityStatus.UNPARSABLE_BUG), |
789 | + (UnparseableBugTrackerVersion, |
790 | + BugWatchActivityStatus.UNPARSABLE_BUG_TRACKER), |
791 | + (UnsupportedBugTrackerVersion, |
792 | + BugWatchActivityStatus.UNSUPPORTED_BUG_TRACKER), |
793 | + (UnknownBugTrackerTypeError, |
794 | + BugWatchActivityStatus.UNSUPPORTED_BUG_TRACKER), |
795 | + (InvalidBugId, BugWatchActivityStatus.INVALID_BUG_ID), |
796 | + (BugNotFound, BugWatchActivityStatus.BUG_NOT_FOUND), |
797 | + (PrivateRemoteBug, BugWatchActivityStatus.PRIVATE_REMOTE_BUG), |
798 | + (socket.timeout, BugWatchActivityStatus.TIMEOUT)] |
799 | + |
800 | + |
801 | +def get_bugwatcherrortype_for_error(error): |
802 | + """Return the correct `BugWatchActivityStatus` for a given error.""" |
803 | + for exc_type, bugwatcherrortype in _exception_to_bugwatcherrortype: |
804 | + if isinstance(error, exc_type): |
805 | + return bugwatcherrortype |
806 | + else: |
807 | + return BugWatchActivityStatus.UNKNOWN |
808 | + |
809 | + |
810 | +def get_remote_system_oops_properties(remote_system): |
811 | + """Return (name, value) tuples describing a remote system. |
812 | + |
813 | + Each item in the list is intended for use as an OOPS property. |
814 | + |
815 | + :remote_system: The `ExternalBugTracker` instance from which the |
816 | + OOPS properties should be extracted. |
817 | + """ |
818 | + return [ |
819 | + ('batch_size', remote_system.batch_size), |
820 | + ('batch_query_threshold', remote_system.batch_query_threshold), |
821 | + ('sync_comments', remote_system.sync_comments), |
822 | + ('externalbugtracker', remote_system.__class__.__name__), |
823 | + ('baseurl', remote_system.baseurl) |
824 | + ] |
825 | |
826 | === modified file 'lib/lp/bugs/scripts/tests/test_bugimport.py' |
827 | --- lib/lp/bugs/scripts/tests/test_bugimport.py 2010-04-29 10:09:24 +0000 |
828 | +++ lib/lp/bugs/scripts/tests/test_bugimport.py 2010-05-05 10:09:40 +0000 |
829 | @@ -30,6 +30,7 @@ |
830 | from lp.bugs.scripts import bugimport |
831 | from lp.bugs.scripts.bugimport import ET |
832 | from lp.bugs.scripts.checkwatches import CheckwatchesMaster |
833 | +from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater |
834 | from lp.registry.interfaces.person import IPersonSet, PersonCreationRationale |
835 | from lp.registry.interfaces.product import IProductSet |
836 | from lp.registry.model.person import generate_nick |
837 | @@ -892,6 +893,29 @@ |
838 | return BugTaskImportance.UNKNOWN |
839 | |
840 | |
841 | +class TestRemoteBugUpdater(RemoteBugUpdater): |
842 | + |
843 | + def __init__(self, parent, external_bugtracker, remote_bug, |
844 | + bug_watch_ids, unmodified_remote_ids, server_time, |
845 | + bugtracker): |
846 | + super(TestRemoteBugUpdater, self). __init__( |
847 | + parent, external_bugtracker, remote_bug, bug_watch_ids, |
848 | + unmodified_remote_ids, server_time) |
849 | + self.bugtracker = bugtracker |
850 | + |
851 | + def _getBugWatchesForRemoteBug(self): |
852 | + """Returns a list of fake bug watch objects. |
853 | + |
854 | + We override this method so that we always return bug watches |
855 | + from our list of fake bug watches. |
856 | + """ |
857 | + return [ |
858 | + bug_watch for bug_watch in ( |
859 | + self.bugtracker.watches_needing_update) |
860 | + if (bug_watch.remotebug == self.remote_bug and |
861 | + bug_watch.id in self.bug_watch_ids) |
862 | + ] |
863 | + |
864 | |
865 | class TestCheckwatchesMaster(CheckwatchesMaster): |
866 | """A mock `CheckwatchesMaster` object.""" |
867 | @@ -905,28 +929,21 @@ |
868 | """See `CheckwatchesMaster`.""" |
869 | return [(TestExternalBugTracker(bug_tracker.baseurl), bug_watches)] |
870 | |
871 | - def _getBugWatchesForRemoteBug(self, remote_bug_id, bug_watch_ids): |
872 | - """Returns a list of fake bug watch objects. |
873 | - |
874 | - We override this method so that we always return bug watches |
875 | - from our list of fake bug watches. |
876 | - """ |
877 | - return [ |
878 | - bug_watch for bug_watch in ( |
879 | - self.bugtracker.watches_needing_update) |
880 | - if (bug_watch.remotebug == remote_bug_id and |
881 | - bug_watch.id in bug_watch_ids) |
882 | - ] |
883 | - |
884 | - |
885 | -class CheckBugWatchesErrorRecoveryTestCase(unittest.TestCase): |
886 | + def remote_bug_updater_factory(self, parent, external_bugtracker, |
887 | + remote_bug, bug_watch_ids, |
888 | + unmodified_remote_ids, server_time): |
889 | + return TestRemoteBugUpdater( |
890 | + self, external_bugtracker, remote_bug, bug_watch_ids, |
891 | + unmodified_remote_ids, server_time, self.bugtracker) |
892 | + |
893 | + |
894 | +class CheckwatchesErrorRecoveryTestCase(unittest.TestCase): |
895 | """Test that errors in the bugwatch import process don't |
896 | invalidate the entire run. |
897 | """ |
898 | layer = LaunchpadZopelessLayer |
899 | |
900 | - def test_checkbugwatches_error_recovery(self): |
901 | - |
902 | + def test_checkwatches_error_recovery(self): |
903 | firefox = getUtility(IProductSet).get(4) |
904 | foobar = getUtility(IPersonSet).get(16) |
905 | params = CreateBugParams( |
Hi Graham,
This mad refactorage is looking good :) I have a few comments but
nothing major, so r=me.
Gavin.
> === modified file 'lib/lp/ bugs/doc/ externalbugtrac ker.txt' bugs/doc/ externalbugtrac ker.txt 2010-04-21 10:33:55 +0000 bugs/doc/ externalbugtrac ker.txt 2010-04-29 10:42:49 +0000 atus() method on the ExternalBugTracker via tatus() method. atusError( remote_ status) ster._convertRe moteStatus( ) will handle these errors and will er._convertRemo teStatus( ) will handle these errors and will UNKNOWN when they occur. It will also log a updater. _convertRemoteS tatus( gExternalBugTra cker(), 'new') updater. _makeRemoteBugU pdater( gExternalBugTra cker(), '1', [1], []) bug_updater. _convertRemoteS tatus(' new') updater. _convertRemoteS tatus( gExternalBugTra cker(), 'spam') bug_updater. _convertRemoteS tatus(' spam') bugs/scripts/ checkwatches/ bugwatchupdater .py' bugs/scripts/ checkwatches/ bugwatchupdater .py 2010-04-26 12:34:47 +0000 bugs/scripts/ checkwatches/ bugwatchupdater .py 2010-04-29 10:42:49 +0000 interfaces. bug import IBugSet scripts. checkwatches. base import ( scripts. checkwatches. utilities import ( system_ oops_properties ) interfaces. person import PersonCreationR ationale (WorkingBase) : ts(self) : scripts. checkwatches. core import ( system_ oops_properties ) watch.bug. id watch.remotebug bugs/scripts/ checkwatches/ core.py' bugs/scripts/ checkwatches/ core.py 2010-04-26 12:34:47 +0000 bugs/scripts/ checkwatches/ core.py 2010-04-29 10:42:49 +0000 database. constants import UTC_NOW database. sqlbase import flush_database_ updates
> --- lib/lp/
> +++ lib/lp/
> @@ -641,7 +641,7 @@
>
> === Converting statuses ===
>
> -Once it has retrieved the bugs from the remote server, CheckwatchesMaster
> +Once it has retrieved the bugs from the remote server, RemoteBugUpdater
> attempts to convert their statuses into Launchpad BugTaskStatuses by
> calling the convertRemoteSt
> its own _convertRemoteS
> @@ -660,17 +660,17 @@
> ... else:
> ... raise UnknownRemoteSt
>
> -CheckwatchesMa
> +RemoteBugUpdat
> return BugTaskStatus.
> warning.
>
> - >>> status = bug_watch_
> - ... StatusConvertin
> + >>> remote_bug_updater = bug_watch_
> + ... StatusConvertin
> + >>> status = remote_
> >>> print status.title
> New
>
> - >>> status = bug_watch_
> - ... StatusConvertin
> + >>> status = remote_
> WARNING...Unknown remote status 'spam'. (OOPS-...)
> >>> print status.title
> Unknown
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -26,9 +26,12 @@
> from lp.bugs.
> from lp.bugs.
> WorkingBase, commit_before)
> +from lp.bugs.
> + get_remote_
> from lp.registry.
>
>
> +
> class BugWatchUpdater
> """Handles the updating of a single BugWatch for checkwatches."""
>
> @@ -81,10 +84,6 @@
> @commit_before
> def importBugCommen
> """Import all the comments from the remote bug."""
> - # Avoid circularity.
> - from lp.bugs.
> - get_remote_
> -
> with self.transaction:
> local_bug_id = self.bug_
> remote_bug_id = self.bug_
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -37,22 +37,21 @@
> from canonical.
> from canonical.
> f...