Closed
Bug 1342377
Opened 8 years ago
Closed 8 years ago
store-failure-lines FailureLine.objects.create() "TypeError: 'group' is an invalid keyword argument for this function"
Categories
(Tree Management :: Treeherder: Data Ingestion, defect, P2)
Tree Management
Treeherder: Data Ingestion
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: jgraham)
References
Details
Attachments
(2 files)
This log contains a new `group` key in each failure line:
https://public-artifacts.taskcluster.net/bjdwfJjOQx-worBDrGCoLQ/0/public/test_info//valgrind-plain_errorsummary.log
(Presumably the output of a try run for bug 1340551)
These lines are handled via:
def create(job_log, log_list):
failure_lines = [
FailureLine.objects.create(repository=job_log.job.repository,
job_guid=job_log.job.guid,
job_log=job_log,
**failure_line)
for failure_line in log_list]
job_log.update_status(JobLog.PARSED)
return failure_lines
(See https://github.com/mozilla/treeherder/blob/495fc898fd29c7fe760aef93fe9151735433e70d/treeherder/log_parser/failureline.py#L84-L92)
Using `**failure_line` seems error-prone.
The fields should be listed explicitly given it's user-provided data.
Flags: needinfo?(james)
Reporter | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
Just to note, in bug 1340551 it looks like we're going the route of printing all tests and their groups at the start of the error summary, rather than adding a new 'group' key to each log line. That may or may not make this bug invalid.
Reporter | ||
Comment 3•8 years ago
|
||
I believe the bug is still valid.
There are two ways we decide to treat the failure line schema:
1) As immutable
2) As something that will evolve over time via bugs/try pushes similar to that one
For case one, we presumably want to skip parsing the log, but (a) the error message should be clearer, (b) this shouldn't result in an exception in New Relic.
For case two, new properties will require schema changes in Treeherder, so can't be auto-ingested even if we wanted, so the parser should gracefully handle them prior to someone following up with a Treeherder bug to add the new feature.
Assignee | ||
Comment 4•8 years ago
|
||
Yes, this is a real bug. I will fix it. Having said that changing the failure_line schema in the database is super-painful; I'm not sure what to do about that if we start adding more information here.
Flags: needinfo?(james)
Comment 5•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8840920 -
Flags: review?(emorley)
Reporter | ||
Comment 6•8 years ago
|
||
Comment on attachment 8840920 [details] [review]
[treeherder] mozilla:failure_line_allow_list > mozilla:master
Thanks! :-)
Attachment #8840920 -
Flags: review?(emorley) → review+
Comment 7•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/75b64d7c1cee5620f419a66a8d14ce2b23bdb9e3
Bug 1342377 - Only store failure_line data from known errorsummary keys (#2200)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → james
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 8•8 years ago
|
||
Bug 1340551 has now landed, which has caused ~3000 exceptions on prod in the last 3 hours, which is only going to increase as it gets merged around.
I'll deploy to prod now.
Comment 9•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Attachment #8842245 -
Flags: review+
Comment 10•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/b6fd5778bb3e8ad0c0de8a43aa6d7ffb9c445374
Bug 1342377 - Ignore unsupported actions when crossreferencing error lines (#2210)
You need to log in
before you can comment on or make changes to this bug.
Description
•