Differential D1133

merge two definitions of item types
ClosedPublic
Actions

Authored by kparal on Feb 16 2017, 9:53 AM.

Details

Reviewers
mkrizek
jskladan
Group Reviewers
libtaskotron
Commits
rLTRN8b6a72fea49e: merge two definitions of item types
Summary

As identified in D1119, we had two locations for definining allowed item
types. This merges it into a single location. Type handling might change
again soon, but for this moment this eliminates code duplication.

A few notes:

  • I picked check.ReportType as the single location, because it shows

up in documentation and our code and tasks use that.

  • The new single definition is a superset of both (therefore we now have

more item types documented and easy to use in reporting than we had
before)

  • There was a clash between bodhi_update and bodhi_id, I picked

bodhi_update because that's what our current tasks use.

  • I renamed yum_repository to dnf_repo, because no task currently

uses it, so it seems safe.

Test Plan

Test suite works, local rpmlint execution works

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to merge two definitions of item types.Feb 16 2017, 9:53 AM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: libtaskotron, jskladan, mkrizek.
kparal added inline comments.Feb 16 2017, 9:55 AM
libtaskotron/check.py
256–265

If you don't like this, the alternative is to depend on https://pypi.python.org/pypi/enum34 and use a proper enum.

jskladan accepted this revision.Feb 16 2017, 10:02 AM

LGTM, thanks for taking care of this

libtaskotron/check.py
256–265

I'm fine with this. python-enum34 is packaged in Fedora, so that would not be that big of a deal, but I don't really see it as that big of a win here, especially since we might be getting rid of these soon anyway.

This revision is now accepted and ready to land.Feb 16 2017, 10:02 AM
Closed by commit rLTRN8b6a72fea49e: merge two definitions of item types (authored by Kamil Páral <kparal@redhat.com>). · Explain WhyFeb 16 2017, 10:07 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mlibtaskotron/check.py (23 lines)
Mlibtaskotron/main.py (17 lines)
Mtesting/test_check.py (2 lines)
Mtesting/test_main.py (22 lines)

Diff 2900

View Options

libtaskotron/check.py

Show First 20 Lines • Show All 238 Lines • ▼ Show 20 Line(s)236 def __str__(self):
239 attrs = vars(self) 239​ attrs = vars(self)
240 attrs['output'] = '<stripped out>' 240​ attrs['output'] = '<stripped out>'
241 return '<%s: %s>' % (self.__class__.__name__, pprint.pformat(attrs)) 241​ return '<%s: %s>' % (self.__class__.__name__, pprint.pformat(attrs))
242 242​
243 243​
244class ReportType(object): 244​class ReportType(object):
245 ''' Enum for different types of :attr:`CheckDetail.report_type`''' 245​ ''' Enum for different types of :attr:`CheckDetail.report_type`'''
246 # the values are used as identifiers in a ResultYAML export 246​ # the values are used as identifiers in a ResultYAML export
247 KOJI_BUILD = 'koji_build' #: 247​ BODHI_UPDATE = 'bodhi_update' #:
248 BODHI_UPDATE = 'bodhi_update' #: 248​ COMPOSE = 'compose' #:
249 YUM_REPOSITORY = 'yum_repository' #: 249​ DIST_GIT_COMMIT = 'dist_git_commit' #:
250 COMPOSE = 'compose' #: 250​ DNF_REPO = 'dnf_repo' #:
251​ DOCKER_IMAGE = 'docker_image' #:
252​ GIT_COMMIT = 'git_commit' #:
253​ KOJI_BUILD = 'koji_build' #:
254​ KOJI_TAG = 'koji_tag' #:
255​
256​ @classmethod
257​ def list(cls):
258​ return [cls.BODHI_UPDATE,
259​ cls.COMPOSE,
260​ cls.DIST_GIT_COMMIT,
261​ cls.DNF_REPO,
262​ cls.DOCKER_IMAGE,
263​ cls.GIT_COMMIT,
264​ cls.KOJI_BUILD,
265​ cls.KOJI_TAG]
kparalAuthorUnsubmitted
Not Done

If you don't like this, the alternative is to depend on https://pypi.python.org/pypi/enum34 and use a proper enum.

jskladanUnsubmitted
Not Done

I'm fine with this. python-enum34 is packaged in Fedora, so that would not be that big of a deal, but I don't really see it as that big of a win here, especially since we might be getting rid of these soon anyway.

251 266​
252 267​
253def export_YAML(check_details): 268​def export_YAML(check_details):
254 '''Generate YAML output used for reporting to ResultsDB. 269​ '''Generate YAML output used for reporting to ResultsDB.
255 270​
256 Note: You need to provide all your :class:`CheckDetail`\s in a single pass 271​ Note: You need to provide all your :class:`CheckDetail`\s in a single pass
257 in order to generate a valid YAML output. You can't call this method several 272​ in order to generate a valid YAML output. You can't call this method several
258 times and then simply join the outputs simply as strings. 273​ times and then simply join the outputs simply as strings.
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

libtaskotron/main.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2015, Red Hat, Inc. 2​# Copyright 2009-2015, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6from __future__ import absolute_import 6​from __future__ import absolute_import
7import logging 7​import logging
8import os.path 8​import os.path
9import argparse 9​import argparse
10import datetime 10​import datetime
11import sys 11​import sys
12import copy 12​import copy
13 13​
14import libtaskotron 14​import libtaskotron
15from libtaskotron import logger 15​from libtaskotron import logger
16from libtaskotron import config 16​from libtaskotron import config
17​from libtaskotron import check
17from libtaskotron.logger import log 18​from libtaskotron.logger import log
18from libtaskotron.overlord import Overlord 19​from libtaskotron.overlord import Overlord
19 20​
20 21​
21# The list of accepted item types on the command line (--type option) ​
22_ITEM_TYPES = [ ​
23 "bodhi_id", ​
24 "koji_build", ​
25 "koji_tag", ​
26 "compose", ​
27 "docker_image", ​
28 "dist_git_commit", ​
29 "git_commit", ​
30] ​
31 ​
32 ​
33def get_argparser(): 22​def get_argparser():
34 '''Get the cmdline parser for the main runner. 23​ '''Get the cmdline parser for the main runner.
35 24​
36 :rtype: :class:`argparse.ArgumentParser` 25​ :rtype: :class:`argparse.ArgumentParser`
37 ''' 26​ '''
38 # IMPORTANT: When you add a new argument, look into `minion.BaseMinion.arg_data_exclude` and 27​ # IMPORTANT: When you add a new argument, look into `minion.BaseMinion.arg_data_exclude` and
39 # update it if needed! 28​ # update it if needed!
40 29​
41 parser = argparse.ArgumentParser() 30​ parser = argparse.ArgumentParser()
42 parser.add_argument("task", help="task formula to run") 31​ parser.add_argument("task", help="task formula to run")
43 parser.add_argument("-a", "--arch", 32​ parser.add_argument("-a", "--arch",
44 choices=["i386", "x86_64", "armhfp", "noarch"], 33​ choices=["i386", "x86_64", "armhfp", "noarch"],
45 action='append', 34​ action='append',
46 help="architecture specifying the item to be checked. If omitted, " 35​ help="architecture specifying the item to be checked. If omitted, "
47 "defaults to noarch.") 36​ "defaults to noarch.")
48 parser.add_argument("-i", "--item", help="item to be checked") 37​ parser.add_argument("-i", "--item", help="item to be checked")
49 parser.add_argument("-t", "--type", 38​ parser.add_argument("-t", "--type",
50 choices=_ITEM_TYPES, 39​ choices=check.ReportType.list(),
51 help="type of --item argument") 40​ help="type of --item argument")
52 parser.add_argument("-j", "--jobid", default="-1", 41​ parser.add_argument("-j", "--jobid", default="-1",
53 help="optional job identifier used to render log urls") 42​ help="optional job identifier used to render log urls")
54 parser.add_argument("-d", "--debug", action="store_true", 43​ parser.add_argument("-d", "--debug", action="store_true",
55 help="Enable debug output " 44​ help="Enable debug output "
56 "(set logging level to 'DEBUG')") 45​ "(set logging level to 'DEBUG')")
57 parser.add_argument("--uuid", default=datetime.datetime.utcnow().strftime("%Y%m%d_%H%M%S_%f"), 46​ parser.add_argument("--uuid", default=datetime.datetime.utcnow().strftime("%Y%m%d_%H%M%S_%f"),
58 help="Unique job identifier for the execution" 47​ help="Unique job identifier for the execution"
▲ Show 20 Lines • Show All 51 Lines • ▼ Show 20 Line(s)92def process_args(raw_args):
110 """ 99​ """
111 # do not modify the input dict 100​ # do not modify the input dict
112 args = copy.deepcopy(raw_args) 101​ args = copy.deepcopy(raw_args)
113 102​
114 # store the original unprocessed args for later use when passing it to the minion 103​ # store the original unprocessed args for later use when passing it to the minion
115 args['_orig_args'] = raw_args 104​ args['_orig_args'] = raw_args
116 105​
117 # process item + type 106​ # process item + type
118 if args['type'] in _ITEM_TYPES: 107​ if args['type'] in check.ReportType.list():
119 args[args['type']] = args['item'] 108​ args[args['type']] = args['item']
120 109​
121 # process arch 110​ # process arch
122 if args['arch'] is None: 111​ if args['arch'] is None:
123 args['arch'] = ['noarch'] 112​ args['arch'] = ['noarch']
124 113​
125 override = {} 114​ override = {}
126 for var in args['override']: 115​ for var in args['override']:
▲ Show 20 Lines • Show All 52 Lines • Show Last 20 Lines
View Options

testing/test_check.py

Show First 20 Lines • Show All 245 Lines • ▼ Show 20 Line(s)188class TestExportYAML(object):
246 246​
247 def test_multi(self): 247​ def test_multi(self):
248 '''Test export with multiple item sections''' 248​ '''Test export with multiple item sections'''
249 cd2 = check.CheckDetail(item='foobar-1.2-3.fc20', outcome='FAILED', 249​ cd2 = check.CheckDetail(item='foobar-1.2-3.fc20', outcome='FAILED',
250 note='dependency error', 250​ note='dependency error',
251 report_type=check.ReportType.BODHI_UPDATE) 251​ report_type=check.ReportType.BODHI_UPDATE)
252 cd3 = check.CheckDetail(item='f20-updates', outcome='INFO', 252​ cd3 = check.CheckDetail(item='f20-updates', outcome='INFO',
253 note='2 stale updates', 253​ note='2 stale updates',
254 report_type=check.ReportType.YUM_REPOSITORY) 254​ report_type=check.ReportType.DNF_REPO)
255 yaml_output = check.export_YAML([self.cd, cd2, cd3]) 255​ yaml_output = check.export_YAML([self.cd, cd2, cd3])
256 yaml_obj = yaml.safe_load(yaml_output) 256​ yaml_obj = yaml.safe_load(yaml_output)
257 257​
258 assert len(yaml_obj['results']) == 3 258​ assert len(yaml_obj['results']) == 3
259 assert yaml_obj['results'][0]['item'] == self.cd.item 259​ assert yaml_obj['results'][0]['item'] == self.cd.item
260 assert yaml_obj['results'][1]['item'] == cd2.item 260​ assert yaml_obj['results'][1]['item'] == cd2.item
261 assert yaml_obj['results'][2]['item'] == cd3.item 261​ assert yaml_obj['results'][2]['item'] == cd3.item
262 262​
▲ Show 20 Lines • Show All 135 Lines • Show Last 20 Lines
View Options

testing/test_main.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2014, Red Hat, Inc. 2​# Copyright 2009-2014, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6import pytest 6​import pytest
7from dingus import Dingus 7​from dingus import Dingus
8import os 8​import os
9 9​
10from libtaskotron import main 10​from libtaskotron import main
11from libtaskotron import config 11​from libtaskotron import config
12​from libtaskotron import check
12 13​
13 14​
14@pytest.mark.usefixtures('setup') 15​@pytest.mark.usefixtures('setup')
15class TestProcessArgs(): 16​class TestProcessArgs():
16 17​
17 @pytest.fixture 18​ @pytest.fixture
18 def setup(self, monkeypatch): 19​ def setup(self, monkeypatch):
19 self.ref_artifactsdir = '/dir/' 20​ self.ref_artifactsdir = '/dir/'
Show All 12 Lines28 self.ref_input = {'arch': ['x86_64'],
32 'uuid': '20150930_153933_359680', 33​ 'uuid': '20150930_153933_359680',
33 'override': []} 34​ 'override': []}
34 35​
35 def test_dont_modify_input(self): 36​ def test_dont_modify_input(self):
36 test_args = main.process_args(self.ref_input) 37​ test_args = main.process_args(self.ref_input)
37 assert test_args is not self.ref_input 38​ assert test_args is not self.ref_input
38 assert test_args != self.ref_input 39​ assert test_args != self.ref_input
39 40​
40 def test_type_koji_build(self): 41​ @pytest.mark.parametrize('itemtype', check.ReportType.list())
42​ def test_type(self, itemtype):
43​ self.ref_input['type'] = itemtype
41 test_args = main.process_args(self.ref_input) 44​ test_args = main.process_args(self.ref_input)
42 assert test_args['koji_build'] == self.ref_input['item'] 45​ assert test_args[itemtype] == self.ref_input['item']
43 ​
44 def test_type_bodhi_id(self): ​
45 self.ref_input['type'] = 'bodhi_id' ​
46 test_args = main.process_args(self.ref_input) ​
47 assert test_args['bodhi_id'] == self.ref_input['item'] ​
48 ​
49 def test_type_koji_tag(self): ​
50 self.ref_input['type'] = 'koji_tag' ​
51 test_args = main.process_args(self.ref_input) ​
52 assert test_args['koji_tag'] == self.ref_input['item'] ​
53 ​
54 def test_type_compose(self): ​
55 self.ref_input['type'] = 'compose' ​
56 test_args = main.process_args(self.ref_input) ​
57 assert test_args['compose'] == self.ref_input['item'] ​
58 46​
59 def test_multiple_arches(self): 47​ def test_multiple_arches(self):
60 self.ref_input['arch'] = ['x86_64', 'i386', 'noarch'] 48​ self.ref_input['arch'] = ['x86_64', 'i386', 'noarch']
61 test_args = main.process_args(self.ref_input) 49​ test_args = main.process_args(self.ref_input)
62 assert test_args['arch'] == self.ref_input['arch'] 50​ assert test_args['arch'] == self.ref_input['arch']
63 51​
64 def test_no_arch(self): 52​ def test_no_arch(self):
65 self.ref_input['arch'] = None 53​ self.ref_input['arch'] = None
▲ Show 20 Lines • Show All 92 Lines • Show Last 20 Lines