Differential D698

Don't ignore arch in fedmsg deduplication
ClosedPublic
Actions

Authored by mkrizek on Dec 16 2015, 6:23 PM.

Details

Reviewers
kparal
tflink
jskladan
Maniphest Tasks
T667: fedmsg deduplication ignores archs
Commits
rRSDB0798b4d43a5b: Don't ignore arch in fedmsg deduplication
Test Plan

Tested locally

Diff Detail

Repository
rRSDB resultsdb
Branch
feature/fix-dedup-arch
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 399
Build 399: arc lint + arc unit
mkrizek retitled this revision from to Don't ignore arch in fedmsg deduplication.Dec 16 2015, 6:23 PM
mkrizek updated this object.
mkrizek edited the test plan for this revision. (Show Details)
mkrizek added reviewers: tflink, kparal, jskladan.
tflink requested changes to this revision.Dec 17 2015, 7:22 AM

This should have some unit tests with it. I'd also like to see this done with fewer joins, if possible

This revision now requires changes to proceed.Dec 17 2015, 7:22 AM
jskladan added a comment.Jan 4 2016, 3:44 PM
resultsdb/controllers/api_v1.py
644
In D698#13337, @tflink wrote:

... I'd also like to see this done with fewer joins, if possible

Since multiple joins on the same table create query like this:

SELECT result.id AS result_id FROM result 
      JOIN resultdata AS resultdata_1 ON result.id = resultdata_1.result_id
      JOIN resultdata AS resultdata_2 ON result.id = resultdata_2.result_id 
WHERE
      resultdata_1.key = :key_1 AND resultdata_1.value = :value_1 
      AND
      resultdata_2.key = :key_2 AND resultdata_2.value = :value_2

I doubt that it could be done in less of them. It could be replaced by nested SELECT, but from what I've read on the PostgreSQL planner, it is better with JOINs. Would be worth a try, though.

mkrizek updated this revision to Diff 1832.Jan 7 2016, 2:23 PM
  • Add unittests
jskladan accepted this revision.Jan 12 2016, 12:47 PM
tflink accepted this revision.Jan 12 2016, 4:45 PM
This revision is now accepted and ready to land.Jan 12 2016, 4:45 PM
mkrizek closed this revision.Jan 13 2016, 8:46 AM

Revision Contents

Diff 1832

View Options

resultsdb/controllers/api_v1.py

Show First 20 Lines • Show All 624 Lines • ▼ Show 20 Line(s)
625 625​
626 626​
627def is_duplicate_result(cur_result): 627​def is_duplicate_result(cur_result):
628 ''' 628​ '''
629 Check whether the current result that is being submitted is already 629​ Check whether the current result that is being submitted is already
630 stored in database. 630​ stored in database.
631 631​
632 Two results are considered duplicates if they have same: 632​ Two results are considered duplicates if they have same:
633 item, testcase and outcome. 633​ item, testcase, outcome and arch.
634 ''' 634​ '''
635 item = None ​
636 for result_data in cur_result.result_data: ​
637 if result_data.key == 'item': ​
638 item = result_data.value ​
639 635​
640 q = db.session.query(Result).filter(Result.id != cur_result.id) 636​ q = db.session.query(Result).filter(Result.id != cur_result.id)
641 637​
642 alias = db.aliased(Testcase) 638​ alias = db.aliased(Testcase)
643 q = q.join(alias).filter(alias.name == cur_result.testcase.name) 639​ q = q.join(alias).filter(alias.name == cur_result.testcase.name)
644 640​
645 alias = db.aliased(ResultData) 641​ for result_data in cur_result.result_data:
646 q = q.join(alias).filter(db.and_(alias.key == 'item', alias.value == item)) 642​ if result_data.key in ['item', 'arch']:
643​ alias = db.aliased(ResultData)
644​ q = q.join(alias).filter(db.and_(alias.key == result_data.key, alias.value == result_data.value))
jskladanUnsubmitted
Not Done
In D698#13337, @tflink wrote:

... I'd also like to see this done with fewer joins, if possible

Since multiple joins on the same table create query like this:

SELECT result.id AS result_id FROM result 
      JOIN resultdata AS resultdata_1 ON result.id = resultdata_1.result_id
      JOIN resultdata AS resultdata_2 ON result.id = resultdata_2.result_id 
WHERE
      resultdata_1.key = :key_1 AND resultdata_1.value = :value_1 
      AND
      resultdata_2.key = :key_2 AND resultdata_2.value = :value_2

I doubt that it could be done in less of them. It could be replaced by nested SELECT, but from what I've read on the PostgreSQL planner, it is better with JOINs. Would be worth a try, though.

647 645​
648 q = q.order_by(db.desc(Result.submit_time)) 646​ q = q.order_by(db.desc(Result.submit_time))
649 647​
650 last_result = q.first() 648​ last_result = q.first()
651 if not last_result: 649​ if not last_result:
652 return False 650​ return False
653 return last_result.outcome == cur_result.outcome 651​ return last_result.outcome == cur_result.outcome
654 652​
▲ Show 20 Lines • Show All 129 Lines • Show Last 20 Lines
View Options

testing/functest_api.py

Show First 20 Lines • Show All 412 Lines • ▼ Show 20 Line(s)407 def test_get_results(self):
413 413​
414 assert r.status_code == 200 414​ assert r.status_code == 200
415 assert data['data'][0]['outcome'] == self.ref_outcome 415​ assert data['data'][0]['outcome'] == self.ref_outcome
416 assert data['data'][0]['id'] == self.ref_job_id 416​ assert data['data'][0]['id'] == self.ref_job_id
417 assert data['data'][0]['testcase']['name'] == self.ref_testcase_name 417​ assert data['data'][0]['testcase']['name'] == self.ref_testcase_name
418 assert data['href'] == self.prefix + 'results' 418​ assert data['href'] == self.prefix + 'results'
419 assert data['next'] == self.prefix + 'results?page=1' 419​ assert data['next'] == self.prefix + 'results?page=1'
420 assert data['prev'] == None 420​ assert data['prev'] == None
421 assert data['pages'] == 1 ​
422 assert data['total'] == 1 ​
423 421​
424 def test_get_empty_results(self): 422​ def test_get_empty_results(self):
425 r = self.app.get('/api/v1.0/results') 423​ r = self.app.get('/api/v1.0/results')
426 424​
427 data = json.loads(r.data) 425​ data = json.loads(r.data)
428 426​
429 assert r.status_code == 200 427​ assert r.status_code == 200
430 assert data['data'] == [] 428​ assert data['data'] == []
431 429​
432 def test_get_testcases_results(self): 430​ def test_get_testcases_results(self):
433 self.test_create_result() 431​ self.test_create_result()
434 432​
435 r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name) 433​ r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name)
436 434​
437 data = json.loads(r.data) 435​ data = json.loads(r.data)
438 436​
439 assert r.status_code == 200 437​ assert r.status_code == 200
440 assert data['data'][0]['outcome'] == self.ref_outcome 438​ assert data['data'][0]['outcome'] == self.ref_outcome
441 assert data['data'][0]['id'] == self.ref_job_id 439​ assert data['data'][0]['id'] == self.ref_job_id
442 assert data['data'][0]['testcase']['name'] == self.ref_testcase_name 440​ assert data['data'][0]['testcase']['name'] == self.ref_testcase_name
443 assert data['href'] == self.prefix + 'testcases/testcase/results' 441​ assert data['href'] == self.prefix + 'testcases/testcase/results'
444 assert data['next'] == self.prefix + 'testcases/testcase/results?page=1' 442​ assert data['next'] == self.prefix + 'testcases/testcase/results?page=1'
445 assert data['prev'] == None 443​ assert data['prev'] == None
446 assert data['pages'] == 1 ​
447 assert data['total'] == 1 ​
448 444​
449 def test_get_testcases_empty_results(self): 445​ def test_get_testcases_empty_results(self):
450 r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name) 446​ r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name)
451 447​
452 data = json.loads(r.data) 448​ data = json.loads(r.data)
453 449​
454 assert r.status_code == 404 450​ assert r.status_code == 404
455 assert data['message'] == "Testcase not found" 451​ assert data['message'] == "Testcase not found"
456 452​
457 def test_get_jobs_results(self): 453​ def test_get_jobs_results(self):
458 self.test_create_result() 454​ self.test_create_result()
459 455​
460 r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name) 456​ r = self.app.get('/api/v1.0/testcases/%s/results' % self.ref_testcase_name)
461 457​
462 data = json.loads(r.data) 458​ data = json.loads(r.data)
463 459​
464 assert r.status_code == 200 460​ assert r.status_code == 200
465 assert data['data'][0]['outcome'] == self.ref_outcome 461​ assert data['data'][0]['outcome'] == self.ref_outcome
466 assert data['data'][0]['id'] == self.ref_job_id 462​ assert data['data'][0]['id'] == self.ref_job_id
467 assert data['data'][0]['testcase']['name'] == self.ref_testcase_name 463​ assert data['data'][0]['testcase']['name'] == self.ref_testcase_name
468 assert data['href'] == self.prefix + 'testcases/testcase/results' 464​ assert data['href'] == self.prefix + 'testcases/testcase/results'
469 assert data['next'] == self.prefix + 'testcases/testcase/results?page=1' 465​ assert data['next'] == self.prefix + 'testcases/testcase/results?page=1'
470 assert data['prev'] == None 466​ assert data['prev'] == None
471 assert data['pages'] == 1 ​
472 assert data['total'] == 1 ​
473 467​
474 def test_get_jobs_empty_results(self): 468​ def test_get_jobs_empty_results(self):
475 r = self.app.get('/api/v1.0/jobs/%s/results' % self.ref_job_id) 469​ r = self.app.get('/api/v1.0/jobs/%s/results' % self.ref_job_id)
476 470​
477 data = json.loads(r.data) 471​ data = json.loads(r.data)
478 472​
479 assert r.status_code == 404 473​ assert r.status_code == 404
480 assert data['message'] == "Job not found" 474​ assert data['message'] == "Job not found"
481 475​
482 def test_get_jobs_results_with_jsonp(self): 476​ def test_get_jobs_results_with_jsonp(self):
483 self.test_create_result() 477​ self.test_create_result()
484 478​
485 r = self.app.get('/api/v1.0/testcases/%s/results?callback=wat' % self.ref_testcase_name) 479​ r = self.app.get('/api/v1.0/testcases/%s/results?callback=wat' % self.ref_testcase_name)
486 480​
487 assert r.data.startswith('wat(') 481​ assert r.data.startswith('wat(')
488 data = json.loads(r.data[4:-2]) 482​ data = json.loads(r.data[4:-2])
489 483​
490 assert r.status_code == 200 484​ assert r.status_code == 200
491 assert data['data'][0]['outcome'] == self.ref_outcome 485​ assert data['data'][0]['outcome'] == self.ref_outcome
492 assert data['data'][0]['id'] == self.ref_job_id 486​ assert data['data'][0]['id'] == self.ref_job_id
493 assert data['data'][0]['testcase']['name'] == self.ref_testcase_name 487​ assert data['data'][0]['testcase']['name'] == self.ref_testcase_name
494 assert data['href'] == self.prefix + 'testcases/testcase/results?callback=wat' 488​ assert data['href'] == self.prefix + 'testcases/testcase/results?callback=wat'
495 assert data['next'] == self.prefix + 'testcases/testcase/results?callback=wat&page=1' 489​ assert data['next'] == self.prefix + 'testcases/testcase/results?callback=wat&page=1'
496 assert data['prev'] == None 490​ assert data['prev'] == None
497 assert data['pages'] == 1 ​
498 assert data['total'] == 1 ​
View Options

testing/functest_duplicate_result.py

  • This file was added.
1​# Copyright 2016, Red Hat, Inc.
2​#
3​# This program is free software; you can redistribute it and/or modify
4​# it under the terms of the GNU General Public License as published by
5​# the Free Software Foundation; either version 2 of the License, or
6​# (at your option) any later version.
7​#
8​# This program is distributed in the hope that it will be useful,
9​# but WITHOUT ANY WARRANTY; without even the implied warranty of
10​# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11​# GNU General Public License for more details.
12​#
13​# You should have received a copy of the GNU General Public License along
14​# with this program; if not, write to the Free Software Foundation, Inc.,
15​# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
16​#
17​# Authors:
18​# Martin Krizek <mkrizek@redhat.com>
19​
20​
21​import os
22​import tempfile
23​
24​import resultsdb
25​import resultsdb.cli
26​
27​from resultsdb import db
28​from resultsdb.models.results import Testcase, Job, Result, ResultData
29​from resultsdb.controllers.api_v1 import is_duplicate_result
30​
31​
32​class TestFuncDuplicateResult():
33​ @classmethod
34​ def setup_class(cls):
35​ cls.dbfile = tempfile.NamedTemporaryFile(delete=False)
36​ cls.dbfile.close()
37​ resultsdb.app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///%s' % cls.dbfile.name
38​
39​ @classmethod
40​ def teardown_class(cls):
41​ os.unlink(cls.dbfile.name)
42​
43​ def setup_method(self, method):
44​ resultsdb.cli.initialize_db(destructive=True)
45​
46​ self.ref_testcase_name = "testcase"
47​ self.ref_testcase_url = "http://fedoraqa.fedoraproject.org/%s" % self.ref_testcase_name
48​ self.ref_job_uuid = '12-3456-7890'
49​ self.ref_job_url = "http://fedoraqa.fedoraproject.org"
50​ self.ref_job_name = "F20 Virtualization Testday"
51​ self.ref_status = "SCHEDULED"
52​ self.ref_outcome = "PASSED"
53​ self.ref_result_summary = "1 PASSED, 0 FAILED"
54​ self.ref_result_log_url = "http://fedoraqa.fedoraproject.org/logs"
55​ self.ref_item = 'chat-2.8.8-21.fc20'
56​ self.ref_type = 'koji_build'
57​ self.ref_arch = 'x86_64'
58​
59​ self.testcase = Testcase(self.ref_testcase_name, self.ref_testcase_url)
60​ db.session.add(self.testcase)
61​ db.session.commit()
62​
63​ self.job = Job(self.ref_status, self.ref_job_url, self.ref_job_name, self.ref_job_uuid)
64​ db.session.add(self.job)
65​ db.session.commit()
66​
67​ self.create_result(self.testcase, self.ref_outcome, self.ref_item, self.ref_arch)
68​
69​ def create_result(self, testcase, outcome, item=None, arch=None):
70​ result = Result(self.job,
71​ testcase,
72​ outcome,
73​ self.ref_result_log_url,
74​ self.ref_result_summary)
75​
76​ if item:
77​ ResultData(result, 'item', item)
78​
79​ ResultData(result, 'type', self.ref_type)
80​
81​ if arch:
82​ ResultData(result, 'arch', arch)
83​
84​ db.session.add(result)
85​ db.session.commit()
86​
87​ return result
88​
89​ def test_duplicate_result(self):
90​ result = self.create_result(self.testcase, self.ref_outcome, self.ref_item, self.ref_arch)
91​
92​ assert is_duplicate_result(result)
93​
94​ def test_duplicate_result_more_results(self):
95​ result = self.create_result(self.testcase, self.ref_outcome, self.ref_item, self.ref_arch)
96​ self.create_result(self.testcase, self.ref_outcome, 'item1', 'arch1')
97​ self.create_result(self.testcase, self.ref_outcome, 'item2', 'arch2')
98​ self.create_result(self.testcase, self.ref_outcome, 'item3', 'arch3')
99​
100​ assert is_duplicate_result(result)
101​
102​ def test_not_duplicate_result_item(self):
103​ result = self.create_result(self.testcase, self.ref_outcome, item='chat-2.8.8-21.fc21', arch=self.ref_arch)
104​
105​ assert not is_duplicate_result(result)
106​
107​ def test_not_duplicate_result_arch(self):
108​ result = self.create_result(self.testcase, self.ref_outcome, item=self.ref_item, arch='i686')
109​
110​ assert not is_duplicate_result(result)
111​
112​ def test_not_duplicate_result_testcase(self):
113​ testcase = Testcase('random_testcase_unique', 'random_testcase_unique_url')
114​ db.session.add(testcase)
115​ db.session.commit()
116​
117​ result = self.create_result(testcase, self.ref_outcome, self.ref_item, self.ref_arch)
118​
119​ assert not is_duplicate_result(result)
120​
121​ def test_not_duplicate_result_outcome(self):
122​ result = self.create_result(self.testcase, 'FAILED', self.ref_item, self.ref_arch)
123​
124​ assert not is_duplicate_result(result)