2010-09-28 Daniel Bates <dbates@rim.com>
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Sep 2010 07:25:40 +0000 (07:25 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Sep 2010 07:25:40 +0000 (07:25 +0000)
        Rollout changeset 68493 <http://trac.webkit.org/changeset/68493>
        <https://bugs.webkit.org/show_bug.cgi?id=39136>

        Rollout changeset 68493 because it broke Sheriffbot's rollout feature.
        In particular, this change caused Sheriffbot to raise an exception when
        trying to parse the bug id on a bug page. We need to look into this
        some more.

        * Scripts/webkitpy/common/net/bugzilla.py:
        * Scripts/webkitpy/common/net/bugzilla_unittest.py:
        * Scripts/webkitpy/tool/bot/sheriff.py:
        * Scripts/webkitpy/tool/commands/download.py:
        * Scripts/webkitpy/tool/commands/queues.py:
        * Scripts/webkitpy/tool/commands/upload.py:
        * Scripts/webkitpy/tool/steps/closebug.py:
        * Scripts/webkitpy/tool/steps/obsoletepatches.py:
        * Scripts/webkitpy/tool/steps/preparechangelog.py:
        * Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@68496 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py
WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py
WebKitTools/Scripts/webkitpy/tool/commands/download.py
WebKitTools/Scripts/webkitpy/tool/commands/queues.py
WebKitTools/Scripts/webkitpy/tool/commands/upload.py
WebKitTools/Scripts/webkitpy/tool/steps/closebug.py
WebKitTools/Scripts/webkitpy/tool/steps/obsoletepatches.py
WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py
WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py

index 25a5f0c..3e46f0e 100644 (file)
@@ -1,3 +1,24 @@
+2010-09-28  Daniel Bates  <dbates@rim.com>
+
+        Rollout changeset 68493 <http://trac.webkit.org/changeset/68493>
+        <https://bugs.webkit.org/show_bug.cgi?id=39136>
+
+        Rollout changeset 68493 because it broke Sheriffbot's rollout feature.
+        In particular, this change caused Sheriffbot to raise an exception when
+        trying to parse the bug id on a bug page. We need to look into this
+        some more.
+
+        * Scripts/webkitpy/common/net/bugzilla.py:
+        * Scripts/webkitpy/common/net/bugzilla_unittest.py:
+        * Scripts/webkitpy/tool/bot/sheriff.py:
+        * Scripts/webkitpy/tool/commands/download.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/tool/commands/upload.py:
+        * Scripts/webkitpy/tool/steps/closebug.py:
+        * Scripts/webkitpy/tool/steps/obsoletepatches.py:
+        * Scripts/webkitpy/tool/steps/preparechangelog.py:
+        * Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py:
+
 2010-09-28  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
index f149e2f..cc64fac 100644 (file)
@@ -45,39 +45,6 @@ from webkitpy.thirdparty.autoinstalled.mechanize import Browser
 from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, SoupStrainer
 
 
-class BugzillaError(Exception):
-
-    def __init__(self, bug_id, error_message=None):
-        Exception.__init__(self, error_message)
-        self.bug_id = bug_id
-        self.error_message = error_message
-
-    def bug_id(self):
-        return self.bug_id
-
-    def error_message(self):
-        return self.error_message
-
-    # The following error messages were extracted from <https://bugs.webkit.org/bugzilla.dtd> as of 09/12/2010.
-
-    def is_invalid_bug_id(self):
-        return self.error_message == "InvalidBugId"
-
-    def is_not_found(self):
-        return self.error_message == "NotFound"
-
-    def is_not_permitted_to_view_bug(self):
-        return self.error_message == "NotPermitted"
-
-    def __str__(self):
-        if self.is_invalid_bug_id():
-            return "Bug %s is invalid" % self.bug_id
-        if self.is_not_found():
-            return "Bug %s does not exist" % self.bug_id
-        if self.is_not_permitted_to_view_bug():
-            return "Not permitted to view bug %s" % self.bug_id
-        return "Bugzilla encountered an error when processing bug %s: '%s'" % (self.bug_id, self.error_message)
-
 def parse_bug_id(message):
     if not message:
         return None
@@ -543,12 +510,8 @@ class Bugzilla(object):
 
     def _parse_bug_page(self, page):
         soup = BeautifulSoup(page)
-        bug_element = soup.find("bug")
-        bug_id = soup.find("bug_id").string
-        if bug_element.has_key("error"):
-            raise BugzillaError(bug_id, bug_element["error"])
         bug = {}
-        bug["id"] = int(bug_id)
+        bug["id"] = int(soup.find("bug_id").string)
         bug["title"] = self._string_contents(soup.find("short_desc"))
         bug["bug_status"] = self._string_contents(soup.find("bug_status"))
         bug["reporter_email"] = self._string_contents(soup.find("reporter"))
index 7e355f2..32f23cd 100644 (file)
@@ -31,7 +31,7 @@ import unittest
 import datetime
 
 from webkitpy.common.config.committers import CommitterList, Reviewer, Committer
-from webkitpy.common.net.bugzilla import Bugzilla, BugzillaError, BugzillaQueries, parse_bug_id, CommitterValidator, Bug
+from webkitpy.common.net.bugzilla import Bugzilla, BugzillaQueries, parse_bug_id, CommitterValidator, Bug
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup
@@ -72,14 +72,6 @@ class CommitterValidatorTest(unittest.TestCase):
         self.assertEqual(validator._flag_permission_rejection_message("foo@foo.com", "review"), expected_messsage)
 
 
-class BugzillaErrorTest(unittest.TestCase):
-    def test_str(self):
-        self.assertEqual(BugzillaError(bug_id=12345, error_message="InvalidBugId").__str__(), "Bug 12345 is invalid")
-        self.assertEqual(BugzillaError(bug_id=12345, error_message="NotFound").__str__(), "Bug 12345 does not exist")
-        self.assertEqual(BugzillaError(bug_id=12345, error_message="NotPermitted").__str__(), "Not permitted to view bug 12345")
-        self.assertEqual(BugzillaError(bug_id=12345, error_message="Thingamajig broke").__str__(), "Bugzilla encountered an error when processing bug 12345: 'Thingamajig broke'")
-
-
 class BugzillaTest(unittest.TestCase):
     _example_attachment = '''
         <attachment
@@ -149,57 +141,6 @@ class BugzillaTest(unittest.TestCase):
         self.assertEquals(None, parse_bug_id("http://www.webkit.org/b/12345"))
         self.assertEquals(None, parse_bug_id("http://bugs.webkit.org/show_bug.cgi?ctype=xml&id=12345"))
 
-    _example_unauthorized_bug = """
-    <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
-    <!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/bugzilla.dtd">
-
-    <bugzilla version="3.2.3"
-              urlbase="https://bugs.webkit.org/"
-              maintainer="admin@webkit.org"
-              exporter="unauthorized_person@example.com"
-    >
-
-        <bug error="NotPermitted">
-          <bug_id>12345</bug_id>
-        </bug>
-
-    </bugzilla>
-    """
-
-    _example_not_found_bug = """
-    <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
-    <!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/bugzilla.dtd">
-
-    <bugzilla version="3.2.3"
-              urlbase="https://bugs.webkit.org/"
-              maintainer="admin@webkit.org"
-              exporter="unauthorized_person@example.com"
-    >
-
-        <bug error="NotFound">
-          <bug_id>12345</bug_id>
-        </bug>
-
-    </bugzilla>
-    """
-
-    _example_invalid_bug_id_bug = """
-    <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
-    <!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/bugzilla.dtd">
-
-    <bugzilla version="3.2.3"
-              urlbase="https://bugs.webkit.org/"
-              maintainer="admin@webkit.org"
-              exporter="unauthorized_person@example.com"
-    >
-
-        <bug error="InvalidBugId">
-          <bug_id>A</bug_id>
-        </bug>
-
-    </bugzilla>
-    """
-
     _example_bug = """
 <?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
 <!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/bugzilla.dtd">
@@ -287,33 +228,6 @@ ZEZpbmlzaExvYWRXaXRoUmVhc29uOnJlYXNvbl07Cit9CisKIEBlbmQKIAogI2VuZGlmCg==
         }],
     }
 
-    def test_bug_parsing_for_bugzilla_not_permitted_error(self):
-        didPassTest = False
-        try:
-            Bugzilla()._parse_bug_page(self._example_unauthorized_bug)
-        except BugzillaError, e:
-            self.assertTrue(e.is_not_permitted_to_view_bug())
-            didPassTest = True
-        self.assertTrue(didPassTest, "Failed to raise exception.")
-
-    def test_bug_parsing_for_bugzilla_not_found_error(self):
-        didPassTest = False
-        try:
-            Bugzilla()._parse_bug_page(self._example_not_found_bug)
-        except BugzillaError, e:
-            self.assertTrue(e.is_not_found())
-            didPassTest = True
-        self.assertTrue(didPassTest, "Failed to raise exception.")
-
-    def test_bug_parsing_for_bugzilla_invalid_bug_id_error(self):
-        didPassTest = False
-        try:
-            Bugzilla()._parse_bug_page(self._example_invalid_bug_id_bug)
-        except BugzillaError, e:
-            self.assertTrue(e.is_invalid_bug_id())
-            didPassTest = True
-        self.assertTrue(didPassTest, "Failed to raise exception.")
-
     # FIXME: This should move to a central location and be shared by more unit tests.
     def _assert_dictionaries_equal(self, actual, expected):
         # Make sure we aren't parsing more or less than we expect
index 34190f4..a38c3cf 100644 (file)
@@ -27,7 +27,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 from webkitpy.common.checkout.changelog import view_source_url
-from webkitpy.common.net.bugzilla import Bugzilla, BugzillaError, parse_bug_id
+from webkitpy.common.net.bugzilla import parse_bug_id
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.grammar import join_with_separators
@@ -64,13 +64,6 @@ class Sheriff(object):
             raise ScriptError(message="The rollout reason may not begin "
                               "with - (\"%s\")." % rollout_reason)
 
-        try:
-            self._tool.bugs.fetch_bug(self._tool.checkout().commit_info_for_revision(svn_revision))
-        except BugzillaError, e:
-            if (e.is_not_permitted_to_view_bug()):
-                raise ScriptError(message="SheriffBot is not authorized to view %s." % self._tool.bugs.bug_url_for_bug_id(e.bug_id()))
-            raise ScriptError(message="Could not determine the corresponding bug for r%s." % svn_revision)
-
         output = self._sheriffbot.run_webkit_patch([
             "create-rollout",
             "--force-clean",
index 5c16ac6..9916523 100644 (file)
@@ -34,7 +34,6 @@ from optparse import make_option
 import webkitpy.tool.steps as steps
 
 from webkitpy.common.checkout.changelog import ChangeLog, view_source_url
-from webkitpy.common.net.bugzilla import BugzillaError
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.commands.abstractsequencedcommand import AbstractSequencedCommand
 from webkitpy.tool.commands.stepsequence import StepSequence
@@ -158,31 +157,16 @@ class AbstractPatchSequencingCommand(AbstractPatchProcessingCommand):
 
 class ProcessAttachmentsMixin(object):
     def _fetch_list_of_patches_to_process(self, options, args, tool):
-        all_patches = []
-        for patch_id in args:
-            try:
-                all_patches.append(tool.bugs.fetch_attachment(patch_id))
-            except BugzillaError, e:
-                if options.non_interactive:
-                    raise
-                log("Failed to fetch attachment %s: '%s'; ignoring." % (patch_id, e))
-                continue
-        return all_patches
+        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
 
 
 class ProcessBugsMixin(object):
     def _fetch_list_of_patches_to_process(self, options, args, tool):
         all_patches = []
         for bug_id in args:
-            try:
-                patches = tool.bugs.fetch_bug(bug_id).reviewed_patches()
-                log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id))
-                all_patches += patches
-            except BugzillaError, e:
-                if options.non_interactive:
-                    raise
-                log("'%s'; ignoring." % e)
-                continue
+            patches = tool.bugs.fetch_bug(bug_id).reviewed_patches()
+            log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id))
+            all_patches += patches
         return all_patches
 
 
index a87ebe3..80fd2ea 100644 (file)
@@ -35,7 +35,7 @@ from datetime import datetime
 from optparse import make_option
 from StringIO import StringIO
 
-from webkitpy.common.net.bugzilla import BugzillaError, CommitterValidator
+from webkitpy.common.net.bugzilla import CommitterValidator
 from webkitpy.common.net.statusserver import StatusServer
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.common.system.deprecated_logging import error, log
@@ -353,14 +353,7 @@ class AbstractReviewQueue(AbstractPatchQueue, PersistentPatchCollectionDelegate,
     def next_work_item(self):
         patch_id = self._patches.next()
         if patch_id:
-            try:
-                return self._tool.bugs.fetch_attachment(patch_id)
-            except BugzillaError, e:
-                # This may occur if the bug with attachment ID patch_id becomes a security
-                # bug between the time we fetched all attachment IDs from the review queue
-                # and now. So, we ignore this attachment.
-                log("'%s'; ignoring." % e)
-                pass
+            return self._tool.bugs.fetch_attachment(patch_id)
 
     def should_proceed_with_work_item(self, patch):
         raise NotImplementedError, "subclasses must implement"
index e444efd..107d8db 100644 (file)
@@ -37,7 +37,7 @@ from optparse import make_option
 import webkitpy.tool.steps as steps
 
 from webkitpy.common.config.committers import CommitterList
-from webkitpy.common.net.bugzilla import BugzillaError, parse_bug_id
+from webkitpy.common.net.bugzilla import parse_bug_id
 from webkitpy.common.system.user import User
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool.commands.abstractsequencedcommand import AbstractSequencedCommand
@@ -82,14 +82,7 @@ class CleanPendingCommit(AbstractDeclarativeCommand):
     def execute(self, options, args, tool):
         committers = CommitterList()
         for bug_id in tool.bugs.queries.fetch_bug_ids_from_pending_commit_list():
-            try:
-                bug = self._tool.bugs.fetch_bug(bug_id)
-            except BugzillaError, e:
-                # This may occur if the bug becomes a security bug between the time
-                # we fetched its ID from the pending commit list and now. We ignore
-                # this bug.
-                log(e)
-                continue
+            bug = self._tool.bugs.fetch_bug(bug_id)
             patches = bug.patches(include_obsolete=True)
             for patch in patches:
                 flags_to_clear = self._flags_to_clear_on_patch(patch)
@@ -111,11 +104,7 @@ class AssignToCommitter(AbstractDeclarativeCommand):
 
     def _assign_bug_to_last_patch_attacher(self, bug_id):
         committers = CommitterList()
-        try:
-            bug = self._tool.bugs.fetch_bug(bug_id)
-        except BugzillaError, e:
-            log(e)
-            return
+        bug = self._tool.bugs.fetch_bug(bug_id)
         if not bug.is_unassigned():
             assigned_to_email = bug.assigned_to_email()
             log("Bug %s is already assigned to %s (%s)." % (bug_id, assigned_to_email, committers.committer_by_email(assigned_to_email)))
index 153b470..e77bc24 100644 (file)
@@ -28,7 +28,6 @@
 
 from webkitpy.tool.steps.abstractstep import AbstractStep
 from webkitpy.tool.steps.options import Options
-from webkitpy.common.net.bugzilla import BugzillaError
 from webkitpy.common.system.deprecated_logging import log
 
 
@@ -44,11 +43,7 @@ class CloseBug(AbstractStep):
             return
         # Check to make sure there are no r? or r+ patches on the bug before closing.
         # Assume that r- patches are just previous patches someone forgot to obsolete.
-        try:
-            patches = self._tool.bugs.fetch_bug(state["patch"].bug_id()).patches()
-        except BugzillaError, e:
-            log(e)
-            return
+        patches = self._tool.bugs.fetch_bug(state["patch"].bug_id()).patches()
         for patch in patches:
             if patch.review() == "?" or patch.review() == "+":
                 log("Not closing bug %s as attachment %s has review=%s.  Assuming there are more patches to land from this bug." % (patch.bug_id(), patch.id(), patch.review()))
index 4f52249..de508c6 100644 (file)
@@ -29,7 +29,6 @@
 from webkitpy.tool.grammar import pluralize
 from webkitpy.tool.steps.abstractstep import AbstractStep
 from webkitpy.tool.steps.options import Options
-from webkitpy.common.net.bugzilla import BugzillaError
 from webkitpy.common.system.deprecated_logging import log
 
 
@@ -44,11 +43,7 @@ class ObsoletePatches(AbstractStep):
         if not self._options.obsolete_patches:
             return
         bug_id = state["bug_id"]
-        try:
-            patches = self._tool.bugs.fetch_bug(bug_id).patches()
-        except BugzillaError, e:
-            log(e)
-            return
+        patches = self._tool.bugs.fetch_bug(bug_id).patches()
         if not patches:
             return
         log("Obsoleting %s on bug %s" % (pluralize("old patch", len(patches)), bug_id))
index 2bd5f20..ce04024 100644 (file)
@@ -29,7 +29,6 @@
 import os
 
 from webkitpy.common.checkout.changelog import ChangeLog
-from webkitpy.common.net.bugzilla import BugzillaError
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.steps.abstractstep import AbstractStep
 from webkitpy.tool.steps.options import Options
@@ -55,7 +54,7 @@ class PrepareChangeLog(AbstractStep):
             changelog = ChangeLog(changelog_path)
             if not changelog.latest_entry().bug_id():
                 changelog.set_short_description_and_bug_url(
-                    self.cached_lookup(state, "bug_title"),  # Will raise an exception if not authorized to retrieve the bug title.
+                    self.cached_lookup(state, "bug_title"),
                     self._tool.bugs.bug_url_for_bug_id(bug_id))
 
     def run(self, state):
index b0286a0..e46b790 100644 (file)
@@ -32,7 +32,6 @@ from webkitpy.common.checkout.changelog import ChangeLog
 from webkitpy.tool.grammar import pluralize
 from webkitpy.tool.steps.abstractstep import AbstractStep
 from webkitpy.tool.steps.options import Options
-from webkitpy.common.net.bugzilla import BugzillaError
 from webkitpy.common.system.deprecated_logging import log, error
 
 class UpdateChangeLogsWithReviewer(AbstractStep):
@@ -44,11 +43,7 @@ class UpdateChangeLogsWithReviewer(AbstractStep):
         ]
 
     def _guess_reviewer_from_bug(self, bug_id):
-        try:
-            patches = self._tool.bugs.fetch_bug(bug_id).reviewed_patches()
-        except BugzillaError, e:
-            log(e)
-            return None
+        patches = self._tool.bugs.fetch_bug(bug_id).reviewed_patches()
         if len(patches) != 1:
             log("%s on bug %s, cannot infer reviewer." % (pluralize("reviewed patch", len(patches)), bug_id))
             return None