PR#4304: Enable specific number of builds for dist-repo tasks - koji

koji

#4304 Enable specific number of builds for dist-repo tasks

Opened 2 months ago by jcupova. Modified 6 hours ago

jcupova/koji issue-3943 into master

Enable specific number of builds for dist-repo tasks

Jana Librova • 11 days ago

b12fe00

cli/koji_cli/commands.py

file modified

+8 -1

		`@@ -7674,6 +7674,8 @@`
		`parser.add_option("--volume", help="Generate repo on given volume")`
		`parser.add_option('--non-latest', dest='latest', default=True,`
		`action='store_false', help='Include older builds, not just the latest')`
		`+ parser.add_option("--latest-n", type='int', metavar="N",`
		`+ help="Only include the latest N builds")`
		`parser.add_option('--multilib', default=None, metavar="CONFIG",`
		`help='Include multilib packages in the repository using the given '`
		`'config file')`
		`@@ -7704,6 +7706,8 @@`
		`parser.error('Please specify one or more GPG key IDs (or --allow-missing-signatures)')`
		`if task_opts.allow_missing_signatures and task_opts.skip_missing_signatures:`
		`parser.error('allow_missing_signatures and skip_missing_signatures are mutually exclusive')`
		`+ if not task_opts.latest and task_opts.latest_n:`
		`+ parser.error("Only --non-latest or --latest-n=N may be specified. Not both.")`
		`activate_session(session, options)`
		`stuffdir = unique_path('cli-dist-repo')`
		`if task_opts.comps:`
		`@@ -7778,7 +7782,6 @@`
		`'event': task_opts.event,`
		`'volume': task_opts.volume,`
		`'inherit': not task_opts.noinherit,`
		`- 'latest': task_opts.latest,`
		`'multilib': task_opts.multilib,`
		`'split_debuginfo': task_opts.split_debuginfo,`
		`'skip_missing_signatures': task_opts.skip_missing_signatures,`
		`@@ -7787,6 +7790,10 @@`
		`'zck_dict_dir': task_opts.zck_dict_dir,`
		`'write_signed_rpms': task_opts.write_signed_rpms,`
		`}`
		`+ if task_opts.latest_n:`
		`+ opts['latest'] = task_opts.latest_n`
		`+ else:`
		`+ opts['latest'] = task_opts.latest`
		`if task_opts.skip_stat is not None:`
		`opts['createrepo_skip_stat'] = task_opts.skip_stat`
		`task_id = session.distRepo(tag, keys, **opts)`

tests/test_cli/test_dist_repo.py

file modified

+36

		`@@ -125,6 +125,15 @@`
		`quiet=self.options.quiet,`
		`poll_interval=self.options.poll_interval, topurl=self.options.topurl)`

		`+ def test_handle_dist_repo_with_latest_n(self):`
		`+ arguments = [self.tag_name, self.fake_key, '--latest-n=2']`
		`+ self.__run_test_handle_dist_repo(arguments, return_value=True)`
		`+ self.watch_tasks.assert_called_with(`
		`+ self.session,`
		`+ [self.task_id],`
		`+ quiet=self.options.quiet,`
		`+ poll_interval=self.options.poll_interval, topurl=self.options.topurl)`
		`+`
		`def test_handle_dist_repo_nowait(self):`
		`arguments = [self.tag_name, self.fake_key, '--nowait']`
		`self.__run_test_handle_dist_repo(arguments, return_value=None)`
		`@@ -151,6 +160,14 @@`
		`],`
		`'err_str': 'allow_missing_signatures and skip_missing_signatures are mutually '`
		`'exclusive'`
		`+ },`
		`+ {`
		`+ 'arg': [`
		`+ self.tag_name, self.fake_key,`
		`+ '--non-latest',`
		`+ '--latest-n=2'`
		`+ ],`
		`+ 'err_str': 'Only --non-latest or --latest-n=N may be specified. Not both.'`
		`}`
		`]`

		`@@ -205,6 +222,24 @@`
		`self.session.getTag.assert_has_calls(expected_calls)`
		`self.session.getFullInheritance.assert_called_with(tag_2['name'])`

		`+ # Case 5. Arches list has no arches`
		`+ tag_1 = copy.copy(self.TAG)`
		`+ tag_1.update({'arches': 'noarch'})`
		`+ tag_2 = copy.copy(self.TAG)`
		`+ tag_2.update({'arches': 'noarch'})`
		`+ self.session.getTag.side_effect = [tag_2, tag_1]`
		`+ self.session.getFullInheritance.return_value = self.INHERITANCE`
		`+ expected = self.format_error_message('No arches left.')`
		`+ self.assert_system_exit(`
		`+ handle_dist_repo,`
		`+ self.options,`
		`+ self.session,`
		`+ [tag_2['name'], self.fake_key],`
		`+ stderr=expected)`
		`+ expected_calls = [mock.call(tag_2['name']), mock.call(tag_1['id'])]`
		`+ self.session.getTag.assert_has_calls(expected_calls)`
		`+ self.session.getFullInheritance.assert_called_with(tag_2['name'])`
		`+`
		`def test_handle_dist_repo_with_comp(self):`
		`comp_file = 'comp.xml'`
		`arguments = [self.tag_name, self.fake_key, '--comp', comp_file]`
		`@@ -343,6 +378,7 @@`
		`--event=EVENT Use tag content at event`
		`--volume=VOLUME Generate repo on given volume`
		`--non-latest Include older builds, not just the latest`
		`+ --latest-n=N Only include the latest N builds`
		`--multilib=CONFIG Include multilib packages in the repository using the`
		`given config file`
		`--noinherit Do not consider tag inheritance`

jcupova commented 2 months ago

Fixes: https://pagure.io/koji/issue/3943

tkopecek commented 2 months ago

@mikem any better idea for naming? I'm not happy with this, but not sure how to improve it.

mikem commented 2 months ago

It is potentially misleading to have --non-latest as a boolean option and --latest as an int. Also, it's worth noting we have separate --latest-n and --latest for the list-tagged command. Granted a boolean --latest for this command is the default behavior, but I guess it would cancel out a --non-latest.

I don't love the "latest_n" name, but we do have prior art. Might be best to mimic the list-tagged args.

rebased onto 7904e4e

2 months ago

jcupova commented 2 months ago

Updated.

mikem commented a month ago

You seem to have changed the default behavior to include all builds. This is not what we want.

mikem commented a month ago

Previously we had --non-latest which is unfortunately named. It was a boolean option that caused us to include all builds in the inheritance. This was not the default behavior and should not be. By default, we should only include the latest builds.

We don't really need a --latest option here, since this is the default behavior. I suggested it as a no-op for consistency with list-tagged, but in retrospect it's probably more confusion than its worth. The default latest behavior for list-tagged is different.

The simplest version of this fix is to leave --non-latest alone and simply add --latest-n to allow users to specify the requested behavior.

2 new commits added