From c3066e598d67c4b06cabbbce2de5f60d83d8063e Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:54:38 +0000 Subject: [PATCH 1/8] Revert "Publish more metadata in bus messages." This reverts commit ed508799ebc0cb772d129ba61ffd5759f4c5ffd0 in order to make the message structure match the original Taskotron messages that were previously being published. --- diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py index 5ec453e..cd78405 100644 --- a/resultsdb/messaging.py +++ b/resultsdb/messaging.py @@ -75,6 +75,7 @@ class FedmsgPlugin(MessagingPlugin): task = dict( (datum.key, datum.value) for datum in result.data + if datum.key in ('item', 'type',) ) task['name'] = result.testcase.name msg = { @@ -137,6 +138,7 @@ class StompPlugin(MessagingPlugin): task = dict( (datum.key, datum.value) for datum in result.data + if datum.key in ('item', 'type',) ) task['name'] = result.testcase.name msg = { From 1ea4bb91901ccd99806e82598a0d655a3368099e Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:54:38 +0000 Subject: [PATCH 2/8] Revert "Add 'scenario' to identifiers in get_prev_result" This reverts commit 2067ce544a6d9b5d1aa1f49023bd668ef3c817ef. --- diff --git a/resultsdb/controllers/api_v2.py b/resultsdb/controllers/api_v2.py index 732b06a..9caa1f1 100644 --- a/resultsdb/controllers/api_v2.py +++ b/resultsdb/controllers/api_v2.py @@ -687,14 +687,14 @@ def create_result(): def get_prev_result(result): """ - Find previous result with the same testcase, item, type, arch and - scenario. Return None if no result is found. + Find previous result with the same testcase, item, type, and arch. + Return None if no result is found. """ q = db.session.query(Result).filter(Result.id != result.id) q = q.filter_by(testcase_name=result.testcase_name) for result_data in result.data: - if result_data.key in ['item', 'type', 'arch', 'scenario']: + if result_data.key in ['item', 'type', 'arch']: alias = db.aliased(ResultData) q = q.join(alias).filter( db.and_(alias.key == result_data.key, alias.value == result_data.value)) diff --git a/testing/functest_create_fedmsg.py b/testing/functest_create_fedmsg.py index b0207fc..15fc222 100644 --- a/testing/functest_create_fedmsg.py +++ b/testing/functest_create_fedmsg.py @@ -89,12 +89,10 @@ class TestFuncCreateFedmsg(): self.ref_result_item = 'perl-Specio-0.25-1.fc26' self.ref_result_type = 'koji_build' self.ref_result_arch = 'x86_64' - self.ref_result_scenario = 'x86_64.efi' self.ref_result_data = { 'item': self.ref_result_item, 'type': self.ref_result_type, 'arch': self.ref_result_arch, - 'scenario': self.ref_result_scenario, 'moo': ['boo', 'woof'], } self.ref_result_ref_url = 'http://example.com/testing.result' @@ -148,8 +146,6 @@ class TestFuncCreateFedmsg(): assert result_data.value == self.ref_result_type if result_data.key == 'arch': assert result_data.value == self.ref_result_arch - if result_data.key == 'scenario': - assert result_data.value == self.ref_result_scenario self.helper_create_result() prev_result = apiv2.get_prev_result(self.ref_result_obj) @@ -164,8 +160,6 @@ class TestFuncCreateFedmsg(): assert result_data.value == self.ref_result_type if result_data.key == 'arch': assert result_data.value == self.ref_result_arch - if result_data.key == 'scenario': - assert result_data.value == self.ref_result_scenario ref_outcome = 'FAILED' if self.ref_result_outcome == ref_outcome: @@ -183,8 +177,6 @@ class TestFuncCreateFedmsg(): assert result_data.value == self.ref_result_type if result_data.key == 'arch': assert result_data.value == self.ref_result_arch - if result_data.key == 'scenario': - assert result_data.value == self.ref_result_scenario def test_get_prev_result_different_item(self): data = copy.deepcopy(self.ref_result_data) @@ -210,14 +202,6 @@ class TestFuncCreateFedmsg(): prev_result = apiv2.get_prev_result(self.ref_result_obj) assert prev_result is None - def test_get_prev_result_different_scenario(self): - data = copy.deepcopy(self.ref_result_data) - data['scenario'] = data['scenario'] + '.fake' - self.helper_create_result(data=data) - - prev_result = apiv2.get_prev_result(self.ref_result_obj) - assert prev_result is None - def test_get_prev_result_different_testcase_name(self): self.helper_create_result(testcase={'name': self.ref_testcase_name + '.fake'}) From a90efff2d40998593c3d9f39461dae07b4f32af5 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:54:47 +0000 Subject: [PATCH 3/8] Revert "fix fedmsg tests to include 'scenario' in fake result" This reverts commit 21b4534e1e9a36fc70288b23382b374e655196d1. --- diff --git a/testing/functest_create_fedmsg.py b/testing/functest_create_fedmsg.py index 15fc222..6342f39 100644 --- a/testing/functest_create_fedmsg.py +++ b/testing/functest_create_fedmsg.py @@ -38,7 +38,7 @@ class MyResultData(object): class MyResult(object): - def __init__(self, id, testcase_name, outcome, item, item_type, arch, scenario): + def __init__(self, id, testcase_name, outcome, item, item_type, arch): self.id = id self.testcase_name = testcase_name self.outcome = outcome @@ -46,7 +46,6 @@ class MyResult(object): MyResultData('item', item), MyResultData('type', item_type), MyResultData('arch', arch), - MyResultData('scenario', scenario), ] @@ -97,8 +96,7 @@ class TestFuncCreateFedmsg(): } self.ref_result_ref_url = 'http://example.com/testing.result' self.ref_result_obj = MyResult( - 0, self.ref_testcase_name, self.ref_result_outcome, self.ref_result_item, - self.ref_result_type, self.ref_result_arch, self.ref_result_scenario) + 0, self.ref_testcase_name, self.ref_result_outcome, self.ref_result_item, self.ref_result_type, self.ref_result_arch) def teardown_method(self, method): # Reset this for each test. From ca17e3c60f7734aff4100fce53cfaa274c28ba0c Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:54:51 +0000 Subject: [PATCH 4/8] refactor FedmsgPlugin to eliminate nested 'msg' dict --- diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py index cd78405..b279793 100644 --- a/resultsdb/messaging.py +++ b/resultsdb/messaging.py @@ -69,7 +69,7 @@ class FedmsgPlugin(MessagingPlugin): """ A fedmsg plugin, used to publish to the fedmsg bus. """ def publish(self, message): - fedmsg.publish(**message) + fedmsg.publish(topic='result.new', modname=self.modname, msg=message) def create_message(self, result, prev_result): task = dict( @@ -79,27 +79,23 @@ class FedmsgPlugin(MessagingPlugin): ) task['name'] = result.testcase.name msg = { - 'topic': 'result.new', - 'modname': self.modname, - 'msg': { - 'task': task, - 'result': { - 'id': result.id, - 'submit_time': result.submit_time.strftime("%Y-%m-%d %H:%M:%S UTC"), - 'prev_outcome': prev_result.outcome if prev_result else None, - 'outcome': result.outcome, - 'log_url': result.ref_url, - } + 'task': task, + 'result': { + 'id': result.id, + 'submit_time': result.submit_time.strftime("%Y-%m-%d %H:%M:%S UTC"), + 'prev_outcome': prev_result.outcome if prev_result else None, + 'outcome': result.outcome, + 'log_url': result.ref_url, } } # For the v1 API if hasattr(result, 'job'): - msg['msg']['result']['job_url'] = result.job.ref_url + msg['result']['job_url'] = result.job.ref_url # For the v2 API if hasattr(result, 'group'): - msg['msg']['result']['group_url'] = result.group.ref_url + msg['result']['group_url'] = result.group.ref_url return msg From 767f59378a4749b7aaf6ce064a45d5a152a5e5dd Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:56:31 +0000 Subject: [PATCH 5/8] consolidate create_message() methods into one place Now that the two messaging plugins have identical implementations of create_message(), we can just make it a single function. --- diff --git a/resultsdb/controllers/api_v1.py b/resultsdb/controllers/api_v1.py index 446ddb5..01664d9 100644 --- a/resultsdb/controllers/api_v1.py +++ b/resultsdb/controllers/api_v1.py @@ -38,7 +38,7 @@ from resultsdb import app, db from resultsdb.serializers.api_v1 import Serializer from resultsdb.models.results import Group, Result, Testcase, ResultData from resultsdb.models.results import JOB_STATUS, RESULT_OUTCOME -from resultsdb.messaging import load_messaging_plugin +from resultsdb.messaging import load_messaging_plugin, create_message QUERY_LIMIT = 20 @@ -573,7 +573,7 @@ def create_result(): name=app.config['MESSAGE_BUS_PLUGIN'], kwargs=app.config['MESSAGE_BUS_KWARGS'], ) - plugin.publish(plugin.create_message(result, prev_result)) + plugin.publish(create_message(result, prev_result)) return jsonify(SERIALIZE(result)), 201 diff --git a/resultsdb/controllers/api_v2.py b/resultsdb/controllers/api_v2.py index 9caa1f1..d442ddd 100644 --- a/resultsdb/controllers/api_v2.py +++ b/resultsdb/controllers/api_v2.py @@ -35,7 +35,7 @@ from resultsdb import app, db from resultsdb.serializers.api_v2 import Serializer from resultsdb.models.results import Group, Result, Testcase, ResultData from resultsdb.models.results import RESULT_OUTCOME -from resultsdb.messaging import load_messaging_plugin +from resultsdb.messaging import load_messaging_plugin, create_message from resultsdb.lib.helpers import non_empty, dict_or_string, list_or_none QUERY_LIMIT = 20 @@ -677,7 +677,7 @@ def create_result(): name=app.config['MESSAGE_BUS_PLUGIN'], kwargs=app.config['MESSAGE_BUS_KWARGS'], ) - plugin.publish(plugin.create_message(result, prev_result)) + plugin.publish(create_message(result, prev_result)) else: app.logger.debug("Skipping messaging, result %d outcome has not changed", result.id) diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py index b279793..7ae5573 100644 --- a/resultsdb/messaging.py +++ b/resultsdb/messaging.py @@ -28,11 +28,39 @@ import logging log = logging.getLogger(__name__) +def create_message(result, prev_result=None): + task = dict( + (datum.key, datum.value) + for datum in result.data + if datum.key in ('item', 'type',) + ) + task['name'] = result.testcase.name + msg = { + 'task': task, + 'result': { + 'id': result.id, + 'submit_time': result.submit_time.strftime("%Y-%m-%d %H:%M:%S UTC"), + 'prev_outcome': prev_result.outcome if prev_result else None, + 'outcome': result.outcome, + 'log_url': result.ref_url, + } + } + + # For the v1 API + if hasattr(result, 'job'): + msg['result']['job_url'] = result.job.ref_url + + # For the v2 API + if hasattr(result, 'group'): + msg['result']['group_url'] = result.group.ref_url + + return msg + + class MessagingPlugin(object): """ Abstract base class that messaging plugins must extend. - Two abstract methods are declared which must be implemented: - - create_message(result, prev_result=None) + One abstract method is declared which must be implemented: - publish(message) """ @@ -43,10 +71,6 @@ class MessagingPlugin(object): setattr(self, key, value) @abc.abstractmethod - def create_message(self, result, prev_result=None): - pass - - @abc.abstractmethod def publish(self, message): pass @@ -61,9 +85,6 @@ class DummyPlugin(MessagingPlugin): self.history.append(message) log.info("%r->%r" % (self, message)) - def create_message(self, result, prev_result): - return dict(id=result.id) - class FedmsgPlugin(MessagingPlugin): """ A fedmsg plugin, used to publish to the fedmsg bus. """ @@ -71,34 +92,6 @@ class FedmsgPlugin(MessagingPlugin): def publish(self, message): fedmsg.publish(topic='result.new', modname=self.modname, msg=message) - def create_message(self, result, prev_result): - task = dict( - (datum.key, datum.value) - for datum in result.data - if datum.key in ('item', 'type',) - ) - task['name'] = result.testcase.name - msg = { - 'task': task, - 'result': { - 'id': result.id, - 'submit_time': result.submit_time.strftime("%Y-%m-%d %H:%M:%S UTC"), - 'prev_outcome': prev_result.outcome if prev_result else None, - 'outcome': result.outcome, - 'log_url': result.ref_url, - } - } - - # For the v1 API - if hasattr(result, 'job'): - msg['result']['job_url'] = result.job.ref_url - - # For the v2 API - if hasattr(result, 'group'): - msg['result']['group_url'] = result.group.ref_url - - return msg - class StompPlugin(MessagingPlugin): def __init__(self, **kwargs): @@ -130,34 +123,6 @@ class StompPlugin(MessagingPlugin): finally: conn.disconnect() - def create_message(self, result, prev_result): - task = dict( - (datum.key, datum.value) - for datum in result.data - if datum.key in ('item', 'type',) - ) - task['name'] = result.testcase.name - msg = { - 'task': task, - 'result': { - 'id': result.id, - 'submit_time': result.submit_time.strftime("%Y-%m-%d %H:%M:%S UTC"), - 'prev_outcome': prev_result.outcome if prev_result else None, - 'outcome': result.outcome, - 'log_url': result.ref_url, - } - } - - # For the v1 API - if hasattr(result, 'job'): - msg['result']['job_url'] = result.job.ref_url - - # For the v2 API - if hasattr(result, 'group'): - msg['result']['group_url'] = result.group.ref_url - - return msg - def load_messaging_plugin(name, kwargs): """ Instantiate and return the appropriate messaging plugin. """ diff --git a/testing/functest_api_v20.py b/testing/functest_api_v20.py index 605771f..4bb34a2 100644 --- a/testing/functest_api_v20.py +++ b/testing/functest_api_v20.py @@ -845,4 +845,4 @@ class TestFuncApiV20(): self.helper_create_result() plugin = resultsdb.messaging.DummyPlugin assert len(plugin.history) == 1, plugin.history - assert plugin.history == [{'id': 1}] + assert plugin.history[0]['result']['id'] == 1 diff --git a/testing/functest_create_fedmsg.py b/testing/functest_create_fedmsg.py index 6342f39..e0b5ed3 100644 --- a/testing/functest_create_fedmsg.py +++ b/testing/functest_create_fedmsg.py @@ -205,9 +205,3 @@ class TestFuncCreateFedmsg(): prev_result = apiv2.get_prev_result(self.ref_result_obj) assert prev_result is None - - def test_message_publication(self): - self.helper_create_result() - plugin = resultsdb.messaging.DummyPlugin - assert len(plugin.history) == 1, plugin.history - assert plugin.history == [{'id': 1}] From 0da4be7717c26cffb92bdb407c680ad7b72852d5 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:56:34 +0000 Subject: [PATCH 6/8] fix typo in api_v1 create_job() Should be 'uuid' not 'iuuid'. This regressed in commit 62c49f7fac88d88c954dccbb2a532e9f67b82faf which introduced API v2. Also add a test case to cover creating a result in API v1. --- diff --git a/resultsdb/controllers/api_v1.py b/resultsdb/controllers/api_v1.py index 01664d9..26984ac 100644 --- a/resultsdb/controllers/api_v1.py +++ b/resultsdb/controllers/api_v1.py @@ -314,7 +314,7 @@ def create_job(): except HTTPException as error: return jsonify(error.data), error.code - if not args['iuuid']: + if not args['uuid']: args['uuid'] = str(uuid.uuid1()) job = Group(args['uuid'], args['ref_url'], args['name']) diff --git a/testing/functest_api_v10.py b/testing/functest_api_v10.py new file mode 100644 index 0000000..889edb9 --- /dev/null +++ b/testing/functest_api_v10.py @@ -0,0 +1,72 @@ +# Copyright 2016, Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +import json +import datetime +import os +import tempfile +import copy + +import resultsdb +from resultsdb import db +import resultsdb.cli +import resultsdb.messaging +from resultsdb.models.results import Result + + +class TestFuncApiV10(object): + + def setup_method(self, method): + self.dbfile = tempfile.NamedTemporaryFile(delete=False) + self.dbfile.close() + resultsdb.app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///%s' % self.dbfile.name + resultsdb.app.config['MESSAGE_BUS_PUBLISH'] = True + resultsdb.app.config['MESSAGE_BUS_PLUGIN'] = 'dummy' + resultsdb.cli.initialize_db(destructive=True) + self.app = resultsdb.app.test_client() + + def teardown_method(self, method): + resultsdb.messaging.DummyPlugin.history = [] + db.session.remove() + os.unlink(self.dbfile.name) + + def test_create_result(self): + job_data = { + 'name': 'dist.rpmlint', + 'ref_url': 'https://taskotron.example.com/execdb/', + } + r = self.app.post('/api/v1.0/jobs', data=json.dumps(job_data), content_type='application/json') + assert r.status_code == 201 + job_id = json.loads(r.data)['id'] + + result_data = { + 'job_id': job_id, + 'outcome': 'FAILED', + 'testcase_name': 'dist.rpmlint', + 'summary': '78 errors, 150 warnings', + 'result_data': { + 'type': ['koji_build'], + 'item': ['openfst-1.6.6-1.fc28'], + }, + 'log_url': 'https://taskotron.example.com/artifacts/', + } + r = self.app.post('/api/v1.0/results', data=json.dumps(result_data), content_type='application/json') + assert r.status_code == 201 + result_id = json.loads(r.data)['id'] + + # Check that the result was stored in the database. + result = db.session.query(Result).get(result_id) + assert result.outcome == 'FAILED' From 939f3baadf3d4148c77e4be5488428e1326cbe0c Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 04:56:34 +0000 Subject: [PATCH 7/8] fix 'job_url' and 'group_url' keys in messages The 'job_url' key is supposed to appear in messages when a result is created using API v1, but this regressed in commit 62c49f7fac88d88c954dccbb2a532e9f67b82faf which introduced API v2. The create_fedmsg() function was still referring to the `result.job` attribute even though it had been removed from the model in that commit. This patch restores the previous intended behaviour for API v1 messages, with a test case. The 'group_url' key was introduced in the same commit as an analog when results are created using API v2, but this also never worked because of a typo: it was referring to a `result.group` attribute but the model actually has `result.groups` (plural). The same typo was carried over in commit fd4ebfb0e8389931213acc9eefc40d07e158d30a ("flexible messaging"). Since this never worked, and thus the 'group_url' key has never been included in any messages, this patch simply drops it. --- diff --git a/resultsdb/controllers/api_v1.py b/resultsdb/controllers/api_v1.py index 26984ac..1e1ac20 100644 --- a/resultsdb/controllers/api_v1.py +++ b/resultsdb/controllers/api_v1.py @@ -573,7 +573,7 @@ def create_result(): name=app.config['MESSAGE_BUS_PLUGIN'], kwargs=app.config['MESSAGE_BUS_KWARGS'], ) - plugin.publish(create_message(result, prev_result)) + plugin.publish(create_message(result, prev_result, include_job_url=True)) return jsonify(SERIALIZE(result)), 201 diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py index 7ae5573..c4c63b4 100644 --- a/resultsdb/messaging.py +++ b/resultsdb/messaging.py @@ -28,7 +28,7 @@ import logging log = logging.getLogger(__name__) -def create_message(result, prev_result=None): +def create_message(result, prev_result=None, include_job_url=False): task = dict( (datum.key, datum.value) for datum in result.data @@ -46,13 +46,8 @@ def create_message(result, prev_result=None): } } - # For the v1 API - if hasattr(result, 'job'): - msg['result']['job_url'] = result.job.ref_url - - # For the v2 API - if hasattr(result, 'group'): - msg['result']['group_url'] = result.group.ref_url + if include_job_url: # only in the v1 API + msg['result']['job_url'] = result.groups[0].ref_url if result.groups else None return msg diff --git a/testing/functest_api_v10.py b/testing/functest_api_v10.py index 889edb9..ca544f3 100644 --- a/testing/functest_api_v10.py +++ b/testing/functest_api_v10.py @@ -70,3 +70,13 @@ class TestFuncApiV10(object): # Check that the result was stored in the database. result = db.session.query(Result).get(result_id) assert result.outcome == 'FAILED' + + # Check that a message was emitted. + plugin = resultsdb.messaging.DummyPlugin + assert len(plugin.history) == 1, plugin.history + assert plugin.history[0]['task']['item'] == 'openfst-1.6.6-1.fc28' + assert plugin.history[0]['task']['type'] == 'koji_build' + assert plugin.history[0]['result']['id'] == 1 + assert plugin.history[0]['result']['outcome'] == 'FAILED' + assert plugin.history[0]['result']['log_url'] == 'https://taskotron.example.com/artifacts/' + assert plugin.history[0]['result']['job_url'] == 'https://taskotron.example.com/execdb/' diff --git a/testing/functest_api_v20.py b/testing/functest_api_v20.py index 4bb34a2..7f639cf 100644 --- a/testing/functest_api_v20.py +++ b/testing/functest_api_v20.py @@ -845,4 +845,8 @@ class TestFuncApiV20(): self.helper_create_result() plugin = resultsdb.messaging.DummyPlugin assert len(plugin.history) == 1, plugin.history + assert plugin.history[0]['task']['item'] == self.ref_result_item + assert plugin.history[0]['task']['type'] == self.ref_result_type assert plugin.history[0]['result']['id'] == 1 + assert plugin.history[0]['result']['outcome'] == self.ref_result_outcome + assert plugin.history[0]['result']['log_url'] == self.ref_result_ref_url From aa54e5b94e37dd1beccc78927dc6c06c856041b8 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Feb 21 2018 05:02:55 +0000 Subject: [PATCH 8/8] generic message format which matches the HTTP API v2 Fixes #92. This changes the message structure to contain the complete result data, in the same format as the HTTP API v2. Existing message structure and de-duplication logic is preserved in the new publish_taskotron_message() function, which is disabled by default but can be enabled for backwards compatibility by setting MESSAGE_BUS_PUBLISH_TASKOTRON=True. This function is hardcoded to only send on fedmsg and with 'taskotron' as the topic. --- diff --git a/resultsdb/config.py b/resultsdb/config.py index 564cf4f..e4ba6b8 100644 --- a/resultsdb/config.py +++ b/resultsdb/config.py @@ -69,7 +69,7 @@ class Config(object): # the fedmsg plugin expects an extra `modname` argument that can be used to # configure the topic, like this: # ... - # e.g. org.fedoraproject.prod.taskotron.result.new + # e.g. org.fedoraproject.prod.resultsdb.result.new MESSAGE_BUS_KWARGS = {} ## Alternatively, you could use the 'stomp' messaging plugin. @@ -88,6 +88,9 @@ class Config(object): # }, #} + # Publish Taskotron-compatible fedmsgs on the 'taskotron' topic + MESSAGE_BUS_PUBLISH_TASKOTRON = False + class ProductionConfig(Config): DEBUG = False diff --git a/resultsdb/controllers/api_v1.py b/resultsdb/controllers/api_v1.py index 1e1ac20..1eda3f1 100644 --- a/resultsdb/controllers/api_v1.py +++ b/resultsdb/controllers/api_v1.py @@ -38,7 +38,7 @@ from resultsdb import app, db from resultsdb.serializers.api_v1 import Serializer from resultsdb.models.results import Group, Result, Testcase, ResultData from resultsdb.models.results import JOB_STATUS, RESULT_OUTCOME -from resultsdb.messaging import load_messaging_plugin, create_message +from resultsdb.messaging import load_messaging_plugin, create_message, publish_taskotron_message QUERY_LIMIT = 20 @@ -565,40 +565,16 @@ def create_result(): db.session.add(result) if app.config['MESSAGE_BUS_PUBLISH']: - prev_result = get_prev_result(result) - # result is considered duplicate of prev_result when - # outcomes are the same. - if not prev_result or prev_result.outcome != result.outcome: - plugin = load_messaging_plugin( - name=app.config['MESSAGE_BUS_PLUGIN'], - kwargs=app.config['MESSAGE_BUS_KWARGS'], - ) - plugin.publish(create_message(result, prev_result, include_job_url=True)) + plugin = load_messaging_plugin( + name=app.config['MESSAGE_BUS_PLUGIN'], + kwargs=app.config['MESSAGE_BUS_KWARGS'], + ) + plugin.publish(create_message(result)) - return jsonify(SERIALIZE(result)), 201 - - -def get_prev_result(result): - ''' - Find previous result with the same: - item, testcase, outcome and arch. - - Return None if no result is found. - ''' - q = db.session.query(Result).filter(Result.id != result.id) - - alias = db.aliased(Testcase) - q = q.join(alias).filter(alias.name == result.testcase.name) + if app.config['MESSAGE_BUS_PUBLISH_TASKOTRON']: + publish_taskotron_message(result) - for result_data in result.data: - if result_data.key in ['item', 'arch']: - alias = db.aliased(ResultData) - q = q.join(alias).filter( - db.and_(alias.key == result_data.key, alias.value == result_data.value)) - - q = q.order_by(db.desc(Result.submit_time)) - - return q.first() + return jsonify(SERIALIZE(result)), 201 # ============================================================================= diff --git a/resultsdb/controllers/api_v2.py b/resultsdb/controllers/api_v2.py index d442ddd..983f5ed 100644 --- a/resultsdb/controllers/api_v2.py +++ b/resultsdb/controllers/api_v2.py @@ -35,7 +35,7 @@ from resultsdb import app, db from resultsdb.serializers.api_v2 import Serializer from resultsdb.models.results import Group, Result, Testcase, ResultData from resultsdb.models.results import RESULT_OUTCOME -from resultsdb.messaging import load_messaging_plugin, create_message +from resultsdb.messaging import load_messaging_plugin, create_message, publish_taskotron_message from resultsdb.lib.helpers import non_empty, dict_or_string, list_or_none QUERY_LIMIT = 20 @@ -669,40 +669,19 @@ def create_result(): if app.config['MESSAGE_BUS_PUBLISH']: app.logger.debug("Preparing to publish message for result id %d", result.id) - prev_result = get_prev_result(result) - # result is considered duplicate of prev_result when - # outcomes are the same. - if not prev_result or prev_result.outcome != result.outcome: - plugin = load_messaging_plugin( - name=app.config['MESSAGE_BUS_PLUGIN'], - kwargs=app.config['MESSAGE_BUS_KWARGS'], - ) - plugin.publish(create_message(result, prev_result)) - else: - app.logger.debug("Skipping messaging, result %d outcome has not changed", result.id) + plugin = load_messaging_plugin( + name=app.config['MESSAGE_BUS_PLUGIN'], + kwargs=app.config['MESSAGE_BUS_KWARGS'], + ) + plugin.publish(create_message(result)) + if app.config['MESSAGE_BUS_PUBLISH_TASKOTRON']: + app.logger.debug("Preparing to publish Taskotron message for result id %d", result.id) + publish_taskotron_message(result) return jsonify(SERIALIZE(result)), 201 -def get_prev_result(result): - """ - Find previous result with the same testcase, item, type, and arch. - Return None if no result is found. - """ - q = db.session.query(Result).filter(Result.id != result.id) - q = q.filter_by(testcase_name=result.testcase_name) - - for result_data in result.data: - if result_data.key in ['item', 'type', 'arch']: - alias = db.aliased(ResultData) - q = q.join(alias).filter( - db.and_(alias.key == result_data.key, alias.value == result_data.value)) - - q = q.order_by(db.desc(Result.submit_time)) - return q.first() - - # ============================================================================= # TESTCASES # ============================================================================= diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py index c4c63b4..c08e27e 100644 --- a/resultsdb/messaging.py +++ b/resultsdb/messaging.py @@ -24,11 +24,59 @@ import pkg_resources import fedmsg +from resultsdb import db +from resultsdb.models.results import Result, ResultData +from resultsdb.serializers.api_v2 import Serializer + import logging log = logging.getLogger(__name__) -def create_message(result, prev_result=None, include_job_url=False): +SERIALIZE = Serializer().serialize + + +def get_prev_result(result): + """ + Find previous result with the same testcase, item, type, and arch. + Return None if no result is found. + + Note that this logic is Taskotron-specific: it does not consider the + possibility that a result may be distinguished by other keys in the data + (for example 'scenario' which is used in OpenQA results). But this is only + used for publishing Taskotron compatibility messages, thus we keep this + logic as is. + """ + q = db.session.query(Result).filter(Result.id != result.id) + q = q.filter_by(testcase_name=result.testcase_name) + + for result_data in result.data: + if result_data.key in ['item', 'type', 'arch']: + alias = db.aliased(ResultData) + q = q.join(alias).filter( + db.and_(alias.key == result_data.key, alias.value == result_data.value)) + + q = q.order_by(db.desc(Result.submit_time)) + return q.first() + + +def publish_taskotron_message(result, include_job_url=False): + """ + Publish a fedmsg on the taskotron topic with Taskotron-compatible structure. + + These messages are deprecated, consumers should consume from the resultsdb + topic instead. + """ + prev_result = get_prev_result(result) + if prev_result is not None and prev_result.outcome == result.outcome: + # If the previous result had the same outcome, skip publishing + # a message for this new result. + # This was intended as a workaround to avoid spammy messages from the + # dist.depcheck task, which tends to produce a very large number of + # identical results for any given build, because of the way that it is + # designed. + log.debug("Skipping Taskotron message for result %d, outcome has not changed", result.id) + return + task = dict( (datum.key, datum.value) for datum in result.data @@ -49,7 +97,12 @@ def create_message(result, prev_result=None, include_job_url=False): if include_job_url: # only in the v1 API msg['result']['job_url'] = result.groups[0].ref_url if result.groups else None - return msg + fedmsg.publish(topic='result.new', modname='taskotron', msg=msg) + + +def create_message(result): + # Re-use the same structure as in the HTTP API v2. + return SERIALIZE(result) class MessagingPlugin(object): diff --git a/testing/functest_api_v10.py b/testing/functest_api_v10.py index ca544f3..9ab59d1 100644 --- a/testing/functest_api_v10.py +++ b/testing/functest_api_v10.py @@ -51,6 +51,7 @@ class TestFuncApiV10(object): r = self.app.post('/api/v1.0/jobs', data=json.dumps(job_data), content_type='application/json') assert r.status_code == 201 job_id = json.loads(r.data)['id'] + job_uuid = json.loads(r.data)['uuid'] result_data = { 'job_id': job_id, @@ -74,9 +75,11 @@ class TestFuncApiV10(object): # Check that a message was emitted. plugin = resultsdb.messaging.DummyPlugin assert len(plugin.history) == 1, plugin.history - assert plugin.history[0]['task']['item'] == 'openfst-1.6.6-1.fc28' - assert plugin.history[0]['task']['type'] == 'koji_build' - assert plugin.history[0]['result']['id'] == 1 - assert plugin.history[0]['result']['outcome'] == 'FAILED' - assert plugin.history[0]['result']['log_url'] == 'https://taskotron.example.com/artifacts/' - assert plugin.history[0]['result']['job_url'] == 'https://taskotron.example.com/execdb/' + assert plugin.history[0]['data']['item'] == ['openfst-1.6.6-1.fc28'] + assert plugin.history[0]['data']['type'] == ['koji_build'] + assert plugin.history[0]['id'] == 1 + assert plugin.history[0]['outcome'] == 'FAILED' + assert plugin.history[0]['groups'] == [job_uuid] + assert plugin.history[0]['note'] == '78 errors, 150 warnings' + assert plugin.history[0]['ref_url'] == 'https://taskotron.example.com/artifacts/' + assert plugin.history[0]['testcase']['name'] == 'dist.rpmlint' diff --git a/testing/functest_api_v20.py b/testing/functest_api_v20.py index 7f639cf..36080b9 100644 --- a/testing/functest_api_v20.py +++ b/testing/functest_api_v20.py @@ -845,8 +845,11 @@ class TestFuncApiV20(): self.helper_create_result() plugin = resultsdb.messaging.DummyPlugin assert len(plugin.history) == 1, plugin.history - assert plugin.history[0]['task']['item'] == self.ref_result_item - assert plugin.history[0]['task']['type'] == self.ref_result_type - assert plugin.history[0]['result']['id'] == 1 - assert plugin.history[0]['result']['outcome'] == self.ref_result_outcome - assert plugin.history[0]['result']['log_url'] == self.ref_result_ref_url + assert plugin.history[0]['data']['item'] == [self.ref_result_item] + assert plugin.history[0]['data']['type'] == [self.ref_result_type] + assert plugin.history[0]['id'] == 1 + assert plugin.history[0]['outcome'] == self.ref_result_outcome + assert plugin.history[0]['ref_url'] == self.ref_result_ref_url + assert plugin.history[0]['groups'] == [self.ref_group_uuid] + assert plugin.history[0]['note'] == self.ref_result_note + assert plugin.history[0]['testcase']['name'] == self.ref_testcase_name diff --git a/testing/functest_create_fedmsg.py b/testing/functest_create_fedmsg.py index e0b5ed3..bd1ad5e 100644 --- a/testing/functest_create_fedmsg.py +++ b/testing/functest_create_fedmsg.py @@ -25,7 +25,6 @@ import copy import resultsdb import resultsdb.cli -import resultsdb.controllers.api_v2 as apiv2 import resultsdb.messaging @@ -127,12 +126,12 @@ class TestFuncCreateFedmsg(): return r, data def test_get_prev_result_no_results(self): - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result is None def test_get_prev_result_exists(self): self.helper_create_result() - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result.id == 1 assert prev_result.outcome == self.ref_result_outcome @@ -146,7 +145,7 @@ class TestFuncCreateFedmsg(): assert result_data.value == self.ref_result_arch self.helper_create_result() - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result.id == 2 assert prev_result.outcome == self.ref_result_outcome @@ -163,7 +162,7 @@ class TestFuncCreateFedmsg(): if self.ref_result_outcome == ref_outcome: ref_outcome = 'PASSED' self.helper_create_result(outcome=ref_outcome) - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result.id == 3 assert prev_result.outcome == ref_outcome @@ -181,7 +180,7 @@ class TestFuncCreateFedmsg(): data['item'] = data['item'] + '.fake' self.helper_create_result(data=data) - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result is None def test_get_prev_result_different_type(self): @@ -189,7 +188,7 @@ class TestFuncCreateFedmsg(): data['type'] = data['type'] + '.fake' self.helper_create_result(data=data) - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result is None def test_get_prev_result_different_arch(self): @@ -197,11 +196,11 @@ class TestFuncCreateFedmsg(): data['arch'] = data['arch'] + '.fake' self.helper_create_result(data=data) - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result is None def test_get_prev_result_different_testcase_name(self): self.helper_create_result(testcase={'name': self.ref_testcase_name + '.fake'}) - prev_result = apiv2.get_prev_result(self.ref_result_obj) + prev_result = resultsdb.messaging.get_prev_result(self.ref_result_obj) assert prev_result is None