Differential D133

Add support for downloading SRPM for koji directive
ClosedPublic
Actions

Authored by mkrizek on Jun 11 2014, 12:01 PM.

Details

Reviewers
kparal
tflink
jskladan
Maniphest Tasks
T210: koji directive does not support downloading srpms
Commits
rLTRNf3ee0072a004: Add support for downloading SRPM for koji directive
Summary

Downloads only source srpm when arch is 'src'.

Fixes T210

Test Plan

Unit tests

koji:

action: download
koji_build: xchat-2.8.8-21.fc20
arch: src

Diff Detail

Repository
rLTRN libtaskotron
Lint
Lint Skipped
Unit
Unit Tests Skipped
mkrizek retitled this revision from to Add support for downloading SRPM for koji directive.Jun 11 2014, 12:01 PM
mkrizek updated this object.
mkrizek edited the test plan for this revision. (Show Details)
mkrizek added reviewers: tflink, kparal, jskladan.
mkrizek added a task: T210: koji directive does not support downloading srpms.
tflink requested changes to this revision.Jun 11 2014, 1:24 PM

Minor nit but otherwise looks good

libtaskotron/directives/koji_directive.py
75

I think that adding 'noarch' is valid for all cases except when 'src' is the only arch specified. We don't have any tasks that do this right now, but if arch is something like ['x86_64', 'src'], 'noarch' should be appended.

This revision now requires changes to proceed.Jun 11 2014, 1:24 PM
mkrizek updated this revision to Diff 398.Jun 12 2014, 1:01 PM

Address issue mentioned in the review

kparal added inline comments.Jun 12 2014, 2:08 PM
libtaskotron/directives/koji_directive.py
12–40

Can you please document the arch behavior? What gets added to the list when, that src is supported, etc.

90

I think it should be 'src' in self.arches here. The directive should be able to download src rpms in addition to requested arch rpms.

mkrizek updated this revision to Diff 400.Jun 12 2014, 2:38 PM

Addressing issues

kparal accepted this revision.Jun 12 2014, 3:34 PM

Looks good (comment below).

libtaskotron/directives/koji_directive.py
36

I got a bit confused by the wording, maybe this could work better:

Additional supported ``arch`` argument value: ``src``
mkrizek closed this revision.Jun 12 2014, 3:37 PM
mkrizek updated this revision to Diff 405.

Closed by commit rLTRNf3ee0072a004 (authored by @mkrizek).

Revision Contents

Diff 405

View Options

libtaskotron/directives/koji_directive.py

1from __future__ import absolute_import 1​from __future__ import absolute_import
2 2​
3from libtaskotron.koji_utils import KojiClient 3​from libtaskotron.koji_utils import KojiClient
4from libtaskotron.logger import log 4​from libtaskotron.logger import log
5from libtaskotron.directives import BaseDirective 5​from libtaskotron.directives import BaseDirective
6from libtaskotron.exceptions import TaskotronDirectiveError 6​from libtaskotron.exceptions import TaskotronDirectiveError
7from libtaskotron import rpm_utils 7​from libtaskotron import rpm_utils
8 8​
9directive_class = 'KojiDirective' 9​directive_class = 'KojiDirective'
10 10​
11class KojiDirective(BaseDirective): 11​class KojiDirective(BaseDirective):
12 """The koji directive interfaces with Koji to facilitate various koji 12​ """The koji directive interfaces with Koji to facilitate various koji
13 actions. 13​ actions.
14 14​
15 Format:: 15​ Format::
16 16​
17 koji: target_dir=<target_dir> command=[download, download_tag] <command args> 17​ koji: target_dir=<target_dir> command=[download, download_tag] <command args>
18 18​
19 Valid Commands: 19​ Valid Commands:
20 20​
21 download 21​ download
22 Downloads the koji build specified as ``koji_build`` 22​ Downloads the koji build specified as ``koji_build``
23 23​
24 Format:: 24​ Format::
25 25​
26 koji: command=download koji_build=<N(E)VR to download> arch=<desired arch> 26​ koji: command=download koji_build=<N(E)VR to download> arch=<desired arch>
27​
27 download_tag 28​ download_tag
28 Downloads rpms tagged by ``koji_tag`` 29​ Downloads rpms tagged by ``koji_tag``
29 30​
30 Format:: 31​ Format::
31 32​
32 koji: command=download_tag koji_tag=<koji tag to download> arch=<desired arch> 33​ koji: command=download_tag koji_tag=<koji tag to download> arch=<desired arch>
34​
35​
36​ Additional supported ``arch`` argument value: ``src``
kparalUnsubmitted
Not Done

I got a bit confused by the wording, maybe this could work better:

Additional supported ``arch`` argument value: ``src``
37​
38​ Note about the ``arch`` argument: if not requested, ``noarch`` is added to the list
39​ of arches (unless ``src`` is the only one requested)
33 """ 40​ """
kparalUnsubmitted
Not Done

Can you please document the arch behavior? What gets added to the list when, that src is supported, etc.

34 41​
35 def __init__(self, koji_session=None): 42​ def __init__(self, koji_session=None):
36 super(KojiDirective, self).__init__() 43​ super(KojiDirective, self).__init__()
37 if koji_session is None: 44​ if koji_session is None:
38 self.koji = KojiClient(koji_session) 45​ self.koji = KojiClient(koji_session)
39 else: 46​ else:
40 self.koji = koji_session 47​ self.koji = koji_session
41 48​
Show All 16 Lines49 def process(self, input_data, env_data):
58 65​
59 if not 'target_dir' in input_data: 66​ if not 'target_dir' in input_data:
60 self.target_dir = env_data['workdir'] 67​ self.target_dir = env_data['workdir']
61 else: 68​ else:
62 self.target_dir = input_data['target_dir'] 69​ self.target_dir = input_data['target_dir']
63 70​
64 self.arches = input_data['arch'] 71​ self.arches = input_data['arch']
65 72​
66 # https://phab.qadevel.cloud.fedoraproject.org/T155 73​ if 'noarch' not in self.arches and not self.arches == ['src']:
67 if 'noarch' not in self.arches: ​
68 self.arches.append('noarch') 74​ self.arches.append('noarch')
69 75​
tflinkUnsubmitted
Not Done

I think that adding 'noarch' is valid for all cases except when 'src' is the only arch specified. We don't have any tasks that do this right now, but if arch is something like ['x86_64', 'src'], 'noarch' should be appended.

70 if action == 'download': 76​ if action == 'download':
71 if 'koji_build' not in input_data: 77​ if 'koji_build' not in input_data:
72 detected_args = ', '.join(input_data.keys()) 78​ detected_args = ', '.join(input_data.keys())
73 raise TaskotronDirectiveError( 79​ raise TaskotronDirectiveError(
74 "The koji directive requires 'koji_build' for the 'download'" 80​ "The koji directive requires 'koji_build' for the 'download'"
75 "action. Detected arguments: %s" % detected_args) 81​ "action. Detected arguments: %s" % detected_args)
76 82​
77 nvr = rpm_utils.rpmformat(input_data['koji_build'], 'nvr') 83​ nvr = rpm_utils.rpmformat(input_data['koji_build'], 'nvr')
78 84​
79 log.info("Getting RPMs for koji build %s (%s) and downloading to %s", 85​ log.info("Getting RPMs for koji build %s (%s) and downloading to %s",
80 nvr, str(self.arches), self.target_dir) 86​ nvr, str(self.arches), self.target_dir)
81 output_data['downloaded_rpms'] = self.koji.get_nvr_rpms(nvr, 87​ output_data['downloaded_rpms'] = self.koji.get_nvr_rpms(nvr,
82 self.target_dir, 88​ self.target_dir,
83 self.arches) 89​ arches=self.arches,
90​ src=('src' in self.arches))
kparalUnsubmitted
Not Done

I think it should be 'src' in self.arches here. The directive should be able to download src rpms in addition to requested arch rpms.

84 elif action == 'download_tag': 91​ elif action == 'download_tag':
85 if 'koji_tag' not in input_data: 92​ if 'koji_tag' not in input_data:
86 detected_args = ', '.join(input_data.keys()) 93​ detected_args = ', '.join(input_data.keys())
87 raise TaskotronDirectiveError( 94​ raise TaskotronDirectiveError(
88 "The koji directive requires 'koji_tag' for the 'download_tag'" 95​ "The koji directive requires 'koji_tag' for the 'download_tag'"
89 "action. Detected arguments: %s" % detected_args) 96​ "action. Detected arguments: %s" % detected_args)
90 97​
91 koji_tag = input_data['koji_tag'] 98​ koji_tag = input_data['koji_tag']
92 99​
93 log.info("Getting koji builds for koji tag '%s', arches '%s' and " 100​ log.info("Getting koji builds for koji tag '%s', arches '%s' and "
94 "downloading to %s", koji_tag, str(self.arches), self.target_dir) 101​ "downloading to %s", koji_tag, str(self.arches), self.target_dir)
95 output_data['downloaded_rpms'] = self.koji.get_tagged_rpms(koji_tag, 102​ output_data['downloaded_rpms'] = self.koji.get_tagged_rpms(koji_tag,
96 self.target_dir, 103​ self.target_dir,
97 self.arches) 104​ self.arches)
98 105​
99 return output_data 106​ return output_data
View Options

testing/test_koji_directive.py

Show First 20 Lines • Show All 43 Lines • ▼ Show 20 Line(s)38 def test_parse_download_command(self):
44 stub_koji = Dingus(get_nvr_rpms__returns = self.ref_rpms) 44​ stub_koji = Dingus(get_nvr_rpms__returns = self.ref_rpms)
45 45​
46 test_helper = koji_directive.KojiDirective(stub_koji) 46​ test_helper = koji_directive.KojiDirective(stub_koji)
47 47​
48 test_helper.process(self.ref_input, self.ref_envdata) 48​ test_helper.process(self.ref_input, self.ref_envdata)
49 49​
50 getrpm_calls = stub_koji.calls() 50​ getrpm_calls = stub_koji.calls()
51 requested_nvr = getrpm_calls[0][1][0] 51​ requested_nvr = getrpm_calls[0][1][0]
52​ requested_src = getrpm_calls[0][2]['src']
52 53​
53 assert len(getrpm_calls) == 1 54​ assert len(getrpm_calls) == 1
54 assert requested_nvr == self.ref_nvr 55​ assert requested_nvr == self.ref_nvr
56​ assert not requested_src
55 57​
56 def test_parse_download_tag_command(self): 58​ def test_parse_download_tag_command(self):
57 59​
58 self.ref_input = {'action': 'download_tag', 60​ self.ref_input = {'action': 'download_tag',
59 'arch': self.ref_arch, 61​ 'arch': self.ref_arch,
60 'koji_tag': self.ref_tag} 62​ 'koji_tag': self.ref_tag}
61 63​
62 stub_koji = Dingus(get_tagged_rpms__returns=self.ref_rpms) 64​ stub_koji = Dingus(get_tagged_rpms__returns=self.ref_rpms)
Show All 39 Lines103 ref_input = {'action': 'download', 'arch': ['x86_64'],
102 'koji_build': self.ref_nvr} 104​ 'koji_build': self.ref_nvr}
103 ref_envdata = {'workdir': '/var/tmp/foo'} 105​ ref_envdata = {'workdir': '/var/tmp/foo'}
104 106​
105 stub_koji = Dingus() 107​ stub_koji = Dingus()
106 test_helper = koji_directive.KojiDirective(stub_koji) 108​ test_helper = koji_directive.KojiDirective(stub_koji)
107 test_helper.process(ref_input, ref_envdata) 109​ test_helper.process(ref_input, ref_envdata)
108 110​
109 getrpm_calls = stub_koji.calls() 111​ getrpm_calls = stub_koji.calls()
110 print getrpm_calls 112​ requested_arches = getrpm_calls[0][2]['arches']
111 requested_arches = getrpm_calls[0][1][2] ​
112 113​
113 assert len(getrpm_calls) == 1 114​ assert len(getrpm_calls) == 1
114 assert 'x86_64' in requested_arches 115​ assert 'x86_64' in requested_arches
115 assert 'noarch' in requested_arches 116​ assert 'noarch' in requested_arches
116 117​
117 def test_add_noarch_download_tag_command(self): 118​ def test_add_noarch_download_tag_command(self):
118 ref_input = {'action': 'download_tag', 'arch': ['x86_64'], 119​ ref_input = {'action': 'download_tag', 'arch': ['x86_64'],
119 'koji_tag': self.ref_tag} 120​ 'koji_tag': self.ref_tag}
120 ref_envdata = {'workdir': '/var/tmp/foo'} 121​ ref_envdata = {'workdir': '/var/tmp/foo'}
121 122​
122 stub_koji = Dingus(get_tagged_rpms__returns=self.ref_rpms) 123​ stub_koji = Dingus(get_tagged_rpms__returns=self.ref_rpms)
123 test_helper = koji_directive.KojiDirective(stub_koji) 124​ test_helper = koji_directive.KojiDirective(stub_koji)
124 test_helper.process(ref_input, ref_envdata) 125​ test_helper.process(ref_input, ref_envdata)
125 126​
126 getrpm_calls = stub_koji.calls() 127​ getrpm_calls = stub_koji.calls()
127 requested_arches = getrpm_calls[0][1][2] 128​ requested_arches = getrpm_calls[0][1][2]
128 129​
129 assert len(getrpm_calls) == 1 130​ assert len(getrpm_calls) == 1
130 assert 'x86_64' in requested_arches 131​ assert 'x86_64' in requested_arches
131 assert 'noarch' in requested_arches 132​ assert 'noarch' in requested_arches
133​
134​ def test__download_command_src(self):
135​ ref_input = {'action': 'download', 'arch': ['src'],
136​ 'koji_build': self.ref_nvr}
137​ ref_envdata = {'workdir': '/var/tmp/foo'}
138​
139​ stub_koji = Dingus()
140​ test_helper = koji_directive.KojiDirective(stub_koji)
141​ test_helper.process(ref_input, ref_envdata)
142​
143​ getrpm_calls = stub_koji.calls()
144​ requested_arches = getrpm_calls[0][2]['arches']
145​ requested_src = getrpm_calls[0][2]['src']
146​
147​ assert len(getrpm_calls) == 1
148​ assert ['src'] == requested_arches
149​ assert requested_src