Skip to content

Commit 56e242a

Browse files
Xavier-Dod-fence
authored andcommittedNov 14, 2024
[IMP] runbot: refactor build error models
The initial idea to link an error to another one was a quick solution to group them if they where related, but this became challenging to copute metada regarding errors. - The displayed error message was not always consistent with the real root cause/the error that lead here. - The aggregates (lets says, linked buils ids) could be the one of the error, or from all error messages. Same for the versions, first seen, .. This is confusing to knwo what is the leist we are managing and what is the expecte result to display Main motivation: on a standard error page (will be changed to "assignment"), we want to have the list of error message that is related to this one. We want to know for each message (a real build error) what is the version, first seen, ... This will give more flexibility on the display, The assigned person/team/test-tags, ... are moved to this model The appearance data remains on the build error but are aggregate on the assignation.
1 parent d990b39 commit 56e242a

20 files changed

+945
-461
lines changed
 

‎runbot/__manifest__.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
'author': "Odoo SA",
77
'website': "http://runbot.odoo.com",
88
'category': 'Website',
9-
'version': '5.7',
9+
'version': '5.8',
1010
'application': True,
1111
'depends': ['base', 'base_automation', 'website'],
1212
'data': [

‎runbot/controllers/frontend.py

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def response_wrap(*args, **kwargs):
3030
keep_search = request.httprequest.cookies.get('keep_search', False) == '1'
3131
cookie_search = request.httprequest.cookies.get('search', '')
3232
refresh = kwargs.get('refresh', False)
33-
nb_build_errors = request.env['runbot.build.error'].search_count([('random', '=', True), ('parent_id', '=', False)])
33+
nb_build_errors = request.env['runbot.build.error'].search_count([])
3434
nb_assigned_errors = request.env['runbot.build.error'].search_count([('responsible', '=', request.env.user.id)])
3535
nb_team_errors = request.env['runbot.build.error'].search_count([('responsible', '=', False), ('team_id', 'in', request.env.user.runbot_team_ids.ids)])
3636
kwargs['more'] = more
@@ -459,7 +459,7 @@ def build_errors(self, sort=None, page=1, limit=20, **kwargs):
459459
('responsible', '=', False),
460460
('team_id', 'in', request.env.user.runbot_team_ids.ids)
461461
], order='last_seen_date desc, build_count desc')
462-
domain = [('parent_id', '=', False), ('responsible', '!=', request.env.user.id), ('build_count', '>', 1)]
462+
domain = [('responsible', '!=', request.env.user.id), ('build_count', '>', 1)]
463463
build_errors_count = request.env['runbot.build.error'].search_count(domain)
464464
url_args = {}
465465
url_args['sort'] = sort
@@ -481,7 +481,7 @@ def build_errors(self, sort=None, page=1, limit=20, **kwargs):
481481
@route(['/runbot/teams', '/runbot/teams/<model("runbot.team"):team>',], type='http', auth='user', website=True, sitemap=False)
482482
def team_dashboards(self, team=None, hide_empty=False, **kwargs):
483483
teams = request.env['runbot.team'].search([]) if not team else None
484-
domain = [('id', 'in', team.build_error_ids.ids)] if team else []
484+
domain = [('id', 'in', team.assignment_ids.ids)] if team else []
485485

486486
# Sort & Filter
487487
sortby = kwargs.get('sortby', 'count')
@@ -496,7 +496,7 @@ def team_dashboards(self, team=None, hide_empty=False, **kwargs):
496496
'not_one': {'label': 'Seen more than once', 'domain': [('build_count', '>', 1)]},
497497
}
498498

499-
for trigger in team.build_error_ids.trigger_ids if team else []:
499+
for trigger in team.assignment_ids.trigger_ids if team else []:
500500
k = f'trigger_{trigger.name.lower().replace(" ", "_")}'
501501
searchbar_filters.update(
502502
{k: {'label': f'Trigger {trigger.name}', 'domain': [('trigger_ids', '=', trigger.id)]}}
@@ -510,7 +510,7 @@ def team_dashboards(self, team=None, hide_empty=False, **kwargs):
510510
qctx = {
511511
'team': team,
512512
'teams': teams,
513-
'build_error_ids': request.env['runbot.build.error'].search(domain, order=order),
513+
'build_assignment_ids': request.env['runbot.build.assignment'].search(domain, order=order),
514514
'hide_empty': bool(hide_empty),
515515
'searchbar_sortings': searchbar_sortings,
516516
'sortby': sortby,

‎runbot/data/error_link.xml

+10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@
99
records.action_link_errors()
1010
</field>
1111
</record>
12+
<record model="ir.actions.server" id="action_link_build_errors_contents">
13+
<field name="name">Link build errors contents</field>
14+
<field name="model_id" ref="runbot.model_runbot_build_error" />
15+
<field name="binding_model_id" ref="runbot.model_runbot_build_error" />
16+
<field name="type">ir.actions.server</field>
17+
<field name="state">code</field>
18+
<field name="code">
19+
records.action_link_errors_content()
20+
</field>
21+
</record>
1222
<record model="ir.actions.server" id="action_clean_build_errors">
1323
<field name="name">Re-clean build errors</field>
1424
<field name="model_id" ref="runbot.model_runbot_build_error" />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import logging
2+
3+
_logger = logging.getLogger(__name__)
4+
5+
6+
def migrate(cr, version):
7+
8+
# get seen infos
9+
cr.execute("SELECT error_content_id, min(build_id), min(log_date), max(build_id), max(log_date), count(DISTINCT build_id) FROM runbot_build_error_link GROUP BY error_content_id")
10+
vals_by_error = {error: vals for error, *vals in cr.fetchall()}
11+
12+
# first_seen_build_id was not stored, lets fill it and update all values for good mesure
13+
for error, vals in vals_by_error.items():
14+
cr.execute('UPDATE runbot_build_error_content SET first_seen_build_id = %s, first_seen_date = %s, last_seen_build_id = %s, last_seen_date = %s WHERE id=%s', (vals[0], vals[1], vals[2], vals[3], error))
15+
16+
# generate flattened error hierarchy
17+
cr.execute('''SELECT
18+
id,
19+
parent_id
20+
FROM runbot_build_error_content
21+
ORDER BY id
22+
''')
23+
24+
error_by_parent = {}
25+
for error_id, parent_id in cr.fetchall():
26+
if parent_id:
27+
error_by_parent.setdefault(parent_id, []).append(error_id)
28+
stable = False
29+
while not stable:
30+
stable = True
31+
for parent, child_ids in error_by_parent.items():
32+
for child_id in child_ids:
33+
if parent == child_id:
34+
continue
35+
sub_childrens = error_by_parent.get(child_id)
36+
if sub_childrens:
37+
error_by_parent[parent] = error_by_parent[parent] + sub_childrens
38+
error_by_parent[child_id] = []
39+
stable = False
40+
for parent, child_ids in error_by_parent.items():
41+
if parent in child_ids:
42+
_logger.info('Breaking cycle parent on %s', parent)
43+
error_by_parent[parent] = [c for c in child_ids if c != parent]
44+
cr.execute('UPDATE runbot_build_error_content SET parent_id = null WHERE id=%s', (parent,))
45+
error_by_parent = {parent: chilren for parent, chilren in error_by_parent.items() if chilren}
46+
47+
cr.execute('''SELECT
48+
id,
49+
active,
50+
parent_id
51+
random,
52+
content,
53+
test_tags,
54+
tags_min_version_id,
55+
tags_max_version_id,
56+
team_id,
57+
responsible,
58+
customer,
59+
fixing_commit,
60+
fixing_pr_id
61+
FROM runbot_build_error_content
62+
WHERE parent_id IS null
63+
ORDER BY id
64+
''')
65+
errors = cr.fetchall()
66+
nb_groups = len(error_by_parent)
67+
_logger.info('Creating %s errors', nb_groups)
68+
for error in errors:
69+
error_id, *values = error
70+
children = error_by_parent.get(error_id, [])
71+
assert not error_id in children
72+
all_errors = [error_id, *children]
73+
error_count = len(all_errors)
74+
75+
# vals_by_error order: min(build_id), min(log_date), max(build_id), max(log_date)
76+
build_count = 0
77+
first_seen_build_id = first_seen_date = last_seen_build_id = last_seen_date = None
78+
if error_id in vals_by_error:
79+
error_vals = [vals_by_error[error_id] for error_id in all_errors]
80+
first_seen_build_id = min(vals[0] for vals in error_vals)
81+
first_seen_date = min(vals[1] for vals in error_vals)
82+
last_seen_build_id = max(vals[2] for vals in error_vals)
83+
last_seen_date = max(vals[3] for vals in error_vals)
84+
build_count = sum(vals[4] for vals in error_vals) # not correct for distinct but close enough
85+
assert first_seen_date <= last_seen_date
86+
assert first_seen_build_id <= last_seen_build_id
87+
name = values[2].split('\n')[0]
88+
89+
values = [error_id, *values, last_seen_build_id, first_seen_build_id, last_seen_date, first_seen_date, build_count, error_count, name]
90+
91+
cr.execute('''
92+
INSERT INTO runbot_build_error (
93+
id,
94+
active,
95+
random,
96+
description,
97+
test_tags,
98+
tags_min_version_id,
99+
tags_max_version_id,
100+
team_id,
101+
responsible,
102+
customer,
103+
fixing_commit,
104+
fixing_pr_id,
105+
last_seen_build_id,
106+
first_seen_build_id,
107+
last_seen_date,
108+
first_seen_date,
109+
build_count,
110+
error_count,
111+
name
112+
)
113+
VALUES (%s)
114+
RETURNING id
115+
''' % ', '.join(['%s'] * len(values)), values) # noqa: S608
116+
117+
error_id = cr.fetchone()
118+
cr.execute('UPDATE runbot_build_error_content SET error_id = %s WHERE id in %s', (error_id, tuple(all_errors)))
119+
120+
cr.execute('ALTER TABLE runbot_build_error_content ALTER COLUMN error_id SET NOT NULL')
121+
cr.execute('SELECT max(id) from runbot_build_error')
122+
cr.execute("SELECT SETVAL('runbot_build_error_id_seq', %s)", (cr.fetchone()[0] + 1,))
123+
_logger.info('Done')
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
def migrate(cr, version):
2+
cr.execute('ALTER TABLE runbot_build_error RENAME TO runbot_build_error_content')
3+
cr.execute('ALTER TABLE runbot_build_error_content ADD COLUMN first_seen_build_id INT')
4+
cr.execute('ALTER TABLE runbot_build_error_link RENAME COLUMN build_error_id TO error_content_id')

‎runbot/models/batch.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,9 @@ def _fill_missing(branch_commits, match_type):
237237
# use last not preparing batch to define previous repos_heads instead of branches heads:
238238
# Will allow to have a diff info on base bundle, compare with previous bundle
239239
last_base_batch = self.env['runbot.batch'].search([('bundle_id', '=', bundle.base_id.id), ('state', '!=', 'preparing'), ('category_id', '=', self.category_id.id), ('id', '!=', self.id)], order='id desc', limit=1)
240-
base_head_per_repo = {commit.repo_id.id: commit for commit in last_base_batch.commit_ids}
241-
self._update_commits_infos(base_head_per_repo) # set base_commit, diff infos, ...
240+
if last_base_batch:
241+
base_head_per_repo = {commit.repo_id.id: commit for commit in last_base_batch.commit_ids}
242+
self._update_commits_infos(base_head_per_repo) # set base_commit, diff infos, ...
242243

243244
# 2. FIND missing commit in a compatible base bundle
244245
if bundle.is_base or auto_rebase:
@@ -496,7 +497,6 @@ class BatchSlot(models.Model):
496497
_description = 'Link between a bundle batch and a build'
497498
_order = 'trigger_id,id'
498499

499-
500500
batch_id = fields.Many2one('runbot.batch', index=True)
501501
trigger_id = fields.Many2one('runbot.trigger', index=True)
502502
build_id = fields.Many2one('runbot.build', index=True)

‎runbot/models/build.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ def _compute_global_result(self):
316316
@api.depends('build_error_link_ids')
317317
def _compute_build_error_ids(self):
318318
for record in self:
319-
record.build_error_ids = record.build_error_link_ids.mapped('build_error_id')
319+
record.build_error_ids = record.build_error_link_ids.error_content_id.error_id
320320

321321
def _get_worst_result(self, results, max_res=False):
322322
results = [result for result in results if result] # filter Falsy values
@@ -1182,11 +1182,10 @@ def _get_py_version(self):
11821182

11831183
def _parse_logs(self):
11841184
""" Parse build logs to classify errors """
1185-
BuildError = self.env['runbot.build.error']
11861185
# only parse logs from builds in error and not already scanned
11871186
builds_to_scan = self.search([('id', 'in', self.ids), ('local_result', 'in', ('ko', 'killed', 'warn')), ('build_error_link_ids', '=', False)])
11881187
ir_logs = self.env['ir.logging'].search([('level', 'in', ('ERROR', 'WARNING', 'CRITICAL')), ('type', '=', 'server'), ('build_id', 'in', builds_to_scan.ids)])
1189-
return BuildError._parse_logs(ir_logs)
1188+
return self.env['runbot.build.error']._parse_logs(ir_logs)
11901189

11911190
def _is_file(self, file, mode='r'):
11921191
file_path = self._path(file)

0 commit comments

Comments
 (0)
Please sign in to comment.