Differential D919

koji_utils: do not raise when arches end up being an empty list
ClosedPublic
Actions

Authored by kparal on Jul 4 2016, 1:04 PM.

Details

Reviewers
mkrizek
Group Reviewers
libtaskotron
Commits
rLTRN5ad150025ffe: koji_utils: do not raise when arches end up being an empty list
Summary

Be less strict about arches and related input parameters resulting in
an empty list. Sometimes it's hard to check in advance whether the final
arch list will end up empty or not (task formula). The worst case
scenario is that the method will do nothing - not download anything.
That seems more user friendly than raising an error.

Test Plan

test suite adjusted

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 koji_utils: do not raise when arches end up being an empty list.Jul 4 2016, 1:04 PM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added a reviewer: libtaskotron.
mkrizek accepted this revision.Jul 7 2016, 10:27 AM
mkrizek added a reviewer: mkrizek.
mkrizek added a subscriber: mkrizek.

LGTM

libtaskotron/ext/fedora/koji_utils.py
204

Maybe we can return [] right away?

This revision is now accepted and ready to land.Jul 7 2016, 10:27 AM
Closed by commit rLTRN5ad150025ffe: koji_utils: do not raise when arches end up being an empty list (authored by kparal). · Explain WhyJul 7 2016, 10:49 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 2348

View Options

libtaskotron/directives/koji_directive.py

Show First 20 Lines • Show All 71 Lines • ▼ Show 20 Line(s)68 target_dir:
72 default: ${workdir} 72​ default: ${workdir}
73returns: | 73​returns: |
74 A dictionary containing following items: 74​ A dictionary containing following items:
75 75​
76 * `downloaded_rpms`: (list of str) a list of local filenames of the downloaded 76​ * `downloaded_rpms`: (list of str) a list of local filenames of the downloaded
77 RPMs 77​ RPMs
78raises: | 78​raises: |
79 * :class:`.TaskotronRemoteError`: if downloading failed 79​ * :class:`.TaskotronRemoteError`: if downloading failed
80 * :class:`.TaskotronValueError`: if the combination of ``arch``, ``arch_exclude`` and ``src`` ​
81 results in nothing to be downloaded ​
82version_added: 0.4 80​version_added: 0.4
83""" 81​"""
84 82​
85EXAMPLES = """ 83​EXAMPLES = """
86Rpmlint needs to download a specific build from Koji, all architectures 84​Rpmlint needs to download a specific build from Koji, all architectures
87including src.rpm:: 85​including src.rpm::
88 86​
89 - name: download rpms from koji 87​ - name: download rpms from koji
▲ Show 20 Lines • Show All 135 Lines • Show Last 20 Lines
View Options

libtaskotron/ext/fedora/koji_utils.py

Show First 20 Lines • Show All 153 Lines • ▼ Show 20 Line(s)142 def nvr_to_urls(self, nvr, arches=['all'], arch_exclude=[], debuginfo=False, src=True):
154 :param arch_exclude: exclude some specific arches, even if they are specified in 154​ :param arch_exclude: exclude some specific arches, even if they are specified in
155 ``arches`` or they are implicit in the default ``'all'`` value 155​ ``arches`` or they are implicit in the default ``'all'`` value
156 :type arch_exclude: list of str 156​ :type arch_exclude: list of str
157 :param bool debuginfo: whether to provide URLs for debuginfo RPM files 157​ :param bool debuginfo: whether to provide URLs for debuginfo RPM files
158 or ignore them 158​ or ignore them
159 :param bool src: whether to include a URL for the source RPM 159​ :param bool src: whether to include a URL for the source RPM
160 :rtype: list of str 160​ :rtype: list of str
161 :raise TaskotronRemoteError: when the requested build doesn't exist 161​ :raise TaskotronRemoteError: when the requested build doesn't exist
162 :raise TaskotronValueError: if ``arches=[]`` and ``src=False``, ​
163 therefore there is nothing to query for ​
164 ''' 162​ '''
165 log.info('Querying Koji for a list of RPMS for: %s', nvr) 163​ log.info('Querying Koji for a list of RPMS for: %s', nvr)
166 164​
167 # add binary arches if base arches are specified 165​ # add binary arches if base arches are specified
168 archlist = list(arches) 166​ archlist = list(arches)
169 for arch in arches: 167​ for arch in arches:
170 if arch in arch_utils.Arches.base: 168​ if arch in arch_utils.Arches.base:
171 archlist.extend(arch_utils.Arches.binary[arch]) 169​ archlist.extend(arch_utils.Arches.binary[arch])
172 arches = list(set(archlist)) 170​ arches = list(set(archlist))
173 171​
174 # if src is enabled, we need to add it so that it is included in the 172​ # if src is enabled, we need to add it so that it is included in the
175 # Koji query 173​ # Koji query
176 if src and ('src' not in arches): 174​ if src and ('src' not in arches):
177 arches.append('src') 175​ arches.append('src')
178 176​
179 # exclude unwanted arches 177​ # exclude unwanted arches
180 arches = [arch for arch in arches if arch not in arch_exclude] 178​ arches = [arch for arch in arches if arch not in arch_exclude]
181 179​
182 # if "nothing" is requested, it's probably an error ​
183 if not arches: 180​ if not arches:
184 raise exc.TaskotronValueError('Nothing to query for, the combination of `arches`, ' 181​ log.debug('Nothing to query for, the combination of `arches`, `arch_exclude` and '
185 '`arch_exclude` and `src` ended up an empty list') 182​ '`src` ended up an empty list.')
183​ return []
186 184​
187 # find the koji build 185​ # find the koji build
188 info = self.session.getBuild(nvr) 186​ info = self.session.getBuild(nvr)
189 if info is None: 187​ if info is None:
190 raise exc.TaskotronRemoteError("No such build found in Koji: %s" % nvr) 188​ raise exc.TaskotronRemoteError("No such build found in Koji: %s" % nvr)
191 189​
192 # list its RPM files 190​ # list its RPM files
193 req_arches = None if 'all' in arches else arches 191​ req_arches = None if 'all' in arches else arches
194 rpms = self.session.listRPMs(buildID=info['id'], arches=req_arches) 192​ rpms = self.session.listRPMs(buildID=info['id'], arches=req_arches)
195 if not debuginfo: 193​ if not debuginfo:
196 rpms = [r for r in rpms if (not r['name'].endswith('-debuginfo')) and 194​ rpms = [r for r in rpms if (not r['name'].endswith('-debuginfo')) and
197 ('-debuginfo-' not in r['name'])] 195​ ('-debuginfo-' not in r['name'])]
198 if not src: 196​ if not src:
199 rpms = [r for r in rpms if not r['arch'] == 'src'] 197​ rpms = [r for r in rpms if r['arch'] != 'src']
200 # exclude arches once again, in case `arch=['all']` was used 198​ # exclude arches once again, in case `arch=['all']` was used
201 rpms = [r for r in rpms if not r['arch'] in arch_exclude] 199​ rpms = [r for r in rpms if not r['arch'] in arch_exclude]
202 200​
201​ if not rpms:
202​ log.debug('After excluding all unwanted arches and RPM files, there is no RPM file '
203​ 'left. Returning an empty list.')
204​ return []
mkrizekUnsubmitted
Not Done

Maybe we can return [] right away?

205​
203 # create URLs 206​ # create URLs
204 baseurl = '/'.join((config.get_config().pkg_url, info['package_name'], 207​ baseurl = '/'.join((config.get_config().pkg_url, info['package_name'],
205 info['version'], info['release'])) 208​ info['version'], info['release']))
206 urls = ['%s/%s' % (baseurl, koji.pathinfo.rpm(r)) for r in rpms] 209​ urls = ['%s/%s' % (baseurl, koji.pathinfo.rpm(r)) for r in rpms]
207 210​
208 return sorted(urls) 211​ return sorted(urls)
209 212​
210 def get_nvr_rpms(self, nvr, dest, arches=['all'], arch_exclude=[], debuginfo=False, src=False): 213​ def get_nvr_rpms(self, nvr, dest, arches=['all'], arch_exclude=[], debuginfo=False, src=False):
211 '''Retrieve the RPMs associated with a build NVR into the specified 214​ '''Retrieve the RPMs associated with a build NVR into the specified
212 directory. 215​ directory.
213 216​
214 For parameters undocumented here, see :meth:`nvr_to_urls`. 217​ For parameters undocumented here, see :meth:`nvr_to_urls`.
215 218​
216 :param str nvr: build NVR 219​ :param str nvr: build NVR
217 :param str dest: location where to store the RPMs 220​ :param str dest: location where to store the RPMs
218 :return: list of local filenames of the grabbed RPMs (might be empty, 221​ :return: list of local filenames of the grabbed RPMs (might be empty,
219 according to your option choices and the particular NVR) 222​ according to your option choices and the particular NVR)
220 :rtype: list of str 223​ :rtype: list of str
221 :raise TaskotronRemoteError: if the files can't be downloaded 224​ :raise TaskotronRemoteError: if the files can't be downloaded
222 :raise TaskotronValueError: if ``arches=[]`` and ``src=False``, ​
223 therefore there is nothing to download ​
224 ''' 225​ '''
225 # always create dest dir, even if nothing gets downloaded 226​ # always create dest dir, even if nothing gets downloaded
226 file_utils.makedirs(dest) 227​ file_utils.makedirs(dest)
227 228​
228 rpm_urls = self.nvr_to_urls(nvr, arches, arch_exclude, debuginfo, src) 229​ rpm_urls = self.nvr_to_urls(nvr, arches, arch_exclude, debuginfo, src)
229 230​
230 rpm_files = [] 231​ rpm_files = []
231 log.info('Fetching %s RPMs for: %s (into %s)', len(rpm_urls), nvr, dest) 232​ log.info('Fetching %s RPMs for: %s (into %s)', len(rpm_urls), nvr, dest)
▲ Show 20 Lines • Show All 61 Lines • Show Last 20 Lines
View Options

testing/test_koji_utils.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2016, Red Hat, Inc. 2​# Copyright 2009-2016, 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​
6'''Unit tests for libtaskotron/koji_utils.py''' 6​'''Unit tests for libtaskotron/koji_utils.py'''
7 7​
8import pytest 8​import pytest
9from dingus import Dingus 9​from dingus import Dingus
10import os 10​import os
11import itertools 11​import itertools
12​import mock
12 13​
13from libtaskotron.ext.fedora import koji_utils 14​from libtaskotron.ext.fedora import koji_utils
14from libtaskotron import exceptions as exc 15​from libtaskotron import exceptions as exc
15from libtaskotron import config 16​from libtaskotron import config
16from libtaskotron import file_utils 17​from libtaskotron import file_utils
17 18​
18from test_file_utils import mock_download 19​from test_file_utils import mock_download
19 20​
▲ Show 20 Lines • Show All 195 Lines • ▼ Show 20 Line(s)213 def test_nvr_to_urls_only_src(self):
215 216​
216 # arch disabled, src enabled 217​ # arch disabled, src enabled
217 test_koji.nvr_to_urls(self.ref_nvr, arches=[], src=True) 218​ test_koji.nvr_to_urls(self.ref_nvr, arches=[], src=True)
218 listrpm_calls = stub_koji.calls('listRPMs') 219​ listrpm_calls = stub_koji.calls('listRPMs')
219 requested_arches = listrpm_calls[0][2]['arches'] 220​ requested_arches = listrpm_calls[0][2]['arches']
220 assert len(requested_arches) == 1 221​ assert len(requested_arches) == 1
221 assert requested_arches[0] == 'src' 222​ assert requested_arches[0] == 'src'
222 223​
223 def should_raise_no_arch_no_src(self): 224​ def should_not_query_no_arch_no_src(self):
224 test_koji = koji_utils.KojiClient(Dingus()) 225​ '''If there's no arch to query for, should not perform the query'''
226​ stub_koji = mock.Mock()
227​ test_koji = koji_utils.KojiClient(stub_koji)
225 228​
226 with pytest.raises(exc.TaskotronValueError): 229​ test_koji.nvr_to_urls(self.ref_nvr, arches=[], src=False)
227 test_koji.nvr_to_urls(self.ref_nvr, arches=[], src=False) 230​ assert not stub_koji.called
228 231​
229 def test_arch_exclude(self): 232​ def test_arch_exclude(self):
230 stub_koji = Dingus(getBuild__returns=self.ref_build, 233​ stub_koji = Dingus(getBuild__returns=self.ref_build,
231 listRPMs__returns=self.ref_rpms) 234​ listRPMs__returns=self.ref_rpms)
232 test_koji = koji_utils.KojiClient(stub_koji) 235​ test_koji = koji_utils.KojiClient(stub_koji)
233 ref_exclude = ['x86_64'] 236​ ref_exclude = ['x86_64']
234 237​
235 # arch_exclude disabled 238​ # arch_exclude disabled
▲ Show 20 Lines • Show All 170 Lines • Show Last 20 Lines