From 257185adc794380b5f1be015c7f6cff91a0e95fb Mon Sep 17 00:00:00 2001 From: Ondrej Nosek Date: May 26 2020 07:50:26 +0000 Subject: Print response data from Pagure for debugging When fedpkg communicates with a web API (Pagure or DistGit), prints (json) response data for debugging. The response is shown when there is verbose mode enabled. Fixes: #396 JIRA: RHELCMP-486 Signed-off-by: Ondrej Nosek --- diff --git a/fedpkg/cli.py b/fedpkg/cli.py index 5a5ca3e..067763f 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -782,6 +782,7 @@ class fedpkgClient(cliClient): def request_repo(self): self._request_repo( + logger=self.log, repo_name=self.args.name, ns=self.args.new_repo_namespace, branch='master', @@ -798,23 +799,26 @@ class fedpkgClient(cliClient): def request_tests_repo(self): self._request_repo( + logger=self.log, repo_name=self.args.name, ns='tests', description=self.args.description, bug=self.args.bug, name=self.name, config=self.config, - anongiturl=self.cmd.anongiturl + anongiturl=self.cmd.anongiturl, ) @staticmethod - def _request_repo(repo_name, ns, description, name, config, branch=None, - summary=None, upstreamurl=None, monitor=None, bug=None, - exception=None, anongiturl=None, initial_commit=True): + def _request_repo(logger, repo_name, ns, description, name, config, + branch=None, summary=None, upstreamurl=None, + monitor=None, bug=None, exception=None, anongiturl=None, + initial_commit=True): """ Implementation of `request_repo`. Submits a request for a new dist-git repo. + :param logger: A logger object. :param repo_name: The repository name string. Typically the value of `self.cmd.repo_name`. :param ns: The repository namespace string, i.e. 'rpms' or 'modules'. @@ -906,7 +910,7 @@ class fedpkgClient(cliClient): pagure_url = config_get_safely(config, pagure_section, 'url') pagure_token = config_get_safely(config, pagure_section, 'token') print(new_pagure_issue( - pagure_url, pagure_token, ticket_title, ticket_body, name)) + logger, pagure_url, pagure_token, ticket_title, ticket_body, name)) def request_branch(self): if self.args.repo_name_for_branch: @@ -918,6 +922,7 @@ class fedpkgClient(cliClient): except rpkgError: active_branch = None self._request_branch( + logger=self.log, service_levels=self.args.sl, all_releases=self.args.all_releases, branch=self.args.branch, @@ -931,13 +936,14 @@ class fedpkgClient(cliClient): ) @staticmethod - def _request_branch(service_levels, all_releases, branch, active_branch, - repo_name, ns, no_git_branch, no_auto_module, - name, config): + def _request_branch(logger, service_levels, all_releases, branch, + active_branch, repo_name, ns, no_git_branch, + no_auto_module, name, config): """ Implementation of `request_branch`. Submits a request for a new branch of a given dist-git repo. + :param logger: A logger object. :param service_levels: A list of service level strings. Typically the value of `self.args.service_levels`. :param all_releases: A boolean indicating if this request should be made @@ -1055,7 +1061,8 @@ class fedpkgClient(cliClient): b, ns, repo_name) print(new_pagure_issue( - pagure_url, pagure_token, ticket_title, ticket_body, name)) + logger, pagure_url, pagure_token, ticket_title, ticket_body, + name)) # For non-standard rpm branch requests, also request a matching new # module repo with a matching branch. @@ -1069,6 +1076,7 @@ class fedpkgClient(cliClient): summary = ('Automatically requested module for ' 'rpms/%s:%s.' % (repo_name, b)) fedpkgClient._request_repo( + logger=logger, repo_name=repo_name, ns='modules', branch='master', @@ -1082,6 +1090,7 @@ class fedpkgClient(cliClient): config=config, ) fedpkgClient._request_branch( + logger=logger, service_levels=service_levels, all_releases=all_releases, branch=b, @@ -1109,6 +1118,7 @@ class fedpkgClient(cliClient): distgit_token = config_get_safely(self.config, distgit_section, 'token') ret = do_fork( + logger=self.log, base_url=distgit_api_base_url, token=distgit_token, repo_name=self.cmd.repo_name, diff --git a/fedpkg/utils.py b/fedpkg/utils.py index 561c563..2c790c7 100644 --- a/fedpkg/utils.py +++ b/fedpkg/utils.py @@ -84,9 +84,10 @@ def get_sl_type(url, sl_name): return None -def new_pagure_issue(url, token, title, body, cli_name): +def new_pagure_issue(logger, url, token, title, body, cli_name): """ Posts a new Pagure issue + :param logger: A logger object :param url: a string of the URL to Pagure :param token: a string of the Pagure API token that has rights to create a ticket @@ -117,13 +118,18 @@ def new_pagure_issue(url, token, title, body, cli_name): base_error_msg = ('The following error occurred while creating a new ' 'issue in Pagure: {0}') + + try: + # Extract response error text (and debug data) + rv_json = rv.json() + logger.debug("Pagure API response: '{0}'".format(rv_json)) + rv_error = rv_json.get('error') + except (ValueError, AttributeError): + rv_error = rv.text + if not rv.ok: # Lets see if the API returned an error message in JSON that we can # show the user - try: - rv_error = rv.json().get('error') - except ValueError: - rv_error = rv.text # show hint for expired token if re.search(r"Invalid or expired token", rv_error, re.IGNORECASE): base_error_msg += '\nFor invalid or expired token refer to ' \ @@ -135,9 +141,10 @@ def new_pagure_issue(url, token, title, body, cli_name): url.rstrip('/'), rv.json()['issue']['id']) -def do_fork(base_url, token, repo_name, namespace, cli_name): +def do_fork(logger, base_url, token, repo_name, namespace, cli_name): """ Creates a fork of the project. + :param logger: A logger object :param base_url: a string of the URL repository :param token: a string of the API token that has rights to make a fork :param repo_name: a string of the repository name @@ -166,15 +173,18 @@ def do_fork(base_url, token, repo_name, namespace, cli_name): 'create a new fork. The error was: {0}'.format(str(error))) raise rpkgError(error_msg) + try: + # Extract response error text (and debug data) + rv_json = rv.json() + logger.debug("Pagure API response: '{0}'".format(rv_json)) + rv_error = rv_json.get('error') + except (ValueError, AttributeError): + rv_error = rv.text + base_error_msg = ('The following error occurred while creating a new fork: {0}') if not rv.ok: # Lets see if the API returned an error message in JSON that we can # show the user - try: - rv_error = rv.json().get('error') - except ValueError: - rv_error = rv.text - if re.search(r"Repo .+ already exists", rv_error, re.IGNORECASE): return False diff --git a/test/test_utils.py b/test/test_utils.py index 904b3cb..9697512 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -355,22 +355,24 @@ class TestNewPagureIssue(unittest.TestCase): def test_raise_error_if_connection_error(self, post): post.side_effect = ConnectionError + logger = Mock() six.assertRaisesRegex( self, rpkgError, 'The connection to Pagure failed', utils.new_pagure_issue, - 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') + logger, 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') def test_responses_not_ok_and_response_body_is_not_json(self, post): rv = Mock(ok=False, text='error') rv.json.side_effect = ValueError post.return_value = rv + logger = Mock() six.assertRaisesRegex( self, rpkgError, 'The following error occurred while creating a new issue', utils.new_pagure_issue, - 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') + logger, 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') def test_responses_not_ok_when_token_is_expired(self, post): rv = Mock( @@ -379,23 +381,26 @@ class TestNewPagureIssue(unittest.TestCase): 'https://pagure.io/settings#api-keys to get or renew your API token.') rv.json.side_effect = ValueError post.return_value = rv + logger = Mock() six.assertRaisesRegex( self, rpkgError, 'For invalid or expired token refer to "fedpkg request-repo -h" to set ' 'a token in your user configuration.', utils.new_pagure_issue, - 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') + logger, 'http://distgit/', '123456', 'new package', {'repo': 'pkg1'}, 'fedpkg') def test_create_pagure_issue(self, post): rv = Mock(ok=True) rv.json.return_value = {'issue': {'id': 1}} post.return_value = rv + logger = Mock() pagure_api_url = 'http://distgit' issue_ticket_body = {'repo': 'pkg1'} - issue_url = utils.new_pagure_issue(pagure_api_url, + issue_url = utils.new_pagure_issue(logger, + pagure_api_url, '123456', 'new package', issue_ticket_body,