Closed
Bug 591563
Opened 14 years ago
Closed 14 years ago
try chooser gacks on invalid platforms
Categories
(Release Engineering :: General, defect, P2)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mozilla, 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.
Reporter | ||
Comment 1•14 years ago
|
||
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.
Reporter | ||
Comment 2•14 years ago
|
||
Fixed the wiki: https://wiki.mozilla.org/index.php?title=Build:TryChooser&diff=249209&oldid=249189
Reporter | ||
Updated•14 years ago
|
Summary: try chooser gacks on comments encoded utf-8 → try chooser gacks on invalid platforms
Comment 3•14 years ago
|
||
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.
Comment 4•14 years ago
|
||
FWIW, there are lots of errors like comment #0 in our emails about exceptions.
Comment 5•14 years ago
|
||
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
Comment 6•14 years ago
|
||
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.
Assignee | ||
Comment 7•14 years ago
|
||
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)
Assignee | ||
Comment 8•14 years ago
|
||
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 9•14 years ago
|
||
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-
Assignee | ||
Comment 10•14 years ago
|
||
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 11•14 years ago
|
||
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-
Assignee | ||
Comment 12•14 years ago
|
||
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 13•14 years ago
|
||
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+
Assignee | ||
Updated•14 years ago
|
Blocks: releng-downtime
Assignee | ||
Comment 14•14 years ago
|
||
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+
Assignee | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•