From 216935617595f09fbe954149833765ab35f3a21f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 12:47:20 +0000 Subject: [PATCH 1/11] hub: add with_owners option to readPackageList Related: https://pagure.io/koji/issue/2780 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index cdcdfe0..f97569d 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1124,7 +1124,7 @@ def pkglist_setarches(taginfo, pkginfo, arches, force=False): def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=False, - with_dups=False): + with_dups=False, with_owners=True): """Returns the package list for the specified tag or user. One of (tagID,userID,pkgID) must be specified @@ -1134,41 +1134,39 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal if tagID is None and userID is None and pkgID is None: raise koji.GenericError('tag,user, and/or pkg must be specified') - packages = {} - fields = (('package.id', 'package_id'), ('package.name', 'package_name'), - ('tag.id', 'tag_id'), ('tag.name', 'tag_name'), - ('users.id', 'owner_id'), ('users.name', 'owner_name'), - ('extra_arches', 'extra_arches'), - ('tag_packages.blocked', 'blocked')) - flist = ', '.join([pair[0] for pair in fields]) - cond1 = eventCondition(event, table='tag_packages') - cond2 = eventCondition(event, table='tag_package_owners') - q = """ - SELECT %(flist)s - FROM tag_packages - JOIN tag on tag.id = tag_packages.tag_id - JOIN package ON package.id = tag_packages.package_id - JOIN tag_package_owners ON - tag_packages.tag_id = tag_package_owners.tag_id AND - tag_packages.package_id = tag_package_owners.package_id - JOIN users ON users.id = tag_package_owners.owner - WHERE %(cond1)s AND %(cond2)s""" + if userID and not with_owners: + raise koji.GenericError("userID and with_owners=False can't be used together") + + tables = ['tag_packages'] + fields = ['package.id', 'package.name', 'tag.id', 'tag.name', 'extra_arches', + 'tag_packages.blocked'] + aliases = ['package_id', 'package_name', 'tag_id', 'tag_name', 'extra_arches', 'blocked'] + joins = ['tag ON tag.id = tag_packages.tag_id', + 'package ON package.id = tag_packages.package_id'] + clauses = [eventCondition(event, table='tag_packages')] if tagID is not None: - q += """ - AND tag.id = %%(tagID)i""" + clauses.append('tag.id = %(tagID)i') if userID is not None: - q += """ - AND users.id = %%(userID)i""" + clauses.append('users.id = %(userID)i') if pkgID is not None: if isinstance(pkgID, int): - q += """ - AND package.id = %%(pkgID)i""" + clauses.append('package.id = %(pkgID)i') else: - q += """ - AND package.name = %%(pkgID)s""" + clauses.append('package.name = %(pkgID)s') + if with_owners: + fields += ['users.id', 'users.name'] + aliases += ['owner_id', 'owner_name'] + joins += [ + 'tag_package_owners ON tag_packages.tag_id = tag_package_owners.tag_id AND \ + tag_packages.package_id = tag_package_owners.package_id', + 'users ON users.id = tag_package_owners.owner' + ] + clauses.append(eventCondition(event, table='tag_package_owners')) + query = QueryProcessor(columns=fields, aliases=aliases, tables=tables, joins=joins, + clauses=clauses, values=locals()) - q = q % locals() - for p in _multiRow(q, locals(), [pair[1] for pair in fields]): + packages = {} + for p in query.execute(): # things are simpler for the first tag pkgid = p['package_id'] if with_dups: @@ -1194,7 +1192,8 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal re_cache[pat] = prog re_list.append(prog) # same query as before, with different params - for p in _multiRow(q, locals(), [pair[1] for pair in fields]): + query.values['tagID'] = tagID + for p in query.execute(): pkgid = p['package_id'] if not with_dups and pkgid in packages: # previous data supercedes From 678f2c82f30bab51f4f5e141327e07c85b21bdc8 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 12:47:20 +0000 Subject: [PATCH 2/11] use with_owners=False --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 231a034..462dfd5 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -353,7 +353,7 @@ def handle_add_pkg(goptions, session, args): print("No such tag: %s" % tag) sys.exit(1) pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'])]) + for p in session.listPackages(tagID=dsttag['id'], with_owners=False)]) to_add = [] for package in args[1:]: package_id = pkglist.get(package, None) @@ -388,7 +388,8 @@ def handle_block_pkg(goptions, session, args): if dsttag is None: error("No such tag: %s" % tag) pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'], inherited=True)]) + for p in session.listPackages(tagID=dsttag['id'], inherited=True, + with_owners=False)]) ret = 0 for package in args[1:]: package_id = pkglist.get(package, None) @@ -425,7 +426,7 @@ def handle_remove_pkg(goptions, session, args): if dsttag is None: error("No such tag: %s" % tag) pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'])]) + for p in session.listPackages(tagID=dsttag['id'], with_owners=False)]) ret = 0 for package in args[1:]: package_id = pkglist.get(package, None) diff --git a/hub/kojihub.py b/hub/kojihub.py index f97569d..97845ad 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1069,7 +1069,7 @@ def pkglist_block(taginfo, pkginfo, force=False): # check pkg list existence tag = get_tag(taginfo, strict=True) pkg = lookup_package(pkginfo, strict=True) - if not readPackageList(tag['id'], pkgID=pkg['id'], inherit=True): + if not readPackageList(tag['id'], pkgID=pkg['id'], inherit=True, with_owners=False): raise koji.GenericError("Package %s is not in tag listing for %s" % (pkg['name'], tag['name'])) pkglist_add(taginfo, pkginfo, block=True, force=force) @@ -1312,7 +1312,8 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, # regardless of inherit setting, we need to use inheritance to read the # package list - packages = readPackageList(tagID=tag, event=event, inherit=True, pkgID=package) + packages = readPackageList(tagID=tag, event=event, inherit=True, pkgID=package, + with_owners=False) # these values are used for each iteration fields = [('tag.id', 'tag_id'), ('tag.name', 'tag_name'), ('build.id', 'id'), @@ -2430,7 +2431,7 @@ def maven_tag_archives(tag_id, event_id=None, inherit=True): For any parent tags where 'maven_include_all' is true, include all versions of a given groupId:artifactId, not just the most-recently-tagged. """ - packages = readPackageList(tagID=tag_id, event=event_id, inherit=True) + packages = readPackageList(tagID=tag_id, event=event_id, inherit=True, with_owners=False) taglist = [tag_id] if inherit: taglist.extend([link['parent_id'] for link in readFullInheritance(tag_id, event_id)]) @@ -2558,9 +2559,11 @@ def repo_init(tag, with_src=False, with_debuginfo=False, event=None, with_separa latest = not tinfo['extra'].get('repo_include_all', False) # Note: the repo_include_all option is not recommended for common use # see https://pagure.io/koji/issue/588 for background - rpms, builds = readTaggedRPMS(tag_id, event=event_id, inherit=True, latest=latest) + rpms, builds = readTaggedRPMS(tag_id, event=event_id, inherit=True, latest=latest, + with_owners=False) groups = readTagGroups(tag_id, event=event_id, inherit=True) - blocks = [pkg for pkg in readPackageList(tag_id, event=event_id, inherit=True).values() + blocks = [pkg for pkg in readPackageList(tag_id, event=event_id, inherit=True, + with_owners=False).values() if pkg['blocked']] repodir = koji.pathinfo.repo(repo_id, tinfo['name']) os.makedirs(repodir) # should not already exist @@ -10944,7 +10947,7 @@ class RootExports(object): if fromtag: assert_tag_access(fromtag_id, user_id=None, force=force) # package list check - pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True) + pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True, with_owners=False) pkg_error = None if pkg_id not in pkgs: pkg_error = "Package %s not in list for %s" % (build['name'], tag['name']) @@ -11027,7 +11030,7 @@ class RootExports(object): # note: we're just running the quick checks now so we can fail # early if appropriate, rather then waiting for the task # Make sure package is on the list for the tag we're adding it to - pkgs = readPackageList(tagID=tag2_id, pkgID=pkg_id, inherit=True) + pkgs = readPackageList(tagID=tag2_id, pkgID=pkg_id, inherit=True, with_owners=False) pkg_error = None if pkg_id not in pkgs: pkg_error = "Package %s not in list for tag %s" % (package, tag2) @@ -11847,7 +11850,7 @@ class RootExports(object): getPackage = staticmethod(lookup_package) def listPackages(self, tagID=None, userID=None, pkgID=None, prefix=None, inherited=False, - with_dups=False, event=None, queryOpts=None): + with_dups=False, event=None, queryOpts=None, with_owners=True): """List if tagID and/or userID is specified, limit the list to packages belonging to the given user or with the given tag. @@ -11879,7 +11882,7 @@ class RootExports(object): pkgID = get_package_id(pkgID, strict=True) result_list = list(readPackageList(tagID=tagID, userID=userID, pkgID=pkgID, inherit=inherited, with_dups=with_dups, - event=event).values()) + event=event, with_owners=with_owners).values()) if with_dups: # when with_dups=True, readPackageList returns a list of list of dicts # convert it to a list of dicts for consistency @@ -11927,7 +11930,7 @@ class RootExports(object): pkg_id = get_package_id(pkg, strict=False) if pkg_id is None or tag_id is None: return False - pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True) + pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True, with_owners=False) if pkg_id not in pkgs: return False else: @@ -14325,7 +14328,7 @@ class HostExports(object): # don't check policy for admins using force assert_policy('tag', policy_data, force=force) # package list check - pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True) + pkgs = readPackageList(tagID=tag_id, pkgID=pkg_id, inherit=True, with_owners=False) pkg_error = None if pkg_id not in pkgs: pkg_error = "Package %s not in list for %s" % (build['name'], tag) diff --git a/www/kojiweb/index.py b/www/kojiweb/index.py index b35782d..0aac456 100644 --- a/www/kojiweb/index.py +++ b/www/kojiweb/index.py @@ -968,7 +968,7 @@ def taginfo(environ, tagID, all='0', packageOrder='package_name', packageStart=N all = int(all) - numPackages = server.count('listPackages', tagID=tag['id'], inherited=True) + numPackages = server.count('listPackages', tagID=tag['id'], inherited=True, with_owners=False) numBuilds = server.count('listTagged', tag=tag['id'], inherit=True) values['numPackages'] = numPackages values['numBuilds'] = numBuilds From 2f4b08c8080bb042db22c0df00f2e3a5ece03a78 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 12:47:20 +0000 Subject: [PATCH 3/11] fix tests for new option --- diff --git a/tests/test_cli/test_add_pkg.py b/tests/test_cli/test_add_pkg.py index fc644a3..cf4a6cb 100644 --- a/tests/test_cli/test_add_pkg.py +++ b/tests/test_cli/test_add_pkg.py @@ -56,7 +56,7 @@ class TestAddPkg(utils.CliTestCase): activate_session_mock.assert_called_once_with(session, options) session.getUser.assert_called_once_with(owner) session.getTag.assert_called_once_with(tag) - session.listPackages.assert_called_once_with(tagID=dsttag['id']) + session.listPackages.assert_called_once_with(tagID=dsttag['id'], with_owners=False) session.packageListAdd.assert_called_once_with( tag, package, owner, **kwargs) session.multiCall.assert_called_once_with(strict=True) @@ -100,7 +100,7 @@ class TestAddPkg(utils.CliTestCase): self.assertEqual(session.mock_calls, [call.getUser(owner), call.getTag(tag), - call.listPackages(tagID=dsttag['id']), + call.listPackages(tagID=dsttag['id'], with_owners=False), call.packageListAdd(tag, packages[0], owner, **kwargs), call.packageListAdd(tag, packages[2], owner, **kwargs), call.multiCall(strict=True)]) diff --git a/tests/test_cli/test_block_pkg.py b/tests/test_cli/test_block_pkg.py index d2439c5..203187b 100644 --- a/tests/test_cli/test_block_pkg.py +++ b/tests/test_cli/test_block_pkg.py @@ -42,7 +42,7 @@ class TestBlockPkg(utils.CliTestCase): activate_session_mock.assert_called_once_with(session, options) session.getTag.assert_called_once_with(tag) session.listPackages.assert_called_once_with( - tagID=dsttag['id'], inherited=True) + tagID=dsttag['id'], inherited=True, with_owners=False) session.packageListBlock.assert_called_once_with( tag, package, force=True) session.multiCall.assert_called_once_with(strict=True) @@ -79,7 +79,7 @@ class TestBlockPkg(utils.CliTestCase): self.assertEqual( session.mock_calls, [ call.getTag(tag), - call.listPackages(tagID=dsttag['id'], inherited=True), + call.listPackages(tagID=dsttag['id'], inherited=True, with_owners=False), call.packageListBlock(tag, packages[0]), call.packageListBlock(tag, packages[1]), call.packageListBlock(tag, packages[2]), @@ -116,7 +116,7 @@ class TestBlockPkg(utils.CliTestCase): activate_session_mock.assert_called_once_with(session, options) session.getTag.assert_called_once_with(tag) session.listPackages.assert_called_once_with( - tagID=dsttag['id'], inherited=True) + tagID=dsttag['id'], inherited=True, with_owners=False) session.packageListBlock.assert_not_called() session.multiCall.assert_not_called() diff --git a/tests/test_cli/test_remove_pkg.py b/tests/test_cli/test_remove_pkg.py index 70f6a60..db12497 100644 --- a/tests/test_cli/test_remove_pkg.py +++ b/tests/test_cli/test_remove_pkg.py @@ -43,8 +43,7 @@ class TestRemovePkg(utils.CliTestCase): # Finally, assert that things were called as we expected. activate_session_mock.assert_called_once_with(session, options) session.getTag.assert_called_once_with(tag) - session.listPackages.assert_called_once_with( - tagID=dsttag['id']) + session.listPackages.assert_called_once_with(tagID=dsttag['id'], with_owners=False) session.packageListRemove.assert_called_once_with( tag, package, **kwargs) session.multiCall.assert_called_once_with(strict=True) @@ -80,12 +79,14 @@ class TestRemovePkg(utils.CliTestCase): activate_session_mock.assert_called_once_with(session, options) self.assertEqual( session.mock_calls, [ - call.getTag(tag), call.listPackages( - tagID=dsttag['id']), call.packageListRemove( - tag, packages[0], **kwargs), call.packageListRemove( - tag, packages[1], **kwargs), call.packageListRemove( - tag, packages[2], **kwargs), call.multiCall( - strict=True)]) + call.getTag(tag), + call.listPackages(tagID=dsttag['id'], with_owners=False), + call.packageListRemove(tag, packages[0], **kwargs), + call.packageListRemove(tag, packages[1], **kwargs), + call.packageListRemove(tag, packages[2], **kwargs), + call.multiCall( strict=True) + ] + ) self.assertNotEqual(rv, 1) @mock.patch('sys.stdout', new_callable=six.StringIO) @@ -119,12 +120,14 @@ class TestRemovePkg(utils.CliTestCase): activate_session_mock.assert_called_once_with(session, options) self.assertEqual( session.mock_calls, [ - call.getTag(tag), call.listPackages( - tagID=dsttag['id']), call.packageListRemove( - tag, packages[0], **kwargs), call.packageListRemove( - tag, packages[1], **kwargs), call.packageListRemove( - tag, packages[2], **kwargs), call.multiCall( - strict=True)]) + call.getTag(tag), + call.listPackages(tagID=dsttag['id'], with_owners=False), + call.packageListRemove(tag, packages[0], **kwargs), + call.packageListRemove(tag, packages[1], **kwargs), + call.packageListRemove(tag, packages[2], **kwargs), + call.multiCall(strict=True) + ] + ) self.assertNotEqual(rv, 1) @mock.patch('sys.stderr', new_callable=six.StringIO) @@ -156,8 +159,7 @@ class TestRemovePkg(utils.CliTestCase): # Finally, assert that things were called as we expected. activate_session_mock.assert_called_once_with(session, options) session.getTag.assert_called_once_with(tag) - session.listPackages.assert_called_once_with( - tagID=dsttag['id']) + session.listPackages.assert_called_once_with(tagID=dsttag['id'], with_owners=False) session.packageListRemove.assert_not_called() session.multiCall.assert_not_called() From cea7f95ab1c189828b7ef7dfe3688d1b0019d3a1 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 12:47:20 +0000 Subject: [PATCH 4/11] update API docs --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 97845ad..1024298 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1130,6 +1130,18 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal One of (tagID,userID,pkgID) must be specified Note that the returned data includes blocked entries + + :param int tagID: filter on tag + :param int userID: filter on package owner + :param int pkgID: filter on package + :param int event: filter on event + :param bool inherit: return also inherited packages + :param bool with_dups: possible duplicates from inheritance, makes no sense + with inherit=False + :param bool with_owners: return also owner info. It needs to be set to True + if userID is not None + + :returns [dict]: list of dicts with package info """ if tagID is None and userID is None and pkgID is None: raise koji.GenericError('tag,user, and/or pkg must be specified') From a7b61b2d952f5c50a1e523304d48be592b631fa2 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 12:47:20 +0000 Subject: [PATCH 5/11] readTaggedBuilds with_owners option --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 1024298..5d814c2 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1301,7 +1301,7 @@ def list_tags(build=None, package=None, perms=True, queryOpts=None, pattern=None def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, owner=None, - type=None): + type=None, with_owners=True): """Returns a list of builds for specified tag set inherit=True to follow inheritance @@ -1315,6 +1315,9 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, # build - id pkg_id version release epoch # tag_listing - id build_id tag_id + if owner and not with_owners: + raise koji.GenericError("owner and with_owners=False can't be used together") + if not isinstance(latest, NUMERIC_TYPES): latest = bool(latest) @@ -1327,6 +1330,8 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, packages = readPackageList(tagID=tag, event=event, inherit=True, pkgID=package, with_owners=False) + st_complete = koji.BUILD_STATES['COMPLETE'] + tables = ['tag_listing'] # these values are used for each iteration fields = [('tag.id', 'tag_id'), ('tag.name', 'tag_name'), ('build.id', 'id'), ('build.id', 'build_id'), ('build.version', 'version'), ('build.release', 'release'), @@ -1338,60 +1343,59 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, ('volume.id', 'volume_id'), ('volume.name', 'volume_name'), ('package.id', 'package_id'), ('package.name', 'package_name'), ('package.name', 'name'), - ("package.name || '-' || build.version || '-' || build.release", 'nvr'), - ('users.id', 'owner_id'), ('users.name', 'owner_name')] - st_complete = koji.BUILD_STATES['COMPLETE'] + ("package.name || '-' || build.version || '-' || build.release", 'nvr')] + joins = [ + 'tag ON tag.id = tag_listing.tag_id', + 'build ON build.id = tag_listing.build_id', + 'events ON events.id = build.create_event', + 'package ON package.id = build.pkg_id', + 'volume ON volume.id = build.volume_id', + ] + if with_owners: + fields += [('users.id', 'owner_id'), ('users.name', 'owner_name')] + joins.append('users ON users.id = build.owner') - type_join = '' if type is None: pass elif type == 'maven': - type_join = 'JOIN maven_builds ON maven_builds.build_id = tag_listing.build_id' + joins.append('maven_builds ON maven_builds.build_id = tag_listing.build_id') fields.extend([('maven_builds.group_id', 'maven_group_id'), ('maven_builds.artifact_id', 'maven_artifact_id'), ('maven_builds.version', 'maven_version')]) elif type == 'win': - type_join = 'JOIN win_builds ON win_builds.build_id = tag_listing.build_id' + joins.append('win_builds ON win_builds.build_id = tag_listing.build_id') fields.append(('win_builds.platform', 'platform')) elif type == 'image': - type_join = 'JOIN image_builds ON image_builds.build_id = tag_listing.build_id' + joins.append('image_builds ON image_builds.build_id = tag_listing.build_id') fields.append(('image_builds.build_id', 'build_id')) else: btype = lookup_name('btype', type, strict=False) if not btype: raise koji.GenericError('unsupported build type: %s' % type) btype_id = btype['id'] - type_join = ('JOIN build_types ON build.id = build_types.build_id ' - 'AND btype_id = %(btype_id)s') - - q = """SELECT %s - FROM tag_listing - JOIN tag ON tag.id = tag_listing.tag_id - JOIN build ON build.id = tag_listing.build_id - %s - JOIN users ON users.id = build.owner - JOIN events ON events.id = build.create_event - JOIN package ON package.id = build.pkg_id - JOIN volume ON volume.id = build.volume_id - WHERE %s AND tag_id=%%(tagid)s - AND build.state=%%(st_complete)i - """ % (', '.join([pair[0] for pair in fields]), type_join, - eventCondition(event, 'tag_listing')) + joins += ['build_types ON build.id = build_types.build_id', + 'btype_id = %(btype_id)s'] + + clauses = [ + eventCondition(event, 'tag_listing'), + 'tag_id = %(tagid)s', + 'build.state = %(st_complete)i' + ] if package: - q += """AND package.name = %(package)s - """ + clauses.append('package.name = %(package)s') if owner: - q += """AND users.name = %(owner)s - """ - q += """ORDER BY tag_listing.create_event DESC - """ - # i.e. latest first + clauses.append('users.name = %(owner)s') + queryOpts = {'order': '-tag_listing.create_event'} # latest first + query = QueryProcessor(columns=[x[0] for x in fields], aliases=[x[1] for x in fields], + tables=tables, joins=joins, clauses=clauses, values=locals(), + opts=queryOpts) builds = [] seen = {} # used to enforce the 'latest' option for tagid in taglist: # log_error(koji.db._quoteparams(q,locals())) - for build in _multiRow(q, locals(), [pair[1] for pair in fields]): + query.values['tagid'] = tagid + for build in query.execute(): pkgid = build['package_id'] pinfo = packages.get(pkgid, None) if pinfo is None or pinfo['blocked']: From 9cf605079550aacc63c153bdd5e456123be5935f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 6/11] x --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 5d814c2..e875ba6 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1343,7 +1343,8 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, ('volume.id', 'volume_id'), ('volume.name', 'volume_name'), ('package.id', 'package_id'), ('package.name', 'package_name'), ('package.name', 'name'), - ("package.name || '-' || build.version || '-' || build.release", 'nvr')] + ("package.name || '-' || build.version || '-' || build.release", 'nvr'), + ('tag_listing.create_event', 'create_event')] joins = [ 'tag ON tag.id = tag_listing.tag_id', 'build ON build.id = tag_listing.build_id', @@ -1385,7 +1386,7 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, clauses.append('package.name = %(package)s') if owner: clauses.append('users.name = %(owner)s') - queryOpts = {'order': '-tag_listing.create_event'} # latest first + queryOpts = {'order': '-create_event'} # latest first query = QueryProcessor(columns=[x[0] for x in fields], aliases=[x[1] for x in fields], tables=tables, joins=joins, clauses=clauses, values=locals(), opts=queryOpts) From 25d7d0db6dd5693bd998785f3039f1c51b4bc705 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 7/11] use listTagged with_owner=False --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 462dfd5..3e9a297 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -6736,14 +6736,14 @@ def handle_untag_build(goptions, session, args): if options.all: builds = [] for pkg in args[1:]: - builds.extend(session.listTagged(args[0], package=pkg)) + builds.extend(session.listTagged(args[0], package=pkg, with_owners=False)) elif options.non_latest: if options.force and len(args) == 1: - tagged = session.listTagged(args[0]) + tagged = session.listTagged(args[0], with_owners=False) else: tagged = [] for pkg in args[1:]: - tagged.extend(session.listTagged(args[0], package=pkg)) + tagged.extend(session.listTagged(args[0], package=pkg, with_owners=False)) # listTagged orders entries latest first seen_pkg = {} builds = [] @@ -6761,7 +6761,7 @@ def handle_untag_build(goptions, session, args): tagged = [] with session.multicall() as m: for pkg in pkgs: - tagged.append(m.listTagged(args[0], package=pkg)) + tagged.append(m.listTagged(args[0], package=pkg, with_owners=False)) # flatten tagged = list(itertools.chain(*[t.result for t in tagged])) idx = dict([(b['nvr'], b) for b in tagged]) diff --git a/hub/kojihub.py b/hub/kojihub.py index e875ba6..c8284de 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -11282,12 +11282,12 @@ class RootExports(object): task.setPriority(priority, recurse=recurse) def listTagged(self, tag, event=None, inherit=False, prefix=None, latest=False, package=None, - owner=None, type=None): + owner=None, type=None, with_owners=True): """List builds tagged with tag""" # lookup tag id tag = get_tag(tag, strict=True, event=event)['id'] results = readTaggedBuilds(tag, event, inherit=inherit, latest=latest, package=package, - owner=owner, type=type) + owner=owner, type=type, with_owners=with_owners) if prefix: prefix = prefix.lower() results = [build for build in results diff --git a/koji/util.py b/koji/util.py index 304e112..a3aef3b 100644 --- a/koji/util.py +++ b/koji/util.py @@ -118,7 +118,8 @@ def checkForBuilds(session, tag, builds, event, latest=False): if latest: tagged_list = session.getLatestBuilds(tag, event=event, package=build['name']) else: - tagged_list = session.listTagged(tag, event=event, package=build['name'], inherit=True) + tagged_list = session.listTagged(tag, event=event, package=build['name'], + inherit=True, with_owners=False) for tagged in tagged_list: if tagged['version'] == build['version'] and tagged['release'] == build['release']: break diff --git a/util/koji-gc b/util/koji-gc index fe9be82..f788caf 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -626,7 +626,7 @@ def handle_delete(just_salvage=False): """ print("Getting list of builds in trash...") trashcan_tag = options.trashcan_tag - trash = sorted([(b['nvr'], b) for b in session.listTagged(trashcan_tag)]) + trash = sorted([(b['nvr'], b) for b in session.listTagged(trashcan_tag, with_owners=False)]) print("...got %i builds" % len(trash)) # XXX - it would be better if there were more appropriate server calls for this grace_period = options.grace_period diff --git a/util/koji-sidetag-cleanup b/util/koji-sidetag-cleanup index bf1cbc4..2cd18d0 100644 --- a/util/koji-sidetag-cleanup +++ b/util/koji-sidetag-cleanup @@ -188,7 +188,7 @@ def clean_empty(tags): deleted = [] session.multicall = True for tag in tags: - session.listTagged(tag['id']) + session.listTagged(tag['id'], with_owners=False) for tag, tagged in zip(tags, session.multiCall()): if len(tagged[0]) == 0: candidates.append(tag) From 3f078d76add0ec643bc4275f045f2c8fffc762e4 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 8/11] updated API docs --- diff --git a/hub/kojihub.py b/hub/kojihub.py index c8284de..66edb9b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1304,13 +1304,17 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, type=None, with_owners=True): """Returns a list of builds for specified tag - set inherit=True to follow inheritance - set event to query at a time in the past - set latest=True to get only the latest build per package - set latest=N to get only the N latest tagged RPMs - - If type is not None, restrict the list to builds of the given type. Currently the supported - types are 'maven', 'win', and 'image'. + :param int tag: tag ID + :param int event: query at a time in the past + :param bool inherit: follow inheritance + :param bool|int latest: True for latest build per package, N to get N latest builds per package + :param int package: filter on package name + :param str owner: filter on user name + :param str type: restrict the list to builds of the given type. Currently the supported + types are 'maven', 'win', and 'image'. + :param bool with_owners: set to False for faster query (owner_id and owner_name will be + missing in the result) + :returns [dict]: list of buildinfo dicts """ # build - id pkg_id version release epoch # tag_listing - id build_id tag_id From 72b77beaf3afc6513690889727d528d740747f08 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 9/11] remove with_owners from readTaggedBuilds --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 66edb9b..d359398 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1146,13 +1146,18 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal if tagID is None and userID is None and pkgID is None: raise koji.GenericError('tag,user, and/or pkg must be specified') - if userID and not with_owners: + if userID is not None and not with_owners: raise koji.GenericError("userID and with_owners=False can't be used together") tables = ['tag_packages'] - fields = ['package.id', 'package.name', 'tag.id', 'tag.name', 'extra_arches', - 'tag_packages.blocked'] - aliases = ['package_id', 'package_name', 'tag_id', 'tag_name', 'extra_arches', 'blocked'] + fields = [ + ('package.id', 'package_id'), + ('package.name', 'package_name'), + ('tag.id', 'tag_id'), + ('tag.name', 'tag_name'), + ('extra_arches', 'extra_arches'), + ('tag_packages.blocked', 'blocked'), + ] joins = ['tag ON tag.id = tag_packages.tag_id', 'package ON package.id = tag_packages.package_id'] clauses = [eventCondition(event, table='tag_packages')] @@ -1166,14 +1171,14 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal else: clauses.append('package.name = %(pkgID)s') if with_owners: - fields += ['users.id', 'users.name'] - aliases += ['owner_id', 'owner_name'] + fields += [('users.id', 'owner_id'), ('users.name', 'owner_name')] joins += [ 'tag_package_owners ON tag_packages.tag_id = tag_package_owners.tag_id AND \ tag_packages.package_id = tag_package_owners.package_id', 'users ON users.id = tag_package_owners.owner' ] clauses.append(eventCondition(event, table='tag_package_owners')) + fields, aliases = zip(*fields) query = QueryProcessor(columns=fields, aliases=aliases, tables=tables, joins=joins, clauses=clauses, values=locals()) @@ -1301,7 +1306,7 @@ def list_tags(build=None, package=None, perms=True, queryOpts=None, pattern=None def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, owner=None, - type=None, with_owners=True): + type=None): """Returns a list of builds for specified tag :param int tag: tag ID @@ -1312,16 +1317,11 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, :param str owner: filter on user name :param str type: restrict the list to builds of the given type. Currently the supported types are 'maven', 'win', and 'image'. - :param bool with_owners: set to False for faster query (owner_id and owner_name will be - missing in the result) :returns [dict]: list of buildinfo dicts """ # build - id pkg_id version release epoch # tag_listing - id build_id tag_id - if owner and not with_owners: - raise koji.GenericError("owner and with_owners=False can't be used together") - if not isinstance(latest, NUMERIC_TYPES): latest = bool(latest) @@ -1343,6 +1343,7 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, ('build.completion_time', 'completion_time'), ('build.start_time', 'start_time'), ('build.task_id', 'task_id'), + ('users.id', 'owner_id'), ('users.name', 'owner_name'), ('events.id', 'creation_event_id'), ('events.time', 'creation_time'), ('volume.id', 'volume_id'), ('volume.name', 'volume_name'), ('package.id', 'package_id'), ('package.name', 'package_name'), @@ -1355,10 +1356,8 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None, 'events ON events.id = build.create_event', 'package ON package.id = build.pkg_id', 'volume ON volume.id = build.volume_id', + 'users ON users.id = build.owner', ] - if with_owners: - fields += [('users.id', 'owner_id'), ('users.name', 'owner_name')] - joins.append('users ON users.id = build.owner') if type is None: pass From 399dcd3d3fc5abb1ac4579dc36c3ce59989d25a7 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 10/11] cleanup removed options --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 3e9a297..462dfd5 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -6736,14 +6736,14 @@ def handle_untag_build(goptions, session, args): if options.all: builds = [] for pkg in args[1:]: - builds.extend(session.listTagged(args[0], package=pkg, with_owners=False)) + builds.extend(session.listTagged(args[0], package=pkg)) elif options.non_latest: if options.force and len(args) == 1: - tagged = session.listTagged(args[0], with_owners=False) + tagged = session.listTagged(args[0]) else: tagged = [] for pkg in args[1:]: - tagged.extend(session.listTagged(args[0], package=pkg, with_owners=False)) + tagged.extend(session.listTagged(args[0], package=pkg)) # listTagged orders entries latest first seen_pkg = {} builds = [] @@ -6761,7 +6761,7 @@ def handle_untag_build(goptions, session, args): tagged = [] with session.multicall() as m: for pkg in pkgs: - tagged.append(m.listTagged(args[0], package=pkg, with_owners=False)) + tagged.append(m.listTagged(args[0], package=pkg)) # flatten tagged = list(itertools.chain(*[t.result for t in tagged])) idx = dict([(b['nvr'], b) for b in tagged]) diff --git a/hub/kojihub.py b/hub/kojihub.py index d359398..176b72f 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2579,8 +2579,7 @@ def repo_init(tag, with_src=False, with_debuginfo=False, event=None, with_separa latest = not tinfo['extra'].get('repo_include_all', False) # Note: the repo_include_all option is not recommended for common use # see https://pagure.io/koji/issue/588 for background - rpms, builds = readTaggedRPMS(tag_id, event=event_id, inherit=True, latest=latest, - with_owners=False) + rpms, builds = readTaggedRPMS(tag_id, event=event_id, inherit=True, latest=latest) groups = readTagGroups(tag_id, event=event_id, inherit=True) blocks = [pkg for pkg in readPackageList(tag_id, event=event_id, inherit=True, with_owners=False).values() @@ -11285,12 +11284,12 @@ class RootExports(object): task.setPriority(priority, recurse=recurse) def listTagged(self, tag, event=None, inherit=False, prefix=None, latest=False, package=None, - owner=None, type=None, with_owners=True): + owner=None, type=None): """List builds tagged with tag""" # lookup tag id tag = get_tag(tag, strict=True, event=event)['id'] results = readTaggedBuilds(tag, event, inherit=inherit, latest=latest, package=package, - owner=owner, type=type, with_owners=with_owners) + owner=owner, type=type) if prefix: prefix = prefix.lower() results = [build for build in results diff --git a/koji/util.py b/koji/util.py index a3aef3b..304e112 100644 --- a/koji/util.py +++ b/koji/util.py @@ -118,8 +118,7 @@ def checkForBuilds(session, tag, builds, event, latest=False): if latest: tagged_list = session.getLatestBuilds(tag, event=event, package=build['name']) else: - tagged_list = session.listTagged(tag, event=event, package=build['name'], - inherit=True, with_owners=False) + tagged_list = session.listTagged(tag, event=event, package=build['name'], inherit=True) for tagged in tagged_list: if tagged['version'] == build['version'] and tagged['release'] == build['release']: break diff --git a/util/koji-gc b/util/koji-gc index f788caf..fe9be82 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -626,7 +626,7 @@ def handle_delete(just_salvage=False): """ print("Getting list of builds in trash...") trashcan_tag = options.trashcan_tag - trash = sorted([(b['nvr'], b) for b in session.listTagged(trashcan_tag, with_owners=False)]) + trash = sorted([(b['nvr'], b) for b in session.listTagged(trashcan_tag)]) print("...got %i builds" % len(trash)) # XXX - it would be better if there were more appropriate server calls for this grace_period = options.grace_period diff --git a/util/koji-sidetag-cleanup b/util/koji-sidetag-cleanup index 2cd18d0..bf1cbc4 100644 --- a/util/koji-sidetag-cleanup +++ b/util/koji-sidetag-cleanup @@ -188,7 +188,7 @@ def clean_empty(tags): deleted = [] session.multicall = True for tag in tags: - session.listTagged(tag['id'], with_owners=False) + session.listTagged(tag['id']) for tag, tagged in zip(tags, session.multiCall()): if len(tagged[0]) == 0: candidates.append(tag) From 65dc12a5df16938701c04eeb6ec8627d2f8dce23 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 14 2021 14:42:42 +0000 Subject: [PATCH 11/11] backward compatibility for CLI --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 462dfd5..a8bdde2 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -352,8 +352,12 @@ def handle_add_pkg(goptions, session, args): if dsttag is None: print("No such tag: %s" % tag) sys.exit(1) - pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'], with_owners=False)]) + try: + pkglist = session.listPackages(tagID=dsttag['id'], with_owners=False) + except koji.ParameterError: + # performance option added in 1.25 + pkglist = session.listPackages(tagID=dsttag['id']) + pkglist = dict([(p['package_name'], p['package_id']) for p in pkglist]) to_add = [] for package in args[1:]: package_id = pkglist.get(package, None) @@ -387,9 +391,12 @@ def handle_block_pkg(goptions, session, args): dsttag = session.getTag(tag) if dsttag is None: error("No such tag: %s" % tag) - pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'], inherited=True, - with_owners=False)]) + try: + pkglist = session.listPackages(tagID=dsttag['id'], inherited=True, with_owners=False) + except koji.ParameterError: + # performance option added in 1.25 + pkglist = session.listPackages(tagID=dsttag['id'], inherited=True) + pkglist = dict([(p['package_name'], p['package_id']) for p in pkglist]) ret = 0 for package in args[1:]: package_id = pkglist.get(package, None) @@ -425,8 +432,12 @@ def handle_remove_pkg(goptions, session, args): dsttag = session.getTag(tag) if dsttag is None: error("No such tag: %s" % tag) - pkglist = dict([(p['package_name'], p['package_id']) - for p in session.listPackages(tagID=dsttag['id'], with_owners=False)]) + try: + pkglist = session.listPackages(tagID=dsttag['id'], with_owners=False) + except koji.ParameterError: + # performance option added in 1.25 + pkglist = session.listPackages(tagID=dsttag['id']) + pkglist = dict([(p['package_name'], p['package_id']) for p in pkglist]) ret = 0 for package in args[1:]: package_id = pkglist.get(package, None)