From c7b9bc53211d4c4ade3e4eccf0339745af2a6bac Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Fri, 8 Sep 2023 17:07:22 +0200 Subject: [PATCH] [IMP] runbot: use real log date on build errors In the build error view, a list of build is displayed with a confusing create date. The create date in the list is the creation date of the build, leading to a confusion with the creation of the build log creation. With this commit, the real log creation is used in this view. To achieve that, the many2many relation is extended with a log_date which is filled when a build log entry is parsed. --- runbot/__manifest__.py | 4 +- runbot/migrations/16.0.5.5/post-migration.py | 8 +++ runbot/models/build.py | 10 ++- runbot/models/build_error.py | 65 +++++++++++++++++--- runbot/security/ir.model.access.csv | 3 + runbot/tests/test_build_error.py | 33 +++++++--- runbot/views/build_error_views.xml | 5 +- 7 files changed, 106 insertions(+), 22 deletions(-) create mode 100644 runbot/migrations/16.0.5.5/post-migration.py diff --git a/runbot/__manifest__.py b/runbot/__manifest__.py index 6ad3c006..83ef4420 100644 --- a/runbot/__manifest__.py +++ b/runbot/__manifest__.py @@ -2,11 +2,11 @@ { 'name': "runbot", 'summary': "Runbot", - 'description': "Runbot for Odoo 15.0", + 'description': "Runbot for Odoo 16.0", 'author': "Odoo SA", 'website': "http://runbot.odoo.com", 'category': 'Website', - 'version': '5.4', + 'version': '5.5', 'application': True, 'depends': ['base', 'base_automation', 'website'], 'data': [ diff --git a/runbot/migrations/16.0.5.5/post-migration.py b/runbot/migrations/16.0.5.5/post-migration.py new file mode 100644 index 00000000..0625ce8f --- /dev/null +++ b/runbot/migrations/16.0.5.5/post-migration.py @@ -0,0 +1,8 @@ +def migrate(cr, version): + cr.execute( + """ + INSERT INTO runbot_build_error_link(build_id,build_error_id,log_date) + SELECT runbot_build_id,runbot_build_error_id,runbot_build.create_date as create_date + FROM runbot_build_error_ids_runbot_build_rel + LEFT JOIN runbot_build ON runbot_build.id = runbot_build_id; + """) diff --git a/runbot/models/build.py b/runbot/models/build.py index 645dd125..e20f87be 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -219,7 +219,8 @@ class BuildResult(models.Model): orphan_result = fields.Boolean('No effect on the parent result', default=False) build_url = fields.Char('Build url', compute='_compute_build_url', store=False) - build_error_ids = fields.Many2many('runbot.build.error', 'runbot_build_error_ids_runbot_build_rel', string='Errors') + build_error_link_ids = fields.One2many('runbot.build.error.link', 'build_id') + build_error_ids = fields.Many2many('runbot.build.error', compute='_compute_build_error_ids', string='Errors') keep_running = fields.Boolean('Keep running', help='Keep running', index=True) log_counter = fields.Integer('Log Lines counter', default=100) @@ -311,6 +312,11 @@ class BuildResult(models.Model): else: record.global_result = record.local_result + @api.depends('build_error_link_ids') + def _compute_build_error_ids(self): + for record in self: + record.build_error_ids = record.build_error_link_ids.mapped('build_error_id') + def _get_worst_result(self, results, max_res=False): results = [result for result in results if result] # filter Falsy values index = max([self._get_result_score(result) for result in results]) if results else 0 @@ -1128,7 +1134,7 @@ class BuildResult(models.Model): """ Parse build logs to classify errors """ BuildError = self.env['runbot.build.error'] # only parse logs from builds in error and not already scanned - builds_to_scan = self.search([('id', 'in', self.ids), ('local_result', 'in', ('ko', 'killed', 'warn')), ('build_error_ids', '=', False)]) + builds_to_scan = self.search([('id', 'in', self.ids), ('local_result', 'in', ('ko', 'killed', 'warn')), ('build_error_link_ids', '=', False)]) ir_logs = self.env['ir.logging'].search([('level', 'in', ('ERROR', 'WARNING', 'CRITICAL')), ('type', '=', 'server'), ('build_id', 'in', builds_to_scan.ids)]) return BuildError._parse_logs(ir_logs) diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py index a9b9acca..d1072c8c 100644 --- a/runbot/models/build_error.py +++ b/runbot/models/build_error.py @@ -4,6 +4,8 @@ import hashlib import logging import re +from psycopg2 import sql + from collections import defaultdict from dateutil.relativedelta import relativedelta from werkzeug.urls import url_join @@ -13,6 +15,26 @@ from odoo.exceptions import ValidationError, UserError _logger = logging.getLogger(__name__) +class BuildErrorLink(models.Model): + _name = 'runbot.build.error.link' + _description = 'Build Build Error Extended Relation' + _order = 'log_date desc, build_id desc' + + build_id = fields.Many2one('runbot.build', required=True, index=True) + build_error_id =fields.Many2one('runbot.build.error', required=True, index=True) + log_date = fields.Datetime(string='Log date') + host = fields.Char(related='build_id.host') + dest = fields.Char(related='build_id.dest') + version_id = fields.Many2one(related='build_id.version_id') + trigger_id = fields.Many2one(related='build_id.trigger_id') + description = fields.Char(related='build_id.description') + build_url = fields.Char(related='build_id.build_url') + + _sql_constraints = [ + ('error_build_rel_unique', 'UNIQUE (build_id, build_error_id)', 'A link between a build and an error must be unique'), + ] + + class BuildError(models.Model): _name = "runbot.build.error" @@ -35,7 +57,9 @@ class BuildError(models.Model): fixing_pr_id = fields.Many2one('runbot.branch', 'Fixing PR', tracking=True, domain=[('is_pr', '=', True)]) fixing_pr_alive = fields.Boolean('Fixing PR alive', related='fixing_pr_id.alive') fixing_pr_url = fields.Char('Fixing PR url', related='fixing_pr_id.branch_url') - build_ids = fields.Many2many('runbot.build', 'runbot_build_error_ids_runbot_build_rel', string='Affected builds') + build_error_link_ids = fields.One2many('runbot.build.error.link', 'build_error_id') + children_build_error_link_ids = fields.One2many('runbot.build.error.link', compute='_compute_children_build_error_link_ids') + build_ids = fields.Many2many('runbot.build', compute= '_compute_build_ids') bundle_ids = fields.One2many('runbot.bundle', compute='_compute_bundle_ids') version_ids = fields.One2many('runbot.version', compute='_compute_version_ids', string='Versions', search='_search_version') trigger_ids = fields.Many2many('runbot.trigger', compute='_compute_trigger_ids', string='Triggers', search='_search_trigger_ids') @@ -47,9 +71,9 @@ class BuildError(models.Model): children_build_ids = fields.Many2many('runbot.build', compute='_compute_children_build_ids', string='Children builds') error_history_ids = fields.Many2many('runbot.build.error', compute='_compute_error_history_ids', string='Old errors', context={'active_test': False}) first_seen_build_id = fields.Many2one('runbot.build', compute='_compute_first_seen_build_id', string='First Seen build') - first_seen_date = fields.Datetime(string='First Seen Date', related='first_seen_build_id.create_date', depends=['first_seen_build_id']) + first_seen_date = fields.Datetime(string='First Seen Date', compute='_compute_seen_date', store=True) last_seen_build_id = fields.Many2one('runbot.build', compute='_compute_last_seen_build_id', string='Last Seen build', store=True) - last_seen_date = fields.Datetime(string='Last Seen Date', related='last_seen_build_id.create_date', store=True, depends=['last_seen_build_id']) + last_seen_date = fields.Datetime(string='Last Seen Date', compute='_compute_seen_date', store=True) test_tags = fields.Char(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True) @api.constrains('test_tags') @@ -105,6 +129,16 @@ class BuildError(models.Model): build_error.team_id = False return result + @api.depends('build_error_link_ids') + def _compute_build_ids(self): + for record in self: + record.build_ids = record.build_error_link_ids.mapped('build_id') + + @api.depends('build_error_link_ids') + def _compute_children_build_error_link_ids(self): + for record in self: + record.children_build_error_link_ids = record.build_error_link_ids | record.child_ids.build_error_link_ids + @api.depends('build_ids', 'child_ids.build_ids') def _compute_build_counts(self): for build_error in self: @@ -131,6 +165,7 @@ class BuildError(models.Model): for build_error in self: build_error.summary = build_error.content[:80] + @api.depends('build_ids', 'child_ids.build_ids') def _compute_children_build_ids(self): for build_error in self: @@ -142,6 +177,13 @@ class BuildError(models.Model): for build_error in self: build_error.last_seen_build_id = build_error.children_build_ids and build_error.children_build_ids[0] or False + @api.depends('build_error_link_ids', 'child_ids') + def _compute_seen_date(self): + for build_error in self: + error_dates = (build_error.build_error_link_ids | build_error.child_ids.build_error_link_ids).mapped('log_date') + build_error.first_seen_date = error_dates and min(error_dates) + build_error.last_seen_date = error_dates and max(error_dates) + @api.depends('children_build_ids') def _compute_first_seen_build_id(self): for build_error in self: @@ -179,8 +221,11 @@ class BuildError(models.Model): build_errors |= existing_errors for build_error in existing_errors: logs = hash_dict[build_error.fingerprint] - for build in logs.mapped('build_id'): - build.build_error_ids += build_error + self.env['runbot.build.error.link'].create([{ + 'build_id': rec.build_id.id, + 'build_error_id': build_error.id, + 'log_date': rec.create_date} + for rec in logs if rec.build_id not in build_error.build_ids]) # update filepath if it changed. This is optionnal and mainly there in case we adapt the OdooRunner log if logs[0].path != build_error.file_path: @@ -191,13 +236,19 @@ class BuildError(models.Model): # create an error for the remaining entries for fingerprint, logs in hash_dict.items(): - build_errors |= self.env['runbot.build.error'].create({ + new_build_error = self.env['runbot.build.error'].create({ 'content': logs[0].message, 'module_name': logs[0].name, 'file_path': logs[0].path, 'function': logs[0].func, - 'build_ids': [(6, False, [r.build_id.id for r in logs])], }) + build_errors |= new_build_error + self.env['runbot.build.error.link'].create([{ + 'build_id': rec.build_id.id, + 'build_error_id': new_build_error.id, + 'log_date': rec.create_date} + for rec in logs]) + if build_errors: window_action = { diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index 64e3d20a..d47dd647 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -22,6 +22,9 @@ access_runbot_config_step_upgrade_db_manager,runbot_config_step_upgrade_db_manag access_runbot_build_error_user,runbot_build_error_user,runbot.model_runbot_build_error,group_user,1,0,0,0 access_runbot_build_error_admin,runbot_build_error_admin,runbot.model_runbot_build_error,runbot.group_runbot_admin,1,1,1,1 access_runbot_build_error_manager,runbot_build_error_manager,runbot.model_runbot_build_error,runbot.group_runbot_error_manager,1,1,1,1 +access_runbot_build_error_link_user,runbot_runbot_build_error_link_user,runbot.model_runbot_build_error_link,group_user,1,0,0,0 +access_runbot_build_error_link_admin,runbot_runbot_build_error_link_admin,runbot.model_runbot_build_error_link,runbot.group_runbot_admin,1,1,1,0 +access_runbot_build_error_link_manager,runbot_runbot_build_error_link_manager,runbot.model_runbot_build_error_link,runbot.group_runbot_error_manager,1,1,1,0 access_runbot_build_error_tag_user,runbot_build_error_tag_user,runbot.model_runbot_build_error_tag,group_user,1,0,0,0 access_runbot_build_error_tag_admin,runbot_build_error_tag_admin,runbot.model_runbot_build_error_tag,runbot.group_runbot_admin,1,1,1,1 access_runbot_build_error_tag_manager,runbot_build_error_tag_manager,runbot.model_runbot_build_error_tag,runbot.group_runbot_error_manager,1,1,1,1 diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py index 3bf0c7bd..e0df3860 100644 --- a/runbot/tests/test_build_error.py +++ b/runbot/tests/test_build_error.py @@ -1,5 +1,6 @@ import hashlib +from odoo import fields from odoo.exceptions import ValidationError from .common import RunbotCase @@ -30,6 +31,7 @@ class TestBuildError(RunbotCase): def setUp(self): super(TestBuildError, self).setUp() self.BuildError = self.env['runbot.build.error'] + self.BuildErrorLink = self.env['runbot.build.error.link'] self.BuildErrorTeam = self.env['runbot.team'] self.ErrorRegex = self.env['runbot.error.regex'] @@ -62,10 +64,12 @@ class TestBuildError(RunbotCase): def test_merge(self): build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) - error_a = self.BuildError.create({'content': 'foo bar', 'build_ids': [(6, 0, [build_a.id])]}) + error_a = self.BuildError.create({'content': 'foo bar'}) + self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id}) build_b = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) - error_b = self.BuildError.create({'content': 'foo bar', 'build_ids': [(6, 0, [build_b.id])]}) + error_b = self.BuildError.create({'content': 'foo bar'}) + self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id}) (error_a | error_b)._merge() self.assertEqual(len(self.BuildError.search([('fingerprint', '=', error_a.fingerprint)])), 1) @@ -85,10 +89,13 @@ class TestBuildError(RunbotCase): top_error = self.BuildError.create({'content': 'foo foo', 'active': False}) build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) - error_a = self.BuildError.create({'content': 'foo bar', 'parent_id': top_error.id , 'build_ids': [(6, 0, [build_a.id])]}) + error_a = self.BuildError.create({'content': 'foo bar', 'parent_id': top_error.id }) + self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id}) build_b = self.create_test_build({'local_result': 'ko', 'local_state': 'done'}) - error_b = self.BuildError.create({'content': 'foo bar', 'test_tags': 'footag', 'build_ids': [(6, 0, [build_b.id])]}) + error_b = self.BuildError.create({'content': 'foo bar', 'test_tags': 'footag'}) + self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id}) + linked_error = self.BuildError.create({'content': 'foo foo bar', 'parent_id': error_b.id}) (error_a | error_b)._merge() @@ -124,6 +131,7 @@ class TestBuildError(RunbotCase): }) log = { + 'create_date': fields.Datetime.from_string('2023-08-29 00:46:21'), 'message': RTE_ERROR, 'build_id': ko_build.id, 'level': 'ERROR', @@ -144,12 +152,17 @@ class TestBuildError(RunbotCase): ko_build._parse_logs() ok_build._parse_logs() - build_error = self.BuildError.search([('build_ids', 'in', [ko_build.id])]) + # build_error = self.BuildError.search([('build_ids', 'in', [ko_build.id])]) + build_error = self.BuildErrorLink.search([('build_id.id', '=', ko_build.id)]).mapped('build_error_id') self.assertTrue(build_error) self.assertTrue(build_error.fingerprint.startswith('af0e88f3')) self.assertTrue(build_error.cleaned_content.startswith('%'), 'The cleaner should have replace "FAIL: " with a "%" sign by default') + error_link = self.env['runbot.build.error.link'].search([('build_id', '=', ko_build.id), ('build_error_id', '=', build_error.id)]) + self.assertTrue(error_link, 'An error link should exists') + self.assertIn(ko_build, build_error.build_error_link_ids.mapped('build_id'), 'Ko build should be in build_error_link_ids') + self.assertEqual(error_link.log_date, fields.Datetime.from_string('2023-08-29 00:46:21')) self.assertIn(ko_build, build_error.build_ids, 'The parsed build should be added to the runbot.build.error') - self.assertFalse(self.BuildError.search([('build_ids', 'in', [ok_build.id])]), 'A successful build should not associated to a runbot.build.error') + self.assertFalse(self.BuildErrorLink.search([('build_id', '=', ok_build.id)]), 'A successful build should not be associated to a runbot.build.error') self.assertEqual(error_team, build_error.team_id) # Test that build with same error is added to the errors @@ -175,7 +188,7 @@ class TestBuildError(RunbotCase): IrLog.create(log) ko_build_new._parse_logs() self.assertNotIn(ko_build_new, build_error.build_ids, 'The parsed build should not be added to a fixed runbot.build.error') - new_build_error = self.BuildError.search([('build_ids', 'in', [ko_build_new.id])]) + new_build_error = self.BuildErrorLink.search([('build_id', '=', ko_build_new.id)]).mapped('build_error_id') self.assertIn(ko_build_new, new_build_error.build_ids, 'The parsed build with a re-apearing error should generate a new runbot.build.error') self.assertIn(build_error, new_build_error.error_history_ids, 'The old error should appear in history') @@ -185,16 +198,18 @@ class TestBuildError(RunbotCase): error_a = self.env['runbot.build.error'].create({ 'content': 'foo', - 'build_ids': [(6, 0, [build_a.id])], 'active': False # Even a fixed error coul be linked }) + self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id}) + error_b = self.env['runbot.build.error'].create({ 'content': 'bar', - 'build_ids': [(6, 0, [build_b.id])], 'random': True }) + self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id}) + # test that the random bug is parent when linking errors all_errors = error_a | error_b all_errors.action_link_errors() diff --git a/runbot/views/build_error_views.xml b/runbot/views/build_error_views.xml index 49d8258a..972d547f 100644 --- a/runbot/views/build_error_views.xml +++ b/runbot/views/build_error_views.xml @@ -46,14 +46,15 @@ - + - + +