Closed Bug 591563 Opened 12 years ago Closed 12 years ago

try chooser gacks on invalid platforms

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: lsblakk)

References

Details

Attachments

(1 file, 3 obsolete files)

Callek ran into this in http://hg.mozilla.org/try/rev/ea903b1d5371 and http://hg.mozilla.org/try/rev/b06a3cc530a8 .  I wasn't sure why builds weren't firing until I looked at the scheduler master's twistd.log:

2010-08-27 21:24:08-0700 [HTTPPageGetter,client] Unhandled error in Deferred:
2010-08-27 21:24:08-0700 [HTTPPageGetter,client] Unhandled Error
        Traceback (most recent call last):          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-
py2.6-linux-i686.egg/twisted/web/client.py", line 158, in handleResponse
            self.factory.page(response)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/web/client.py", line 323, in page
            self.deferred.callback(page)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 238, in callback
            self._startRunCallbacks(result)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 307, in _startRunCallbacks
            self._runCallbacks()
        --- <exception caught here> ---          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-
py2.6-linux-i686.egg/twisted/internet/defer.py", line 323, in _runCallbacks
            self.result = callback(self.result, *args, **kw)          File "/tools/buildbotcustom/buildbotcustom/misc_scheduler.py", line 36
, in parseDataError            parseData("", c)
          File "/tools/buildbotcustom/buildbotcustom/misc_scheduler.py", line 23
, in parseData
            customBuilders = TryParser(c.comments, s.builderNames)
          File "/tools/buildbotcustom/buildbotcustom/try_parser.py", line 154, i
n TryParser
            builderNames, options.build))          File "/tools/buildbotcustom/buildbotcustom/try_parser.py", line 71, in
 getTestBuilders
            custom_builder = "%s tryserver %s %s %s" % (PRETTY_NAMES[platform], buildType, testType, test)
        exceptions.KeyError: u'win64'

It looks like it's parsing utf-8 and responding in the usual Python way: gacking completely.

I wasn't sure why it was looking for utf-8 but Callek said he changed his hg to handle utf-8 to properly handle people's names.

I *think* if we str() the comment before parsing, or

    try:
        # parse log
    except KeyError:
        # do defaults

We may be able to at least not fail to do anything.

I think the fastest workaround here is to turn off utf-8 in Callek's hg, repush, and turn it back on, but in the meantime I can look to see if the above two solutions work.
Wait.

a) try_parser already passes the message through str():
http://hg.mozilla.org/build/buildbotcustom/file/c1848384d8df/try_parser.py#l14

b) there is no win64:
http://hg.mozilla.org/build/buildbotcustom/file/c1848384d8df/valid_builders.py

There are win764 tests, but no win64 builds.

I'm going to guess that's the problem here, along with not failing gracefully.
Summary: try chooser gacks on comments encoded utf-8 → try chooser gacks on invalid platforms
For posterity I just did a push, without win64 as a platform, and without a newline in the commit message.

http://hg.mozilla.org/try/rev/dd1a7f0cb08d

Though, if we are erroring based on win64 being in the platform list, as far as I understood the plan we should be falling back to the default rules for try.
FWIW, there are lots of errors like comment #0 in our emails about exceptions.
Assignee: nobody → lsblakk
Blocks: 473184
And again on a push this morning:

2010-09-03 05:48:09-0700 [HTTPPageGetter,client] adding change, who fdesre@mozilla.com, 2 files, rev=6658c5cd5d374aa7c4c81101f685dcb33708c50f, branch=try, repository=, comments mobile webapps support try: --p none --m all --u none --t none", category None
2010-09-03 05:48:09-0700 [HTTPPageGetter,client] last changeset 6658c5cd5d374aa7c4c81101f685dcb33708c50f on http://hg.mozilla.org/try
2010-09-03 05:48:09-0700 [HTTPPageGetter,client] Stopping factory <HTTPClientFactory: http://hg.mozilla.org/try/json-pushes?full=1&fromchange=6128f3633570234d9479c0b40ddf435adb441656&tipsonly=1>
2010-09-03 05:48:10-0700 [-] Looking at changes: [<buildbot.changes.changes.Change instance at 0x9f6220c>, <buildbot.changes.changes.Change instance at 0x9e89e8c>]
2010-09-03 05:48:10-0700 [-] Starting factory <HTTPClientFactory: http://hg.mozilla.org/try/rev/6128f3633570234d9479c0b40ddf435adb441656>
2010-09-03 05:48:10-0700 [-] Starting factory <HTTPClientFactory: http://hg.mozilla.org/try/rev/6658c5cd5d374aa7c4c81101f685dcb33708c50f>
2010-09-03 05:48:10-0700 [HTTPPageGetter,client] Couldn't parse data: Requesting default try set.
2010-09-03 05:48:10-0700 [HTTPPageGetter,client] Unhandled error in Deferred:
2010-09-03 05:48:10-0700 [HTTPPageGetter,client] Unhandled Error
        Traceback (most recent call last):
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/web/client.py", line 158, in handleResponse
            self.factory.page(response)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/web/client.py", line 323, in page
            self.deferred.callback(page)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 238, in callback
            self._startRunCallbacks(result)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 307, in _startRunCallbacks
            self._runCallbacks()
        --- <exception caught here> ---
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 323, in _runCallbacks
            self.result = callback(self.result, *args, **kw)
          File "/tools/buildbotcustom/buildbotcustom/misc_scheduler.py", line 39, in parseDataError
            parseData("", c)
          File "/tools/buildbotcustom/buildbotcustom/misc_scheduler.py", line 26, in parseData
            customBuilders = TryParser(c.comments, s.builderNames)
          File "/tools/buildbotcustom/buildbotcustom/try_parser.py", line 157, in TryParser
            builderNames, options.build))
          File "/tools/buildbotcustom/buildbotcustom/try_parser.py", line 70, in getTestBuilders
            custom_builder = "%s tryserver %s %s %s" % (PRETTY_NAMES[platform], buildType, testType, test)
        exceptions.KeyError: u'n'

This happens repeatedly for the same revision, so it looks like the scheduler gets stuck processing the change for a while?
Priority: -- → P2
The problem is in misc_scheduler.py.  In parseDataError, we call parseData("", c), but now parseData looks for comments in c.comments, so an exception is raised again.

We need to restructure this whole function to look for comments in c.comments first, then do the getPage to hit hg.m.o if necessary, then pass the comments to parseData.
Tested this on sm02, threw it a comment and a no-comment and got the right customBuilders back.  Tried it with malformed syntax and got the default set.  Anything else I should be testing against this? Is the u'' stuff an issue?
Attachment #474112 - Flags: review?(catlee)
now with the much cleaner and neater json so that parseData just calls to TryParser with a string.
Attachment #474112 - Attachment is obsolete: true
Attachment #474287 - Flags: review?(catlee)
Attachment #474112 - Flags: review?(catlee)
Comment on attachment 474287 [details] [diff] [review]
[tested] misc_scheduler.py adjusted to look for sendchange comments first, then json for commit comments

>diff --git a/misc_scheduler.py b/misc_scheduler.py
>--- a/misc_scheduler.py
>+++ b/misc_scheduler.py
>@@ -1,58 +1,64 @@
> # Additional Scheduler functions
> # Contributor(s):
> #   Chris AtLee <catlee@mozilla.com>
> #   Lukas Blakk <lsblakk@mozilla.com>
> from twisted.python import log
> from twisted.internet import defer
> from twisted.web.client import getPage
> 
>-import re
>+import re, urllib2, json

json should be imported as 'import buildbot.util.json import json'; it does various checks to make sure the json module is compatible.


>+    def parseData(comments, c):
>+        if not comments:
>+            # still need to parse a comment string to get the default set
>+            comments = ""
>+        customBuilders = TryParser(comments, s.builderNames)
>         buildersPerChange[c] = customBuilders
> 
>     def parseDataError(failure, c):
>         log.msg("Couldn't parse data: Requesting default try set.")
>         parseData("", c)
> 
>     for c in all_changes:
>       try:
>         match = re.search("try", c.branch)
>         if not match:
>             log.msg("Ignoring off-branch %s" % c.branch)
>             continue
>-        # Find out stuff!
>-        d = getPage(str("http://hg.mozilla.org/try/rev/%s" % c.revision))
>+        # Look in comments first for try: syntax
>+        match = re.search("try:", c.comments)
>+        if match:
>+            d = defer.succeed(c.comments)
>+        # otherwise getPage from hg.m.o
>+        else:
>+            req = urllib2.Request( "http://hg.mozilla.org/users/lsblakk_mozilla.com/try-staging/json-pushes?full=1&changeset=%s" % c.revision)
>+            opener = urllib2.build_opener()
>+            data = opener.open(req)
>+            push = json.load(data)
>+            for p in push:
>+              pd = push[p]
>+              changes = pd['changesets']
>+              for change in changes:
>+                match = re.search("try", change['desc'])
>+                if match:
>+                  d = defer.succeed(change['desc'])

We can't use urllib2 here, since the whole master will hang waiting for this request to finish.  We can still use getPage, and then pass the results to json.loads instead of json.load.
Attachment #474287 - Flags: review?(catlee) → review-
ran this in staging and got my comments out of hg commit message.
Attachment #474287 - Attachment is obsolete: true
Attachment #475174 - Flags: review?(catlee)
Comment on attachment 475174 [details] [diff] [review]
[tested] misc_scheduler.py adjusted to look for sendchange comments first, then json for commit comments (now with getPage and buildbot.util json)

>+    def getJSON(data):
>+        push = json.loads(data)
>+        for p in push:
>+            pd = push[p]
>+            changes = pd['changesets']
>+            for change in changes:
>+                match = re.search("try", change['desc'])
>+                if match:
>+                    d = defer.succeed(change['desc'])

Did you test this bit?  I think this code primarily gets run for scheduler for the test masters.  I think you want to be doing "return change['desc']" instead of "d = defer.succeed(change['desc'])" here.

The rest looks good.
Attachment #475174 - Flags: review?(catlee) → review-
ok, returning now from getJSON instead of deferring to nowhere.  also added a couple more output lines so we can see in the log where the comment that is passed to tryparser is generated from (change comments vs. json)
Attachment #475174 - Attachment is obsolete: true
Attachment #475547 - Flags: review?(catlee)
Comment on attachment 475547 [details] [diff] [review]
[tested] misc_scheduler.py adjusted to look for sendchange comments first, then json for commit comments (now with getPage and buildbot.util json & less deferred-ness)

>+
>+    def getJSON(data):
>+        push = json.loads(data)
>+        log.msg("Looking at the push json data for try comments")
>+        for p in push:
>+            pd = push[p]
>+            changes = pd['changesets']
>+            for change in changes:
>+                match = re.search("try", change['desc'])

Looks good!  The only thing I'd change is to make this search for "try:" instead of just "try".
Attachment #475547 - Flags: review?(catlee) → review+
No longer blocks: 473184
Comment on attachment 475547 [details] [diff] [review]
[tested] misc_scheduler.py adjusted to look for sendchange comments first, then json for commit comments (now with getPage and buildbot.util json & less deferred-ness)

http://hg.mozilla.org/build/buildbotcustom/rev/787aaa855dd1

updated the regex match for json to "try:" before commit.
Attachment #475547 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.