Merge lp://staging/~bryce/launchpad/lp-796645-cron-dies-on-404 into lp://staging/launchpad

Proposed by Bryce Harrington
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 13362
Proposed branch: lp://staging/~bryce/launchpad/lp-796645-cron-dies-on-404
Merge into: lp://staging/launchpad
Diff against target: 203 lines (+104/-25)
2 files modified
lib/lp/bugs/scripts/bzremotecomponentfinder.py (+24/-20)
lib/lp/bugs/tests/test_bzremotecomponentfinder.py (+80/-5)
To merge this branch: bzr merge lp://staging/~bryce/launchpad/lp-796645-cron-dies-on-404
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+66541@code.staging.launchpad.net

Commit message

[r=sinzui][bug=796645] Log warning when bug trackers are not available.

Description of the change

The update-bugzilla-remote-components.py cron job can fail when getRemoteProductsAndComponents() hits various HTTPErrors on any bug tracker. (LP: #796645)

This merge adds a general exception handler to catch and ignore any errors thrown by the urllib2 call. This will allow data to be gathered from those bugzillas which are not broken. It does not attempt any corrections, retries, or workarounds for broken bug trackers, it just leaves any previously collected data in place and carries on.

Most of this branch is adding test code so we can synthetically test for HTTPErrors.

This merge modifies getRemoteProductsAndComponents() so that instead of creating a new scraper object per bugtracker, to instead allow the test code to pass in its own scraper, which is used for all requests instead of generating a scraper object for each URL.

The test code can then supply scrapers subclassed from BugzillaRemoteComponentScraper with hardcoded behaviors:

  StaticTextBugzillaRemoteComponentScraper replaces the static text
  parameter. Instead of hardcoding a check for this option, we allow the
  test code to provide the static text via the subclasses' getPage().

  FaultyBugzillaRemoteComponentScraper provides a way to simulate hitting
  various error codes by raising whatever errors we pass to it.

A assertGetRemoteProductsAndComponentsDoesNotAssert() helper method is added to allow us to run getRemoteProductsAndComponents() and check that no assertions were thrown.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

> === modified file 'lib/lp/bugs/scripts/bzremotecomponentfinder.py'
> --- lib/lp/bugs/scripts/bzremotecomponentfinder.py 2011-06-02 22:27:11 +0000
> +++ lib/lp/bugs/scripts/bzremotecomponentfinder.py 2011-07-01 02:59:24 +0000
...
> + try:
> + self.logger.debug("...Fetching page")
> + page_text = bz_bugtracker.getPage()
> + except HTTPError, error:
> + self.logger.error("Error fetching %s: %s" % (
> + lp_bugtracker.baseurl, error))
> + continue
> + except:
> + self.logger.error("Failed to access %s" % (
> + lp_bugtracker.baseurl))
> + continue

We logged code failures, or failures we can fix as errors, we issues with
other sites as warnings. The faults I saw in testing were dead sites or
bad user data...I do not think these cases are errors.

review: Needs Information (code)
Revision history for this message
Curtis Hovey (sinzui) wrote :

Sorry. My question is unintelligable.

Launchpad logs code failures or failures that engineers must fix as errors.
launchpad logs issues with other sites or user data as warnings. The faults
I saw in testing were dead sites or bad user data...I do not think these
cases are errors.

Revision history for this message
Bryce Harrington (bryce) wrote :

Gotcha, I've switched the two errors to warnings.

(When this does function properly and run through all the sites, I'd be interested in seeing the list of warnings if the data can be made available.)

Revision history for this message
Curtis Hovey (sinzui) wrote :

I am sending this to land now.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.