-
Notifications
You must be signed in to change notification settings - Fork 369
Add post log parser to look for repeated test runs and annotate as in… #8696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
this is a WIP with an intentional failure where I would add unittests. I don't want to spend the time adding tests until I know this is something we want. |
9f30375
to
52cf693
Compare
I have tests (I feel they are hacky), and need to review my code to make it more presentable. There are a couple missing things: before landing/review:
before production (ideally):
|
latest code is ready for review and will run everywhere except production. |
@Archaeopteryx I have added another commit to include the new failure classification id, sort of using https://github.com/mozilla/treeherder/blame/3f44c1dbf610206b123c2c3c466e2be5d8da6176/treeherder/model/fixtures/failure_classification.json#L38 as a template. |
continue | ||
if counter < 0: | ||
continue | ||
if current_job.repository.id == 77 and counter >= 20: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition will never be true because the elif
evaluates to True
for a lower counter
value.
# get list of pushes | ||
ids = Push.objects.filter(repository__id=current_job.repository.id).values("id")[:20] | ||
# get list of pushes, find the current push and recent pushes | ||
idlist = Push.objects.filter(repository__id=current_job.repository.id).values("id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will return a list of hundred thousands of pushes for autoland. Possible solutions:
order_by("-id")[-100]
or similar, discard "current" pushes which have a lower ID. Let thefor
loop run.- A union of the current push ID with the push IDs of queryset for the latest pushes before and the earliest after.
|
||
all_groups = ( | ||
Group.objects.filter( | ||
job_logs__job__push__id__in=ids, | ||
job_logs__job__push__id__range=(ids[-1],ids[0]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ids
has no guaranteed order. This looks risky. Can ids
be explicitly ordered before?
@@ -54,7 +41,7 @@ def check_and_mark_intermittent(job_id): | |||
"job_logs__job__job_type__name", | |||
"job_logs__job__push__id", | |||
) | |||
.order_by("-job_logs__job__push__id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change? ID as primary key should be indexed and faster than time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to "Request changes" for the comments added today.
…termittent.