Skip to content
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

add new_failure field to TextLogError and update when we have a… #8254

Merged
merged 1 commit into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions tests/etl/test_load_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from treeherder.etl.artifact import store_job_artifacts
from treeherder.model.models import TextLogError
from treeherder.model.error_summary import get_error_summary


def test_load_textlog_summary_twice(test_repository, test_job):
Expand Down Expand Up @@ -50,8 +51,19 @@ def test_load_non_ascii_textlog_errors(test_job):
"job_guid": test_job.guid,
}

# ensure a result='failed' to treat failure as a NEW_failure
test_job.result = "testfailed"
test_job.save()

store_job_artifacts([text_log_summary_artifact])

# ensure bug_suggestions data is stored and retrieved properly
tle_all = TextLogError.objects.all()
bug_suggestions = get_error_summary(test_job)
for suggestion in bug_suggestions:
tle = next(t for t in tle_all if t.line_number == suggestion["line_number"])
assert suggestion["failure_new_in_rev"] == tle.new_failure

assert TextLogError.objects.count() == 2
assert TextLogError.objects.get(line_number=1587).line == "07:51:28 WARNING - \U000000c3"
assert TextLogError.objects.get(line_number=1588).line == "07:51:29 WARNING - <U+01D400>"
2 changes: 2 additions & 0 deletions tests/webapp/api/test_jobs_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,14 @@ def test_text_log_errors(client, test_job):
"job": 1,
"line": "failure 1",
"line_number": 101,
"new_failure": False,
},
{
"id": 2,
"job": 1,
"line": "failure 2",
"line_number": 102,
"new_failure": False,
},
]

Expand Down
11 changes: 10 additions & 1 deletion treeherder/etl/artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ def store_text_log_summary_artifact(job, text_log_summary_artifact):
ignore_conflicts=True,
)

# Bulk create doesn't return .id field, so query to get them.
log_errors = TextLogError.objects.filter(job=job)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note, this could be a perf impact, we could reconsider this specific approach, maybe only do this if we have a need to update, or convert bulk_create -> save() in a loop


# get error summary immediately (to warm the cache)
# Conflicts may have occured during the insert, but we pass the queryset for performance
bugs = error_summary.get_error_summary(job, queryset=log_errors)
Expand All @@ -42,7 +45,13 @@ def store_text_log_summary_artifact(job, text_log_summary_artifact):
]:
# classify job as `new failure` - for filtering, etc.
job.failure_classification_id = 6
job.save()
job.save(update_fields=["failure_classification_id"])
# for every log_errors (TLE object) there is a corresponding bugs/suggestion
for tle in log_errors:
if tle.line_number == suggestion["line_number"]:
tle.new_failure = True
tle.save(update_fields=["new_failure"])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using update_fields is a perf win, so I get a little bit of a win here for the existing code :)

break


def store_job_artifacts(artifact_data):
Expand Down
21 changes: 21 additions & 0 deletions treeherder/model/migrations/0033_textlogerror_new_failure.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Generated by Django 4.2.13 on 2024-10-21 13:53

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
(
"model",
"0032_rename_failureline_job_guid_repository_failure_lin_job_gui_b67c6d_idx_and_more",
),
]

operations = [
migrations.AddField(
model_name="textlogerror",
name="new_failure",
field=models.BooleanField(default=False),
),
]
1 change: 1 addition & 0 deletions treeherder/model/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,7 @@ class TextLogError(models.Model):
job = models.ForeignKey(Job, on_delete=models.CASCADE, related_name="text_log_error", null=True)
line = models.TextField()
line_number = models.PositiveIntegerField()
new_failure = models.BooleanField(default=False)

# TODO delete this field and unique_together once backfill of jobs in TextLogError table has been completed
step = models.ForeignKey(
Expand Down