PR#80: Added tagging in pagure to tag_package cli command. - fedora-infra/rpmautospec

fedora-infra / rpmautospec

#80 Added tagging in pagure to tag_package cli command.

Merged 4 years ago by pingou. Opened 4 years ago by asaleh.

fedora-infra/ asaleh/rpmautospec tag_package_with_pagure into master

Added tagging in pagure to tag_package.

Adam Saleh • 4 years ago

e088850

koji_plugins/rpmautospec_builder.py

file modified

+23 -1

		`@@ -2,14 +2,21 @@`
		`import logging`
		`import shlex`

		`+ import koji`
		`from koji.plugin import callback`

		`from rpmautospec import process_distgit, tag_package`
		`+ from rpmautospec.py2compat.tagging import PagureTaggingProxy`


		`+ CONFIG_FILE = "/etc/kojid/plugins/rpmautospec.conf"`
		`+ CONFIG = None`
		`+`
		`_log = logging.getLogger(__name__)`
		`_log.setLevel(logging.DEBUG)`

		`+ pagure_proxy = None`
		`+`

		`def _steal_buildroot_object_from_frame_stack():`
		`buildroot = frame_info = frame = stack = None`
		`@@ -47,8 +54,23 @@`
		`_log.info("No %autorel/%autochangelog found, skipping.")`
		`return`

		`+ global CONFIG, pagure_proxy`
		`+`
		`+ if not CONFIG:`
		`+ try:`
		`+ CONFIG = koji.read_config_files([(CONFIG_FILE, True)])`
		`+ except Exception:`
		`+ message = "While attempting to read config file %s, an exception occurred:"`
		`+ _log.exception(message, CONFIG_FILE)`
		`+ return`
		`+`
		`+ if not pagure_proxy:`
		`+ base_url = CONFIG.get("pagure", "url")`
		`+ token = CONFIG.get("pagure", "token")`
		`+ pagure_proxy = PagureTaggingProxy(base_url=base_url, auth_token=token, logger=_log)`
		`+`
		`_log.info("Tagging existing builds...")`
		`- tag_package.tag_package(srcdir, session)`
		`+ tag_package.tag_package(srcdir, session, pagure_proxy)`

		`buildroot = kwargs.get("buildroot")`
		`if not buildroot:`

rpmautospec/tag_package.py

file modified

+21 -3

		`@@ -4,7 +4,7 @@`
		`import os`

		`from .misc import get_package_builds, koji_init, run_command`
		`- from .py2compat.tagging import escape_tag, tag_prefix`
		`+ from .py2compat.tagging import escape_tag, PagureTaggingProxy, tag_prefix`


		`_log = logging.getLogger(__name__)`
		`@@ -18,11 +18,20 @@`
		`)`

		`tag_project_parser.add_argument("worktree_path", help="Path to the dist-git worktree")`
		`+ tag_project_parser.add_argument(`
		`+ "--pagure-url",`
		`+ help="Pagure url for tagging the package in the repo. Requires pagure_token to work.",`
		`+ default="https://src.fedoraproject.org",`
		`+ )`

		`+ tag_project_parser.add_argument(`
		`+ "--pagure-token",`
		`+ help="Pagure token for tagging the package in the repo. Requires pagure_url to work.",`
		`+ )`
		`return subcmd_name`


		`- def tag_package(srcdir, session):`
		`+ def tag_package(srcdir, session, pagure_proxy=None):`
		`koji_init(session)`
		`repopath = srcdir.rstrip(os.path.sep)`

		`@@ -90,10 +99,19 @@`
		`_log.exception("Error while tagging %s with %s:", commit, tag)`
		`else:`
		`_log.info("Tagged commit %s as %s", commit, tag)`
		`+ if pagure_proxy:`
		`+ _log.info("Uploading tag to pagure")`
		`+ pagure_proxy.create_tag(`
		`+ repository="rpms/" + name, tagname=tag, commit_hash=commit`
		`+ )`


		`def main(args):`
		`""" Main method. """`
		`session = koji_init(args.koji_url)`

		`- tag_package(args.worktree_path, session)`
		`+ pagure_proxy = None`
		`+ if args.pagure_url and args.pagure_token:`
		`+ pagure_proxy = PagureTaggingProxy(base_url=args.pagure_url, auth_token=args.pagure_token)`
		`+`
		`+ tag_package(args.worktree_path, session, pagure_proxy)`

tests/koji_plugins/test_rpmautospec_builder.py

file modified

+21 -2

		`@@ -6,6 +6,15 @@`
		`from koji_plugins.rpmautospec_builder import process_distgit_cb`


		`+ class MockConfig:`
		`+ test_config = {`
		`+ "pagure": {"url": "src.fedoraproject.org", "token": "aaabbbcc"},`
		`+ }`
		`+`
		`+ def get(self, section, option, **kwargs):`
		`+ return self.test_config[section][option]`
		`+`
		`+`
		`class TestRpmautospecBuilder:`
		`"""Test the rpmautospec builder plugin for Koji."""`

		`@@ -48,10 +57,20 @@`
		`@mock.patch("rpmautospec.process_distgit.needs_processing")`
		`@mock.patch("rpmautospec.tag_package.tag_package")`
		`@mock.patch("koji_plugins.rpmautospec_builder._steal_buildroot_object_from_frame_stack")`
		`+ @mock.patch("koji_plugins.rpmautospec_builder.pagure_proxy")`
		`+ @mock.patch("koji.read_config_files")`
		`def test_process_distgit_cb(`
		`- self, steal_buildroot_fn, tag_package_fn, needs_processing_fn, testcase`
		`+ self,`
		`+ read_config_files,`
		`+ pagure_proxy,`
		`+ steal_buildroot_fn,`
		`+ tag_package_fn,`
		`+ needs_processing_fn,`
		`+ testcase,`
		`):`
		`"""Test the process_distgit_cb() function"""`
		`+ read_config_files.return_value = MockConfig()`
		`+`
		`buildroot_supplied = testcase != "no buildroot supplied"`
		`rpmautospec_installed = testcase == "rpmautospec installed"`
		`taskinfo_method_responsible = testcase != "other taskinfo method"`
		`@@ -111,7 +130,7 @@`
		`tag_package_fn.assert_not_called()`
		`return`

		`- tag_package_fn.assert_called_once_with(unpacked_repo_dir, koji_session)`
		`+ tag_package_fn.assert_called_once_with(unpacked_repo_dir, koji_session, pagure_proxy)`

		`buildroot.getPackageList.assert_called_once()`
		`buildroot.path_without_to_within.assert_called_once()`

tests/rpmautospec/test_tag_package.py

file modified

+24 -1

		`@@ -39,6 +39,7 @@`
		`"stagingbuildsys",`
		`"wrongbuildsys",`
		`"tagcmdfails",`
		`+ "pagure_tag",`
		`):`
		`# Ignore phenomena handled above and/or in the test method.`
		`pass`
		`@@ -84,17 +85,20 @@`
		`"nosource,stagingbuildsys",`
		`"nosource,wrongbuildsys",`
		`"tagcmdfails",`
		`+ "pagure_tag",`
		`),`
		`)`
		`@mock.patch("rpmautospec.tag_package.run_command")`
		`@mock.patch("rpmautospec.tag_package.koji_init")`
		`@mock.patch("rpmautospec.tag_package.get_package_builds")`
		`- def test_main(self, get_package_builds, koji_init, run_command, phenomena, caplog):`
		`+ @mock.patch("rpmautospec.py2compat.tagging.requests.post")`
		`+ def test_main(self, pagure_post, get_package_builds, koji_init, run_command, phenomena, caplog):`
		`"""Test the tag_package.main() under various conditions."""`
		`caplog.set_level(logging.DEBUG)`

		`phenomena = [p.strip() for p in phenomena.split(",")]`
		`koji_init.return_value = kojiclient = mock.Mock()`
		`+`
		`get_package_builds.return_value = test_builds = get_test_builds(phenomena)`

		`main_args = mock.Mock()`
		`@@ -103,6 +107,12 @@`
		`# attempts to contact a remote Koji instance.`
		`main_args.koji_url = "https://192.0.2.1"`
		`main_args.worktree_path = repopath = "/path/to/my/package/repo/pkgname"`
		`+ pagure_post.return_value.ok = True`
		`+`
		`+ if "pagure_tag" in phenomena:`
		`+ main_args.pagure_url = "https://192.0.2.2"`
		`+ main_args.pagure_token = "token"`
		`+`
		`if "trailingslashpath" in phenomena:`
		`# This shouldn't change anything, really.`
		`main_args.worktree_path += "/"`
		`@@ -138,6 +148,19 @@`

		`tag_package.main(main_args)`

		`+ if "pagure_tag" in phenomena:`
		`+ pagure_post.assert_called_once_with(`
		`+ "https://192.0.2.2/api/0/rpms/pkgname/git/tags",`
		`+ data={`
		`+ "tagname": "build/pkgname-0-1.0-1.fc32",`
		`+ "commit_hash": "0123456789abcdef0123456789abcdef01234567",`
		`+ "message": None,`
		`+ "with_commits": True,`
		`+ "force": False,`
		`+ },`
		`+ headers={"Authorization": "token token"},`
		`+ )`
		`+`
		`if "nosource" in phenomena:`
		`kojiclient.getTaskChildren.assert_called_once_with(test_builds[0]["task_id"])`

no initial comment

zuul commented 4 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

rebased onto a10fe103ad102bf9b98c4ffe1ad72ccc1a6092c7

4 years ago

pingou commented on line 17 of rpmautospec/tag_package.py 4 years ago

Should we have a default?

zuul commented 4 years ago

Build failed.

pre-commit : SUCCESS in 1m 12s
pytest : FAILURE in 1m 07s
rpm-mock-build : FAILURE in 6m 28s

3 new commits added

Added tagging in pagure to builder plugin.
Remove defaults for pagure (would make tagging in pagure noop)
Fix test after merge.

4 years ago

zuul commented 4 years ago

Build failed.

pre-commit : SUCCESS in 1m 06s
pytest : FAILURE in 56s
rpm-mock-build : FAILURE in 5m 00s

4 new commits added

Added tagging in pagure to builder plugin.
Remove defaults for pagure (would make tagging in pagure noop)
Fix test after merge.
Added tagging in pagure to tag_package cli command.

4 years ago

zuul commented 4 years ago

Build succeeded.

pre-commit : SUCCESS in 1m 28s
pytest : SUCCESS in 2m 00s
rpm-mock-build : SUCCESS in 4m 13s

pingou commented on line 15 of rpmautospec/tag_package.py 4 years ago

Let's make this into --pagure-url so the argument remain optional

nphilipp commented on line 17 of rpmautospec/tag_package.py 4 years ago

You can set default="https://src.fedoraproject.org", here.

Edited 4 years ago by nphilipp

nphilipp commented on line 22 of rpmautospec/tag_package.py 4 years ago

As a positional parameter it probably doesn't matter too much (as far as I know, it only has an impact on the rendered help text), but let's make this pagure-token for consistency—the dash will be converted into an underscore just like with --pagure-url above.

rebased onto 4b0882c90ca3809025c421e75778644b53ef3762

4 years ago

zuul commented 4 years ago

Build succeeded.