From 9dea9969f51978271937311af4d6df3e1a814db8 Mon Sep 17 00:00:00 2001 From: Ondřej Nosek Date: Nov 01 2023 02:10:17 +0000 Subject: disable-monitoring: run manually only In the past, after the `retire` function, `disable-monitoring` was executed automatically. This functionality wasn't clear for users. Now only a manual run is possible. Also, more description was added to `disable-monitoring --help` page. It should prevent confusion when the operation fails (most likely because of a missing token or token with a lower ACL). JIRA: RHELCMP-11384 Fixes: #505 Signed-off-by: Ondřej Nosek --- diff --git a/fedpkg/cli.py b/fedpkg/cli.py index 8bb6fff..a9fd8d2 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -734,9 +734,12 @@ class fedpkgClient(cliClient): distgit_api_base_url = config_get_safely(self.config, distgit_section, "apibaseurl") description = textwrap.dedent(''' - Disable monitoring of the current repository + Disable monitoring of the current (retired) repository. - No arguments are needed. + Currently, it doesn't run automatically after the retire + operation - only manually. + + No arguments are required but a valid token is needed. Before the operation, you need to generate a pagure.io API token at: https://{1}/settings/token/new @@ -752,6 +755,8 @@ class fedpkgClient(cliClient): For example: [{0}.distgit] token = + + This operation can be run repeatedly till it succeeds. '''.format(self.name, urlparse(distgit_api_base_url).netloc)) disable_monitoring_parser = self.subparsers.add_parser( @@ -1557,30 +1562,42 @@ class fedpkgClient(cliClient): # Allow retiring in epel if is_epel(self.cmd.branch_merge): super(fedpkgClient, self).retire() - self.do_disable_monitoring() else: state = get_fedora_release_state(self.config, self.name, self.cmd.branch_merge) # Allow retiring in Rawhide and Branched until Final Freeze if state is None or state == 'pending': super(fedpkgClient, self).retire() - self.do_disable_monitoring() else: self.log.error("Fedora release (%s) is in state '%s' - retire operation " "is not allowed." % (self.cmd.branch_merge, state)) + if self.cmd.is_retired(): + self.log.info("To disable monitoring of the (retired) repository use manually " + "the separate command: '{0} disable-monitoring'".format(self.name)) + def do_disable_monitoring(self): """ - Disable monitoring when package is retired. + Disable monitoring when repository is retired. """ distgit_section = '{0}.distgit'.format(self.name) distgit_api_base_url = config_get_safely(self.config, distgit_section, "apibaseurl") distgit_token = config_get_safely(self.config, distgit_section, 'token') - disable_monitoring( - logger=self.log, - base_url=distgit_api_base_url, - token=distgit_token, - repo_name=self.cmd.repo_name, - namespace=self.cmd.ns, - cli_name=self.name, - ) + try: + if self.cmd.is_retired(): + disable_monitoring( + logger=self.log, + base_url=distgit_api_base_url, + token=distgit_token, + repo_name=self.cmd.repo_name, + namespace=self.cmd.ns, + cli_name=self.name, + ) + else: + self.log.error("ERROR: the repository is not yet retired.") + return 1 + except Exception: + self.log.error( + "In case of failure, please look at the help command " + "'{0} disable-monitoring --help'.\n".format(self.name)) + raise diff --git a/test/test_cli.py b/test/test_cli.py index aa1b350..1970515 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -2545,14 +2545,16 @@ class TestRetire(CliTestCase): cli.args.path = '/repo_path' cli.retire() + @patch('fedpkg.Commands.is_retired') @patch('requests.get') - def do_not_retire_release(self, branch, release_state, mock_get): + def do_not_retire_release(self, branch, release_state, mock_get, is_retired): mock_rv = Mock() mock_rv.ok = True mock_rv.json.return_value = { 'state': release_state, } mock_get.return_value = mock_rv + is_retired.return_value = False with patch('sys.argv', ['fedpkg', '--release', branch, 'retire', 'retire_message']): cli = self.new_cli(cfg='fedpkg-test.conf') diff --git a/test/test_retire.py b/test/test_retire.py index 0e2becb..98d7385 100644 --- a/test/test_retire.py +++ b/test/test_retire.py @@ -95,6 +95,10 @@ class RetireTestCase(unittest.TestCase): self.assertRetired('my reason') self.assertEqual(len(client.cmd.push.call_args_list), 1) + args = ['fedpkg', '--release=rawhide', 'disable-monitoring'] + client = self._fake_client(args) + client.do_disable_monitoring() + url = 'https://src.example.com/_dg/anitya/rpms/fedpkg' data = '{"anitya_status": "no-monitoring"}' headers = { @@ -119,17 +123,21 @@ class RetireTestCase(unittest.TestCase): args = ['fedpkg', '--release=rawhide', 'retire', 'my reason'] client = self._fake_client(args) + client.retire() + + self.assertRetired('my reason') + self.assertEqual(len(client.cmd.push.call_args_list), 1) + + args = ['fedpkg', '--release=rawhide', 'disable-monitoring'] + client = self._fake_client(args) six.assertRaisesRegex( self, rpkgError, "The following error occurred while disabling monitoring: Invalid or expired token\n" "For invalid or expired tokens please set a new token", - client.retire + client.do_disable_monitoring ) - self.assertRetired('my reason') - self.assertEqual(len(client.cmd.push.call_args_list), 1) - url = 'https://src.example.com/_dg/anitya/rpms/fedpkg' data = '{"anitya_status": "no-monitoring"}' headers = { @@ -139,9 +147,8 @@ class RetireTestCase(unittest.TestCase): } requests_post.assert_called_once_with(url, data=data, headers=headers, timeout=90) - @mock.patch('requests.post') @mock.patch("requests.get", new=lambda *args, **kwargs: mock.Mock(status_code=404)) - def test_retire_without_namespace(self, requests_post): + def test_retire_without_namespace(self): self._setup_repo('ssh://git@pkgs.example.com/fedpkg') args = ['fedpkg', '--release=rawhide', 'retire', 'my reason'] @@ -151,9 +158,8 @@ class RetireTestCase(unittest.TestCase): self.assertRetired('my reason') self.assertEqual(len(client.cmd.push.call_args_list), 1) - @mock.patch('requests.post') @mock.patch("requests.get", new=lambda *args, **kwargs: mock.Mock(status_code=404)) - def test_package_is_retired_already(self, requests_post): + def test_package_is_retired_already(self): self._setup_repo('ssh://git@pkgs.example.com/fedpkg') with open(os.path.join(self.tmpdir, 'dead.package'), 'w') as f: f.write('dead package')