Closed
Bug 1040418
Opened 10 years ago
Closed 10 years ago
Update log parser regex & bug suggestion blacklist to match TBPL
Categories
(Tree Management :: Treeherder, defect, P1)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: camd, Assigned: emorley)
References
Details
Attachments
(6 files)
(deleted),
patch
|
camd
:
review+
emorley
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
camd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
camd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
camd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
camd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
camd
:
review+
|
Details | Diff | Splinter Review |
We have several areas where we have regular expressions for data and log parsing. The Sheriff's should maintain these files going forward to ensure that we keep up to date with changes that are needed.
Here are the files that will need maintenance. In most cases they somewhat mirror the files used in TBPL. However, the structure *is* a bit different here and there. Much of this was copied from TBPL directly, and converted to python. So you'll even see similar comments and ordering. I've done my best to keep the order the same in the various lists. Hopefully this will make sense when you look into it.
Job data that comes from buildbot:
https://github.com/mozilla/treeherder-service/blob/master/treeherder/etl/buildbot.py
Log parsing for errors:
https://github.com/mozilla/treeherder-service/blob/master/treeherder/log_parser/parsers.pyx#L250
Hopefully this will be self-explanatory enough. But if not, please don't hesitate to contact me and we can work through it together.
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(emorley)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → emorley
Flags: needinfo?(emorley)
Assignee | ||
Updated•10 years ago
|
Priority: -- → P1
Assignee | ||
Updated•10 years ago
|
Blocks: treeherder-sheriff-transition
Assignee | ||
Updated•10 years ago
|
Severity: blocker → critical
Reporter | ||
Comment 2•10 years ago
|
||
In addition to the files mentioned above when updating regexes, etc: when we update the names of the platforms, it should be done directly in this file: https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/values.js
Reporter | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
There were some changes in the TBPL symbols in bug 1013691 that are yet to be reflected in Treeherder.
Assignee | ||
Comment 6•10 years ago
|
||
UI-side...
As of:
https://github.com/mozilla/treeherder-ui/blob/13088bae0aee302bf0bbb8cdb4635f351581c292/webapp/app/js/values.js
the file is up to date compared to:
https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/js/Config.js#l345
Looking at service next.
Assignee | ||
Comment 7•10 years ago
|
||
We have three architectures for our Android builds: armv7, armv6 and x86.
The armv6 jobs were mostly already correct, but many of the armv7 ones were previously listed as x86.
Note that in addition to this, we currently don't differentiate between the arch on which a build was compiled, and the arch it is intended to be run on - however I've filed bug 1056928 for that, and this patch at least makes things more consistent until then.
Attachment #8476801 -
Flags: review?(cdawson)
Assignee | ||
Comment 8•10 years ago
|
||
The tests passed when run locally.
Assignee | ||
Comment 9•10 years ago
|
||
Another file with regex:
https://github.com/mozilla/treeherder-service/blob/master/treeherder/log_parser/utils.py#L7
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8476801 [details] [diff] [review]
Part 1: Correct Android & B2G architecture
Review of attachment 8476801 [details] [diff] [review]:
-----------------------------------------------------------------
Ed, this looks great. Please feel free to merge this to master.
Attachment #8476801 -
Flags: review?(cdawson) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8476801 [details] [diff] [review]
Part 1: Correct Android & B2G architecture
https://github.com/mozilla/treeherder-service/commit/0ae443a35bf677df829d7a83c79abd5693cc8551
Assignee | ||
Updated•10 years ago
|
Attachment #8476801 -
Flags: checkin+
Assignee | ||
Comment 12•10 years ago
|
||
Fixes the following warnings:
treeherder/log_parser/parsers.pyx:217:31: E201 whitespace after '{'
treeherder/log_parser/parsers.pyx:221:1: E302 expected 2 blank lines, found 1
treeherder/log_parser/parsers.pyx:228:5: E303 too many blank lines (2)
treeherder/log_parser/parsers.pyx:249:24: E222 multiple spaces after operator
treeherder/log_parser/parsers.pyx:254:25: E125 continuation line with same indent as next logical line
treeherder/log_parser/parsers.pyx:264:51: E231 missing whitespace after ','
treeherder/log_parser/parsers.pyx:275:9: E126 continuation line over-indented for hanging indent
treeherder/log_parser/parsers.pyx:277:5: E121 continuation line under-indented for hanging indent
treeherder/log_parser/parsers.pyx:328:1: E302 expected 2 blank lines, found 1
treeherder/log_parser/parsers.pyx:350:9: E265 block comment should start with '# '
treeherder/log_parser/parsers.pyx:362:1: E302 expected 2 blank lines, found 1
treeherder/log_parser/parsers.pyx:375:32: F841 local variable 'e' is assigned to but never used
Attachment #8480720 -
Flags: review?(cdawson)
Assignee | ||
Comment 13•10 years ago
|
||
Terms in IN_SEARCH_TERMS are only used in a 'in' membership check, which only does a simple substring match. This moves terms that contain regex to RE_ERR_SEARCH which actually will use them in a re.search() (some were listed in both).
Attachment #8480731 -
Flags: review?(cdawson)
Reporter | ||
Updated•10 years ago
|
Attachment #8480720 -
Flags: review?(cdawson) → review+
Reporter | ||
Comment 14•10 years ago
|
||
Comment on attachment 8480731 [details] [diff] [review]
Part 3: Move log error terms containing regex out of the simple substring list
Ed-- Is line 286 also regex?
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Cameron Dawson [:camd] from comment #14)
> Ed-- Is line 286 also regex?
No, but the escaping there is both unnecessary and causing that term to not work as expected (one of my local changes fixes this) :-)
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8480751 -
Flags: review?(cdawson)
Reporter | ||
Updated•10 years ago
|
Attachment #8480731 -
Flags: review?(cdawson) → review+
Reporter | ||
Updated•10 years ago
|
Attachment #8480751 -
Flags: review?(cdawson) → review+
Assignee | ||
Comment 17•10 years ago
|
||
The treeherder equivalent of:
https://hg.mozilla.org/webtools/tbpl/rev/03d9ff82b54a
https://hg.mozilla.org/webtools/tbpl/rev/9941c86059cb
https://hg.mozilla.org/webtools/tbpl/rev/c6d684e3be13
https://hg.mozilla.org/webtools/tbpl/rev/611685ea448d
https://hg.mozilla.org/webtools/tbpl/rev/ae716547090e
TBPL now uses "/fatal error/i", but the two cases we are interested in are either all uppercase, or all lowercase - so I went with two entries in IN_SEARCH_TERMS rather than using (?i) in RE_ERR_SEARCH. Let me know if you'd prefer the latter instead.
Attachment #8480793 -
Flags: review?(cdawson)
Assignee | ||
Comment 19•10 years ago
|
||
This patch should be a no-op - it just cleans up the expressions used.
* Switch uses of "[0-9]" to "\d"
* Switch uses of "[ ]+" to " +"
* Remove unnecessary escaping
Attachment #8480803 -
Flags: review?(cdawson)
Reporter | ||
Updated•10 years ago
|
Attachment #8480793 -
Flags: review?(cdawson) → review+
Reporter | ||
Updated•10 years ago
|
Attachment #8480803 -
Flags: review?(cdawson) → review+
Assignee | ||
Comment 20•10 years ago
|
||
All attached patches here are now landed (plus a typo followup for part 5, that isn't caught by the tests - I've filed bug 1060765 for improving test coverage):
https://github.com/mozilla/treeherder-service/commit/78b73b81ed05c7777040e5ad3cfad87ac2d3154c
https://github.com/mozilla/treeherder-service/commit/6164e51a2320bb395f5e7944d918d97d597a6993
https://github.com/mozilla/treeherder-service/commit/0a00acfd8bab1cc8cc3b110cdc936c6a6e9fc2b6
https://github.com/mozilla/treeherder-service/commit/b019c9a1b3faec711fdeeccc96f23db4241acb7a
https://github.com/mozilla/treeherder-service/commit/7d0549abeb017de2ca60cb0d89c44c78678ddc88
https://github.com/mozilla/treeherder-service/commit/34c7d78a7270335d7a2aab640e9ccbb02415e961
There's more to do with syncing job types, but I'll break that out to bug 1060763.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Summary: Sheriff's update Treeherder regexes as needed to match TBPL → Update log parser regex & bug suggestion blacklist to match TBPL
Assignee | ||
Comment 21•10 years ago
|
||
(In reply to Cameron Dawson [:camd] from comment #0)
> Here are the files that will need maintenance.
>
> Job data that comes from buildbot:
> https://github.com/mozilla/treeherder-service/blob/master/treeherder/etl/
> buildbot.py
>
> Log parsing for errors:
> https://github.com/mozilla/treeherder-service/blob/master/treeherder/
> log_parser/parsers.pyx#L250
(In reply to Cameron Dawson [:camd] from comment #2)
> In addition to the files mentioned above when updating regexes, etc: when we
> update the names of the platforms, it should be done directly in this file:
> https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/values.js
(In reply to Ed Morley [:edmorley] from comment #9)
> Another file with regex:
> https://github.com/mozilla/treeherder-service/blob/master/treeherder/
> log_parser/utils.py#L7
Also new platforms need adding to PLATFORM_ORDER:
https://github.com/mozilla/treeherder-service/blob/master/treeherder/webapp/api/resultset.py#L14
Assignee | ||
Comment 22•10 years ago
|
||
When we deploy changes on dev/staging/production, are the parsers recompiled automatically, or are manual steps required? If the latter, will just remember to remind people to re-compile when pushing to prod for future requests etc :-)
Flags: needinfo?(jeads)
Comment 23•10 years ago
|
||
The parser changes are recompiled automatically on production on every code push:
https://github.com/mozilla/treeherder-service/blob/master/deployment/update/update.py#L74
There are two bash scripts on dev/stage that carry out the same compile step automatically that we run to push new code changes. However these scripts do not automatically push code to the workers which run on separate AWS instances so that does need to be done manually. We will do this explicitly on dev/stage for these changes.
Flags: needinfo?(jeads)
Assignee | ||
Comment 24•10 years ago
|
||
Ah thank you :-)
You need to log in
before you can comment on or make changes to this bug.
Description
•