zuul/zuul
Code Issues Proposed changes

Merge "Omit succssful jobs on very long gerrit reports"

Browse Source
This commit is contained in:
Zuul
2025-06-05 21:38:52 +00:00
committed by Gerrit Code Review
parent fb496873c8 277fbd2226
commit e5c599854e
7 changed files with 94 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
tests/fakegerrit.py
View File

@ -29,8 +29,11 @@ import copy
import http.server
from zuul.lib.logutil import get_annotated_logger
import zuul.driver.git.gitwatcher as gitwatcher
import zuul.driver.gerrit.gerritconnection as gerritconnection
from zuul.driver.git import gitwatcher
from zuul.driver.gerrit import (
gerritconnection,
gerritreporter,
)
from tests.util import FIXTURE_DIR, random_sha1
import git
@ -938,7 +941,7 @@ class GerritWebServer(object):
message = data['message']
b_len = len(message.encode('utf-8'))
if b_len > gerritconnection.GERRIT_HUMAN_MESSAGE_LIMIT:
if b_len > gerritreporter.GERRIT_HUMAN_MESSAGE_LIMIT:
self.send_response(400, message='Message length exceeded')
self.end_headers()
return

34
tests/unit/test_gerrit.py
View File

@ -18,6 +18,7 @@ import threading
import time
import textwrap
from unittest import mock
import yaml
import zuul.model
import tests.base
@ -306,6 +307,39 @@ class TestGerritWeb(ZuulTestCase):
self.assertIn('... (truncated)',
A.messages[0])
def test_message_too_long_failed_job(self):
# Test that if we have a failed job in a long message, we
# at least include the failed jobs.
config = []
joblist = []
# Use enough jobs with long names to trip the 16k limit
for x in range(60):
name = 'job-%s-%s' % ('x' * 200, x)
job = dict(
name=name,
parent='test-job',
)
config.append(dict(job=job))
joblist.append(name)
config.append(dict(
project=dict(
check=dict(
jobs=joblist,
)
)
))
in_repo_conf = yaml.dump(config)
file_dict = {'.zuul.yaml': in_repo_conf}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)
self.executor_server.failJob('project-test1', A)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertIn('length restrictions', A.messages[0])
self.assertIn('FAILURE', A.messages[0])
self.assertNotIn('SUCCESS', A.messages[0])
def test_dependent_dynamic_line_comment(self):
in_repo_conf = textwrap.dedent(
"""

23
zuul/driver/gerrit/gerritconnection.py
View File

@ -75,17 +75,6 @@ TIMEOUT = 30
# SSH connection timeout
SSH_TIMEOUT = TIMEOUT
# commentSizeLimit default set by Gerrit. Gerrit is a bit
# vague about what this means, it says
#
# Comments which exceed this size will be rejected ... Size
# computation is approximate and may be off by roughly 1% ...
# Default is 16k
#
# This magic number is int((16 << 10) * 0.98). Robot comments
# are accounted for separately.
GERRIT_HUMAN_MESSAGE_LIMIT = 16056
class HTTPConflictException(Exception):
message = "Received response 409"
@ -1531,12 +1520,6 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
cmd += ' --notify %s' % shlex.quote(notify)
if phase1:
if message:
b_len = len(message.encode('utf-8'))
if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
log.info("Message truncated %d > %d" %
(b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
message = ("%s... (truncated)" %
message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
cmd += ' --message %s' % shlex.quote(message)
for key, val in labels.items():
if val is True:
@ -1606,12 +1589,6 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
urllib.parse.quote(str(change.branch), safe=''),
change.id)
log = get_annotated_logger(self.log, zuul_event_id)
b_len = len(message.encode('utf-8'))
if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
log.info("Message truncated %d > %d" %
(b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
message = ("%s... (truncated)" %
message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
data = dict(strict_labels=False)
if notify:
data['notify'] = notify

23
zuul/driver/gerrit/gerritreporter.py
View File

@ -22,6 +22,17 @@ from zuul.lib.logutil import get_annotated_logger
from zuul.model import Change
from zuul.reporter import BaseReporter
# commentSizeLimit default set by Gerrit. Gerrit is a bit
# vague about what this means, it says
#
# Comments which exceed this size will be rejected ... Size
# computation is approximate and may be off by roughly 1% ...
# Default is 16k
#
# This magic number is int((16 << 10) * 0.98). Robot comments
# are accounted for separately.
GERRIT_HUMAN_MESSAGE_LIMIT = 16056
class GerritReporter(BaseReporter):
"""Sends off reports to Gerrit."""
@ -71,6 +82,18 @@ class GerritReporter(BaseReporter):
comments = self.getFileComments(item, change)
if self._create_comment:
message = self._formatItemReport(item, change=change)
b_len = len(message.encode('utf-8'))
if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
log.info("Message too long, using short form")
message = self._formatItemReport(item, change=change,
short=True)
b_len = len(message.encode('utf-8'))
if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
log.info("Message truncated %d > %d" %
(b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
message = ("%s... (truncated)" %
message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
else:
message = ''

2
zuul/driver/github/githubreporter.py
View File

@ -160,7 +160,7 @@ class GithubReporter(BaseReporter):
return ':%s: [%s](%s) %s%s%s%s\n' % (
(emoji,) + job_fields[:6])
def _formatItemReportJobs(self, item):
def _formatItemReportJobs(self, item, short):
# Return the list of jobs portion of the report
ret = ''
jobs_fields, skipped = self._getItemReportJobsFields(item)

2
zuul/driver/pagure/pagurereporter.py
View File

@ -86,7 +86,7 @@ class PagureReporter(BaseReporter):
msg = self._formatItemReportMergeConflict(item, change)
self.addPullComment(item, change, msg)
def _formatItemReportJobs(self, item):
def _formatItemReportJobs(self, item, short):
# Return the list of jobs portion of the report
ret = ''
jobs_fields, skipped = self._getItemReportJobsFields(item)

49
zuul/reporter/__init__.py
View File

@ -162,11 +162,11 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
return format_methods[action]
def _formatItemReport(self, item, change=None,
with_jobs=True, action=None):
with_jobs=True, action=None, short=False):
"""Format a report from the given items. Usually to provide results to
a reporter taking free-form text."""
action = action or self._action
ret = self._getFormatter(action)(item, change, with_jobs)
ret = self._getFormatter(action)(item, change, with_jobs, short)
config_warnings = item.getConfigErrors(errors=False, warnings=True)
if config_warnings:
@ -185,26 +185,26 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
return ret
def _formatItemReportEnqueue(self, item, change, with_jobs=True):
def _formatItemReportEnqueue(self, item, change, with_jobs, short):
return item.manager.pipeline.enqueue_message.format(
pipeline=item.manager.pipeline.getSafeAttributes(),
item_url=item.formatItemUrl())
def _formatItemReportStart(self, item, change, with_jobs=True):
def _formatItemReportStart(self, item, change, with_jobs, short):
return item.manager.pipeline.start_message.format(
pipeline=item.manager.pipeline.getSafeAttributes(),
item_url=item.formatItemUrl())
def _formatItemReportSuccess(self, item, change, with_jobs=True):
def _formatItemReportSuccess(self, item, change, with_jobs, short):
msg = item.manager.pipeline.success_message
if with_jobs:
item_url = item.formatItemUrl()
if item_url is not None:
msg += '\n' + item_url
msg += '\n\n' + self._formatItemReportJobs(item)
msg += '\n\n' + self._formatItemReportJobs(item, short)
return msg
def _formatItemReportFailure(self, item, change, with_jobs=True):
def _formatItemReportFailure(self, item, change, with_jobs, short):
if len(item.changes) > 1:
_this_change = 'These changes'
_depends = 'depend'
@ -237,7 +237,8 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
change_annotations = {c: ' (config error)'
for c in changes_with_errors}
if with_jobs:
msg = '{}\n{}'.format(msg, self._formatItemReportJobs(item))
msg = '{}\n{}'.format(msg, self._formatItemReportJobs(
item, short))
msg = "{}\n{}".format(
msg, self._formatItemReportOtherChanges(item,
change_annotations))
@ -252,7 +253,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
item_url = item.formatItemUrl()
if item_url is not None:
msg += '\n' + item_url
msg += '\n\n' + self._formatItemReportJobs(item)
msg += '\n\n' + self._formatItemReportJobs(item, short)
return msg
def _getChangesWithErrors(self, item):
@ -268,36 +269,39 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
ret.append(change)
return ret
def _formatItemReportMergeConflict(self, item, change, with_jobs=True):
def _formatItemReportMergeConflict(self, item, change, with_jobs, short):
return item.manager.pipeline.merge_conflict_message
def _formatItemReportMergeFailure(self, item, change, with_jobs=True):
def _formatItemReportMergeFailure(self, item, change, with_jobs, short):
return 'This change was not merged by the code review system.\n'
def _formatItemReportConfigError(self, item, change, with_jobs=True):
def _formatItemReportConfigError(self, item, change, with_jobs, short):
if item.getConfigErrors():
msg = str(item.getConfigErrors()[0].error)
else:
msg = "Unknown configuration error"
return msg
def _formatItemReportNoJobs(self, item, change, with_jobs=True):
def _formatItemReportNoJobs(self, item, change, with_jobs, short):
return item.manager.pipeline.no_jobs_message.format(
pipeline=item.manager.pipeline.getSafeAttributes(),
item_url=item.formatItemUrl())
def _formatItemReportDisabled(self, item, change, with_jobs=True):
def _formatItemReportDisabled(self, item, change, with_jobs=True,
short=False):
if item.current_build_set.result == 'SUCCESS':
return self._formatItemReportSuccess(item, change)
return self._formatItemReportSuccess(item, change,
with_jobs, short)
elif item.current_build_set.result == 'FAILURE':
return self._formatItemReportFailure(item, change)
return self._formatItemReportFailure(item, change,
with_jobs, short)
else:
return self._formatItemReport(item, change)
return self._formatItemReport(item, change, with_jobs, short)
def _formatItemReportDequeue(self, item, change, with_jobs=True):
def _formatItemReportDequeue(self, item, change, with_jobs, short):
msg = item.manager.pipeline.dequeue_message
if with_jobs:
msg += '\n\n' + self._formatItemReportJobs(item)
msg += '\n\n' + self._formatItemReportJobs(item, short)
return msg
def _formatItemReportOtherChanges(self, item, change_annotations):
@ -366,13 +370,18 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
(name, url, result, error, elapsed, voting, success_message))
return jobs_fields, skipped
def _formatItemReportJobs(self, item):
def _formatItemReportJobs(self, item, short):
# Return the list of jobs portion of the report
# If short is true, omit successful jobs for brevity
ret = ''
jobs_fields, skipped = self._getItemReportJobsFields(item)
for job_fields in jobs_fields:
if short and job_fields[2] == 'SUCCESS':
continue
ret += '- %s%s : %s%s%s%s\n' % job_fields[:6]
if skipped:
jobtext = 'job' if skipped == 1 else 'jobs'
ret += 'Skipped %i %s\n' % (skipped, jobtext)
if short:
ret += 'Successful jobs omitted due to length restrictions\n'
return ret