libtaskotron/koji_utils.py
37–38	since this is not used anymore, please remove the line and the respective comment
37–38	Add comment explaining that the buildinfos are returned in the same respective order as the rpms on input.
53	Please explain in a comment why this (and the other similar) block of code is written like this.

libtaskotron/koji_utils.py
41–42	If you want to be more precise here, you can use: :param rpms: ... :type rpms: list of str
44–45	Just a note, I like to highlight param names which makes the descriptions simpler and clearer. So for example you can write it like ... in the same respective order as in ``rpms``
63–75	I understand the purpose here - check error codes and call further queries simultaneously. However, would it be more readable if you split this into two separate cycles? Like this: # check errors for rpminfo in rpminfos: if isinstance(rpminfo, dict): raise ... # request builds self.session.multicall = True for rpminfo in rpminfos: self.session.getBuild(rpminfo[0]['build_id']) buildinfos = self.session.multiCall()
67	`map`->`dict`, if you edited this
72	Can you please also specify which RPM caused the problem and the response from Koji (error code and string)?
73	If you wanted to get really fancy, you could eliminate duplicate queries to the same `build_id`s. There will be probably a lot of them. You could make the list unique, and then match the results back to original list of RPMs.
81–82	Again, let's show the Koji response.
testing/test_koji_utils.py
77–87	I guess this could be extracted into a separate method that would accept two input arguments, and the return them on the first and the second call? Then you would use this method from both `test_*` methods (and possibly some other methods in the future).

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	507	2294b06		Jul 3 2014, 8:52 AM	★	★
Diff 2	512	2294b06	- add comments for rpms_to_build	Jul 3 2014, 9:51 AM	★	★
Diff 3	534	2294b06	- some changes to rpms_to_build	Jul 8 2014, 10:08 AM	★	★
Diff 4	542	2294b06	- remove redundant builds from query	Jul 9 2014, 9:59 AM	★	★
Diff 5	548	a794e09	Commit rLTRNe80dc4835d60547cce1152c495c4e6f0d7832d7a	Jul 11 2014, 8:31 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
e7321d4c1ae5	34c27c256186	cfe6519e2b75	Garret Raziel	remove redundant builds from query	Jul 9 2014, 9:59 AM
cfe6519e2b75	088fc0fddb6b	9f156f7f8430	Garret Raziel	some changes to rpms_to_build	Jul 8 2014, 10:07 AM
9f156f7f8430	ba02108bdb89	89a546e2acd4	Garret Raziel	add comments for rpms_to_build	Jul 3 2014, 9:51 AM
89a546e2acd4	58e12074f2b9	2294b06707e7	Garret Raziel	use multiCall in rpm_to_build in koji utils (Show More…)	Jul 3 2014, 8:45 AM

	Status	Author	Revision
	Closed	garretraziel	D176 use libtaskotron's rpms_to_build with multicall()
	Closed	garretraziel	D175 use multiCall in rpm_to_build in koji utils

Diff 548

libtaskotron/koji_utils.py

Show All 28 Lines		28	def __init__(self, koji_session=None):
29	'''Create a new KojiClient	29		'''Create a new KojiClient
30		30
31	:param koji_session: an existing Koji session instance or ``None`` if	31		:param koji_session: an existing Koji session instance or ``None`` if
32	you want a new default session to be created	32		you want a new default session to be created
33	:type koji_session: :class:`koji.ClientSession`	33		:type koji_session: :class:`koji.ClientSession`
34	'''	34		'''
35	self.session = (koji_session or	35		self.session = (koji_session or
36	koji.ClientSession(config.get_config().koji_url))	36		koji.ClientSession(config.get_config().koji_url))
37		37
38	# contains a rpm_filename -> build info mapping used in rpm_to_build	38		def rpms_to_build(self, rpms):
			jskladanUnsubmitted Not Done since this is not used anymore, please remove the line and the respective comment
			jskladanUnsubmitted Not Done Add comment explaining that the buildinfos are returned in the same respective order as the rpms on input.
39	self._rpm_to_build_cache = {}	39		'''Get list of koji build objects for the rpms. Order of koji objects
40		40		in this list is the same as order of the respective rpm objects.
41		41
42	def rpm_to_build(self, rpm, prefetch=True):	42		:param rpms: list of filenames as either ``/my/path/nvr.a.rpm`` or
			kparalUnsubmitted Not Done If you want to be more precise here, you can use: :param rpms: ... :type rpms: list of str
43	'''Get koji build object for the rpm.	43		``nvr.a.rpm``
44		44		:type rpms: list of str
45	:param str rpm: filename as either ``/my/path/nvr.a.rpm`` or ``nvr.a.rpm``	45		:return: list of Koji buildinfo dictionaries (as returned e.g.
			kparalUnsubmitted Not Done Just a note, I like to highlight param names which makes the descriptions simpler and clearer. So for example you can write it like ... in the same respective order as in ``rpms``
46	:param bool prefetch: if set to True, get list of all rpms for the build	46		from :meth:`koji.getBuild`) in the same respective order as in``rpms``
47	and store it for future use - this will speed up subsequent queries	47		:rtype: list
48	for rpms belonging to already queried build.	48		:raise TaskotronRemoteError: if rpm or it's related build is not
49	:return: Koji buildinfo dictionary (as returned e.g. from :meth:`koji.getBuild`)	49		found
50	:rtype: :class:`bunch.Bunch`
51	:raise TaskotronRemoteError: if rpm or it's related build is not found
52	'''	50		'''
53		51
54	# extract filename from the (possible) path	52		self.session.multicall = True
55	rpm = os.path.split(rpm)[-1]	53		for rpm in rpms:
			jskladanUnsubmitted Not Done Please explain in a comment why this (and the other similar) block of code is written like this.
56		54		# extract filename from the (possible) path
57	if rpm in self._rpm_to_build_cache:	55		rpm = os.path.split(rpm)[-1]
58	return self._rpm_to_build_cache[rpm]	56
59		57		self.session.getRPM(rpm)
60	rpminfo = self.session.getRPM(rpm)	58
61	if rpminfo is None:	59		# the list will contain one element for each method added to the
62	raise exc.TaskotronRemoteError('RPM not found')	60		# multicall, in the order it was added to the multicall
63		61		rpminfos = self.session.multiCall()
64	buildinfo = self.session.getBuild(rpminfo['build_id'])	62
65	if buildinfo is None:	63		# because it is probable that several rpms will come from the same build
66	raise exc.TaskotronRemoteError('Build %r not found' % rpminfo['build_id'])	64		# use set for builds
67		65		builds = set()
68	if prefetch:	66		for i, rpminfo in enumerate(rpminfos):
69	rpms = self.session.listRPMs(rpminfo['build_id'])	67		# according to documentation, multiCall() returns list, where
			kparalUnsubmitted Not Done `map`->`dict`, if you edited this
70	for r in rpms:	68		# each element will be either a one-element list containing the
71	filename = "%s.%s.rpm" % (r['nvr'], r['arch'])	69		# result of the method call, or a dict containing "faultCode" and
72	self._rpm_to_build_cache[filename] = buildinfo	70		# "faultString" keys, describing the error that occurred during the
73	else:	71		# method call.
74	self._rpm_to_build_cache[rpm] = buildinfo	72		#
			kparalUnsubmitted Not Done Can you please also specify which RPM caused the problem and the response from Koji (error code and string)?
75		73		# getRPM() returns None if there is no RPM with the given ID
			kparalUnsubmitted Not Done If you wanted to get really fancy, you could eliminate duplicate queries to the same `build_id`s. There will be probably a lot of them. You could make the list unique, and then match the results back to original list of RPMs.
76	return buildinfo	74		if isinstance(rpminfo, dict):
77		75		raise exc.TaskotronRemoteError('Problem with RPM %s: %d: %s' % (
			kparalUnsubmitted Not Done I understand the purpose here - check error codes and call further queries simultaneously. However, would it be more readable if you split this into two separate cycles? Like this: # check errors for rpminfo in rpminfos: if isinstance(rpminfo, dict): raise ... # request builds self.session.multicall = True for rpminfo in rpminfos: self.session.getBuild(rpminfo[0]['build_id']) buildinfos = self.session.multiCall()
		76		rpms[i], rpminfo["faultCode"], rpminfo["faultString"]))
		77		elif rpminfo[0] is None:
		78		raise exc.TaskotronRemoteError('RPM %s not found' % rpms[i])
		79		else:
		80		builds.add(rpminfo[0]['build_id'])
		81
		82		builds = list(builds) # so that we could use build order
			kparalUnsubmitted Not Done Again, let's show the Koji response.
		83		self.session.multicall = True
		84		for build in builds:
		85		self.session.getBuild(build)
		86
		87		buildinfos = self.session.multiCall()
		88
		89		for i, buildinfo in enumerate(buildinfos):
		90		# see ^
		91		if isinstance(buildinfo, dict):
		92		raise exc.TaskotronRemoteError(
		93		'Problem with build %s: %d: %s' % (
		94		builds[i], buildinfo["faultCode"],
		95		buildinfo["faultString"]))
		96		elif buildinfo[0] is None:
		97		raise exc.TaskotronRemoteError(
		98		'Build %s not found' % builds[i])
		99
		100		build_to_buildinfo = dict(zip(builds, buildinfos))
		101		result = []
		102		for rpminfo in rpminfos:
		103		build_id = rpminfo[0]['build_id']
		104		result.append(build_to_buildinfo[build_id][0])
		105
		106		return result
78		107
79	def nvr_to_urls(self, nvr, arches=None, debuginfo=False, src=True):	108		def nvr_to_urls(self, nvr, arches=None, debuginfo=False, src=True):
80	'''Get list of URLs for RPMs corresponding to a build.	109		'''Get list of URLs for RPMs corresponding to a build.
81		110
82	:param str nvr: build NVR	111		:param str nvr: build NVR
83	:param arches: restrict the arches of builds to provide URLs for. Note:	112		:param arches: restrict the arches of builds to provide URLs for. Note:
84	If basearch ``i386`` is in the list, ``i686`` arch is	113		If basearch ``i386`` is in the list, ``i686`` arch is
85	automatically added (since that's the arch Koji API uses).	114		automatically added (since that's the arch Koji API uses).
▲ Show 20 Lines • Show All 106 Lines • Show Last 20 Lines

View Options

testing/test_koji_utils.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
6	'''Unit tests for libtaskotron/koji_utils.py'''	6		'''Unit tests for libtaskotron/koji_utils.py'''
7		7
8	import pytest	8		import pytest
9	from dingus import Dingus	9		from dingus import Dingus
10		10
11	from libtaskotron import koji_utils	11		from libtaskotron import koji_utils
12	from libtaskotron import exceptions as exc	12		from libtaskotron import exceptions as exc
13	from libtaskotron import config	13		from libtaskotron import config
14		14
		15
		16		# http://stackoverflow.com/questions/3190706/nonlocal-keyword-in-python-2-x
		17		def create_multicall(first, second):
		18		first_call = {"value": True}
		19
		20		def multicall():
		21		if first_call["value"]:
		22		first_call["value"] = False
		23		return first
		24		else:
		25		return second
		26
		27		return multicall
		28
		29
15	class TestKojiClient():	30		class TestKojiClient():
16	def setup_method(self, method):	31		def setup_method(self, method):
17	self.ref_nvr = 'foo-1.2-3.fc99'	32		self.ref_nvr = 'foo-1.2-3.fc99'
18	self.ref_arch = 'noarch'	33		self.ref_arch = 'noarch'
19	self.ref_name = 'foo'	34		self.ref_name = 'foo'
20	self.ref_version = '1.2'	35		self.ref_version = '1.2'
21	self.ref_release = '3.fc99'	36		self.ref_release = '3.fc99'
22	self.ref_buildid = 123456	37		self.ref_buildid = 123456
23	self.ref_filename = "%s.%s.rpm" % (self.ref_nvr, self.ref_arch)	38		self.ref_filename = "%s.%s.rpm" % (self.ref_nvr, self.ref_arch)
24		39
25	self.ref_build = {'package_name': self.ref_name,	40		self.ref_build = {'package_name': self.ref_name,
26	'version': self.ref_version,	41		'version': self.ref_version,
27	'release': self.ref_release,	42		'release': self.ref_release,
28	'id': self.ref_buildid}	43		'id': self.ref_buildid}
29		44
30	self.ref_rpms = [{'name': self.ref_name, 'version': self.ref_version,	45		self.ref_rpms = [{'name': self.ref_name, 'version': self.ref_version,
31	'release': self.ref_release, 'nvr': self.ref_nvr,	46		'release': self.ref_release, 'nvr': self.ref_nvr,
32	'arch': self.ref_arch, 'build_id': self.ref_buildid,},	47		'arch': self.ref_arch, 'build_id': self.ref_buildid,},
33	{'name': self.ref_name, 'version': self.ref_version,	48		{'name': self.ref_name, 'version': self.ref_version,
34	'release': self.ref_release, 'nvr': self.ref_nvr,	49		'release': self.ref_release, 'nvr': self.ref_nvr,
35	'arch': 'src', 'build_id': self.ref_buildid, }]	50		'arch': 'src', 'build_id': self.ref_buildid, }]
36		51
37	def test_rpm_to_build(self):	52		def test_rpms_to_build(self):
38	stub_koji = Dingus(getBuild__returns = self.ref_build,	53		stub_koji = Dingus(getBuild__returns=None,
39	listRPMs__returns = self.ref_rpms,	54		getRPM__returns=None,
40	getRPM__returns = self.ref_rpms[0])	55		multiCall=create_multicall(
		56		[[self.ref_rpms[0]], [self.ref_rpms[1]]],
		57		[[self.ref_build], [self.ref_build]]))
41		58
42	test_koji = koji_utils.KojiClient(stub_koji)	59		test_koji = koji_utils.KojiClient(stub_koji)
43	outcome = test_koji.rpm_to_build(self.ref_filename)	60		outcome = test_koji.rpms_to_build([self.ref_filename,self.ref_filename])
44		61
45	assert outcome is self.ref_build	62		assert outcome == [self.ref_build, self.ref_build]
		63		# because two rpms come from same build, it gets called twice for each
		64		# rpm, once for build
46	assert len(stub_koji.calls) == 3	65		assert len(stub_koji.calls) == 3
47		66
48	# test prefetch
49	outcome = test_koji.rpm_to_build(self.ref_filename)
50	assert len(stub_koji.calls) == 3
51		67
52	def test_rpm_to_build_exceptions(self):	68		def test_rpms_to_build_exceptions(self):
53	stub_koji = Dingus(getRPM__returns = None)	69		stub_koji = Dingus(getRPM__returns=None,
		70		multiCall__returns=[{"faultCode": -1,
		71		"faultString": "failed"}])
54	test_koji = koji_utils.KojiClient(stub_koji)	72		test_koji = koji_utils.KojiClient(stub_koji)
55		73
56	with pytest.raises(exc.TaskotronRemoteError):	74		with pytest.raises(exc.TaskotronRemoteError):
57	test_koji.rpm_to_build(self.ref_filename)	75		test_koji.rpms_to_build([self.ref_filename])
58		76
		77		stub_koji = Dingus(getBuild__returns=None,
		78		getRPM__returns=None,
		79		multiCall=create_multicall([[self.ref_rpms[0]]], [
		80		{"faultCode": -1, "faultString": "failed"}]))
		81
		82		test_koji = koji_utils.KojiClient(stub_koji)
		83		with pytest.raises(exc.TaskotronRemoteError):
		84		test_koji.rpms_to_build([self.ref_filename])
		85
		86		stub_koji = Dingus(getBuild__returns=None,
		87		getRPM__returns=None,
			kparalUnsubmitted Not Done I guess this could be extracted into a separate method that would accept two input arguments, and the return them on the first and the second call? Then you would use this method from both `test_*` methods (and possibly some other methods in the future).
		88		multiCall__returns=[[None]])
		89
		90		test_koji = koji_utils.KojiClient(stub_koji)
		91		with pytest.raises(exc.TaskotronRemoteError):
		92		test_koji.rpms_to_build([self.ref_filename])
59		93
60	stub_koji = Dingus(getBuild__returns = None,	94		stub_koji = Dingus(getBuild__returns=None,
61	listRPMs__returns = self.ref_rpms,	95		getRPM__returns=None,
62	getRPM__returns = self.ref_rpms[0])	96		multiCall=create_multicall([[self.ref_rpms[0]]],
		97		[[None]]))
63		98
64	test_koji = koji_utils.KojiClient(stub_koji)	99		test_koji = koji_utils.KojiClient(stub_koji)
65	with pytest.raises(exc.TaskotronRemoteError):	100		with pytest.raises(exc.TaskotronRemoteError):
66	test_koji.rpm_to_build(self.ref_filename)	101		test_koji.rpms_to_build([self.ref_filename])
67		102
68		103
69	def test_get_noarch_rpmurls_from_nvr(self):	104		def test_get_noarch_rpmurls_from_nvr(self):
70	stub_koji = Dingus(getBuild__returns = self.ref_build,	105		stub_koji = Dingus(getBuild__returns = self.ref_build,
71	listRPMs__returns = self.ref_rpms)	106		listRPMs__returns = self.ref_rpms)
72		107
73	test_koji = koji_utils.KojiClient(stub_koji)	108		test_koji = koji_utils.KojiClient(stub_koji)
74		109
▲ Show 20 Lines • Show All 75 Lines • Show Last 20 Lines

			Path	Packages
M			libtaskotron/koji_utils.py (105 lines)
M			testing/test_koji_utils.py (67 lines)

use multiCall in rpm_to_build in koji utils
ClosedPublic
Actions

Details

Diff Detail

Revision Contents

Diff 548

libtaskotron/koji_utils.py

testing/test_koji_utils.py

use multiCall in rpm_to_build in koji utilsClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 548

libtaskotron/koji_utils.py

testing/test_koji_utils.py

use multiCall in rpm_to_build in koji utils
ClosedPublic
Actions