fedpkg

Clone

#485 `request-branch`: detect existing branch
Merged 2 years ago by onosek. Opened 2 years ago by onosek.
onosek/fedpkg existing_branch  into  master

file modified
+26 -12 
@@ -24,21 +24,21 @@ 
 

  

  import pkg_resources
 

  import six
 

+ from pyrpkg import rpkgError
 

+ from pyrpkg.cli import cliClient
 

  from six.moves import configparser
 

- from six.moves.configparser import NoOptionError, NoSectionError, ConfigParser
 

+ from six.moves.configparser import ConfigParser, NoOptionError, NoSectionError
 

  from six.moves.urllib_parse import urlparse
 

  

  from fedpkg.bugzilla import BugzillaClient
 

- from fedpkg.completers import (build_arches, list_targets,
 

-                                fedpkg_packages, distgit_branches)
 

+ from fedpkg.completers import (build_arches, distgit_branches, fedpkg_packages,
 

+                                list_targets)
 

  from fedpkg.utils import (assert_new_tests_repo, assert_valid_epel_package,
 

                            config_get_safely, do_add_remote, do_fork,
 

                            expand_release, get_dist_git_url,
 

-                           get_fedora_release_state, get_release_branches,
 

-                           get_stream_branches, is_epel, new_pagure_issue,
 

-                           sl_list_to_dict, verify_sls)
 

- from pyrpkg import rpkgError
 

- from pyrpkg.cli import cliClient
 

+                           get_fedora_release_state, get_pagure_branches,
 

+                           get_release_branches, get_stream_branches, is_epel,
 

+                           new_pagure_issue, sl_list_to_dict, verify_sls)
 

  

  RELEASE_BRANCH_REGEX = r'^(f\d+|el\d+|epel\d+)$'
 

  LOCAL_PACKAGE_CONFIG = 'package.cfg'
 
@@ -1063,12 +1063,13 @@ 
 

              no_auto_module=self.args.no_auto_module,
 

              name=self.name,
 

              config=self.config,
 

+             anongiturl=self.cmd.anongiturl,
 

          )
 

  

      @staticmethod
 

      def _request_branch(logger, service_levels, all_releases, branch,
 

                          active_branch, repo_name, ns, no_git_branch,
 

-                         no_auto_module, name, config):
 

+                         no_auto_module, name, config, anongiturl):
 

          """ Implementation of `request_branch`.
 

  

          Submits a request for a new branch of a given dist-git repo.
 
@@ -1094,6 +1095,8 @@ 
 

              used.  Typically the value of `self.name`.
 

          :param config: A dict containing the configuration, loaded from file.
 

              Typically the value of `self.config`.
 

+         :param anongiturl: A string with the name of the anonymous git url.
 

+             Typically the value of `self.cmd.anongiturl`.
 

          :return: None
 

          """
 

  
@@ -1193,9 +1196,19 @@ 
 

              ticket_title = 'New Branch "{0}" for "{1}/{2}"'.format(
 

                  b, ns, repo_name)
 

  

-             print(new_pagure_issue(
 

-                 logger, pagure_url, pagure_token, ticket_title, ticket_body,
 

-                 name))
 

+             # check whether the requested branch was already created
 

+             if b not in get_pagure_branches(
 

+                 logger=logger,
 

+                 url=get_dist_git_url(anongiturl),
 

+                 namespace=ns,
 

+                 repo_name=repo_name
 

+             ):
 

+                 print(new_pagure_issue(
 

+                     logger, pagure_url, pagure_token, ticket_title, ticket_body,
 

+                     name))
 

+             else:
 

+                 logger.warning('Requested branch "{0}" already exists. '
 

+                                'Skipping the operation.'.format(b))
 

  

              # For non-standard rpm branch requests, also request a matching new
 

              # module repo with a matching branch.
 
@@ -1235,6 +1248,7 @@ 
 

                      no_auto_module=True,  # Avoid infinite recursion.
 

                      name=name,
 

                      config=config,
 

+                     anongiturl=anongiturl,
 

                  )
 

  

      def do_distgit_fork(self):

file modified
+47 -2 
@@ -16,12 +16,11 @@ 
 

  

  import git
 

  import requests
 

+ from pyrpkg import rpkgError
 

  from requests.exceptions import ConnectionError
 

  from six.moves.configparser import NoOptionError, NoSectionError
 

  from six.moves.urllib.parse import urlencode, urlparse
 

  

- from pyrpkg import rpkgError
 

- 

  

  def query_pdc(server_url, endpoint, params, timeout=60):
 

      api_url = '{0}/rest_api/v1/{1}/'.format(
 
@@ -234,6 +233,52 @@ 
 

      return True
 

  

  

+ def get_pagure_branches(logger, url, namespace, repo_name):
 

+     """
 

+     Returns list of branches for the repo from Pagure dist-git.
 

+     :param logger: A logger object
 

+     :param url: a string of the URL to Pagure
 

+     :param namespace: a string determines a type of the repository
 

+     :param repo_name: a string of the repository name
 

+     :return: a list of branches
 

+     """
 

+     api_url = '{0}/api/0'.format(url.rstrip('/'))
 

+     get_branches_url = '{0}/{1}/{2}/git/branches'.format(api_url, namespace, repo_name)
 

+ 

+     headers = {
 

+         'Accept': 'application/json',
 

+         'Content-Type': 'application/json'
 

+     }
 

+     try:
 

+         logger.debug('Pagure API request: {0}'.format(get_branches_url))
 

+         rv = requests.get(
 

+             get_branches_url, headers=headers, timeout=60)
 

+     except ConnectionError as error:
 

+         error_msg = ('The connection to Pagure failed while getting a list '
 

+                      'of branches from Pagure. The error was: {0}'.format(
 

+                          str(error)))
 

+         raise rpkgError(error_msg)
 

+ 

+     base_error_msg = ('The following error occurred while getting a list  '
 

+                       'of branches from 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:
 

+         # When specific namespace has no branches, API returns error "Project not found".
 

+         # Do not fail. Return "no branches found" instead.
 

+         logger.debug(base_error_msg.format(rv_error))
 

+         return ()
 

+ 

+     return rv_json.get("branches", ())
 

+ 

+ 

  def get_release_branches(server_url):
 

      """
 

      Get the active Fedora release branches from PDC

file modified
+109 -11 
@@ -913,17 +913,25 @@ 
 

          with patch('sys.argv', new=cli_cmd):
 

              return self.new_cli(name=name, cfg=cfg, user_cfg=user_cfg)
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('sys.stdout', new=StringIO())
 

-     def test_request_branch(self, mock_grb, mock_request_post):
 

+     def test_request_branch(self, mock_grb, mock_request_post, mock_request_get):
 

          """Tests request-branch"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {'issue': {'id': 2}}
 

          mock_request_post.return_value = mock_rv
 

+ 

          # Checkout the f27 branch
 

          self.run_cmd(['git', 'checkout', 'f27'], cwd=self.cloned_repo_path)
 

  
@@ -932,6 +940,8 @@ 
 

          cli = self.get_cli(cli_cmd)
 

          cli.request_branch()
 

  

+         self.assertEqual(1, mock_request_get.call_count)
 

+ 

          expected_issue_content = {
 

              'action': 'new_branch',
 

              'repo': 'testpkg',
 
@@ -949,13 +959,47 @@ 
 

                             'fedora-scm-requests/issue/2')
 

          self.assertEqual(output, expected_output)
 

  

+     @patch('requests.get')
 

+     @patch('requests.post')
 

+     @patch('fedpkg.cli.get_release_branches')
 

+     @patch('sys.stdout', new=StringIO())
 

+     def test_request_existing_branch(self, mock_grb, mock_request_post, mock_request_get):
 

+         """Tests request-existing-branch"""
 

+         mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

+                                  'epel': ['el6', 'epel7']}
 

+ 

+         # branch 'f27' already exists and there is no POST request sent to Pagure (it would create
 

+         # an issue)
 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "f27", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

+         # Checkout the f27 branch
 

+         self.run_cmd(['git', 'checkout', 'f27'], cwd=self.cloned_repo_path)
 

+ 

+         cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path,
 

+                    'request-branch']
 

+         cli = self.get_cli(cli_cmd)
 

+         cli.request_branch()
 

+ 

+         self.assertEqual(1, mock_request_get.call_count)
 

+         mock_request_post.assert_not_called()
 

+ 

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('sys.stdout', new=StringIO())
 

-     def test_request_branch_override(self, mock_grb, mock_request_post):
 

+     def test_request_branch_override(self, mock_grb, mock_request_post, mock_request_get):
 

          """Tests request-branch with an overridden package and branch name"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {'issue': {'id': 2}}
 
@@ -966,6 +1010,8 @@ 
 

          cli = self.get_cli(cli_cmd)
 

          cli.request_branch()
 

  

+         self.assertEqual(1, mock_request_get.call_count)
 

+ 

          expected_issue_content = {
 

              'action': 'new_branch',
 

              'repo': 'nethack',
 
@@ -1010,6 +1056,8 @@ 
 

          cli = self.get_cli(cli_cmd)
 

          cli.request_branch()
 

  

+         self.assertEqual(2, mock_request_get.call_count)
 

+ 

          expected_issue_content = {
 

              'action': 'new_branch',
 

              'repo': 'sudoku',
 
@@ -1040,15 +1088,17 @@ 
 

          """Tests request-epel-branch-override"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7', 'epel8']}
 

-         mock_rv = Mock()
 

-         mock_rv.ok = True
 

-         mock_rv.json.return_value = {'issue': {'id': 2}}
 

-         mock_request_post.return_value = mock_rv
 

  

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {"arches": [], "packages": {}}
 

          mock_request_get.return_value = mock_rv
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {'issue': {'id': 2}}
 

+         mock_request_post.return_value = mock_rv
 

+ 

          # Checkout the epel7 branch
 

          self.run_cmd(['git', 'checkout', 'epel7'], cwd=self.cloned_repo_path)
 

  
@@ -1077,13 +1127,20 @@ 
 

                             'fedora-scm-requests/issue/2')
 

          self.assertEqual(output, expected_output)
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('sys.stdout', new=StringIO())
 

-     def test_request_branch_module(self, mock_grb, mock_request_post):
 

+     def test_request_branch_module(self, mock_grb, mock_request_post, mock_request_get):
 

          """Tests request-branch for a new module branch"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {'issue': {'id': 2}}
 
@@ -1112,12 +1169,20 @@ 
 

                             'fedora-scm-requests/issue/2')
 

          self.assertEqual(output, expected_output)
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

-     def assert_request_branch_container(self, cli_cmd, mock_grb, mock_request_post):
 

+     def assert_request_branch_container(self, cli_cmd, mock_grb, mock_request_post,
 

+                                         mock_request_get):
 

          """Tests request-branch for a new container branch"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {'issue': {'id': 2}}
 
@@ -1157,15 +1222,28 @@ 
 

                     '--repo', 'nethack', '--namespace', 'container', 'f27']
 

          self.assert_request_branch_container(cli_cmd)
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('fedpkg.cli.verify_sls')
 

      @patch('sys.stdout', new=StringIO())
 

      def test_request_branch_sls(self, mock_verify_sls, mock_grb,
 

-                                 mock_request_post):
 

+                                 mock_request_post, mock_request_get):
 

          """Tests request-branch with service levels"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         responses = []
 

+         mock_rv_get = Mock()
 

+         mock_rv_get.ok = True
 

+         mock_rv_get.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         responses.append(mock_rv_get)
 

+         mock_rv_get = Mock()
 

+         mock_rv_get.ok = False
 

+         mock_rv_get.json.return_value = {'error': 'Project not found', 'error_code': 'ENOPROJECT'}
 

+         responses.append(mock_rv_get)
 

+         mock_request_get.side_effect = responses
 

+ 

          responses = []
 

          for idx in range(2, 5):
 

              mock_rv_post = Mock()
 
@@ -1180,6 +1258,7 @@ 
 

          cli = self.get_cli(cli_cmd)
 

          cli.request_branch()
 

  

+         self.assertEqual(2, mock_request_get.call_count)
 

          # Get the data that was submitted to Pagure
 

          output = sys.stdout.getvalue().strip()
 

          # Three bugs are filed.  One for the rpm branch, and one for a new
 
@@ -1247,13 +1326,23 @@ 
 

              post_data)['issue_content'].strip('```'))
 

          self.assertDictEqual(expected_issue_content, actual_issue_content)
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('sys.stdout', new=StringIO())
 

-     def test_request_branch_all_releases(self, mock_grb, mock_request_post):
 

+     def test_request_branch_all_releases(self, mock_grb, mock_request_post, mock_request_get):
 

          """Tests request-branch with the '--all-releases' option """
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         get_side_effect = []
 

+         for i in range(1, 4):
 

+             mock_rv = Mock()
 

+             mock_rv.ok = True
 

+             mock_rv.json.return_value = {"branches": ["main", "rawhide"]}
 

+             get_side_effect.append(mock_rv)
 

+         mock_request_post.side_effect = get_side_effect
 

+ 

          post_side_effect = []
 

          for i in range(1, 4):
 

              mock_rv = Mock()
 
@@ -1268,6 +1357,8 @@ 
 

          cli = self.get_cli(cli_cmd)
 

          cli.request_branch()
 

  

+         self.assertEqual(3, mock_request_get.call_count)
 

+ 

          for i in range(3):
 

              expected_issue_content = {
 

                  'action': 'new_branch',
 
@@ -1431,13 +1522,20 @@ 
 

          with six.assertRaisesRegex(self, rpkgError, expected_error):
 

              cli.request_branch()
 

  

+     @patch('requests.get')
 

      @patch('requests.post')
 

      @patch('fedpkg.cli.get_release_branches')
 

      @patch('sys.stdout', new=StringIO())
 

-     def test_request_with_repo_option(self, mock_grb, mock_request_post):
 

+     def test_request_with_repo_option(self, mock_grb, mock_request_post, mock_request_get):
 

          """Test request branch with option --repo"""
 

          mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],
 

                                   'epel': ['el6', 'epel7']}
 

+ 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {"branches": ["f25", "f26", "main", "rawhide"]}
 

+         mock_request_get.return_value = mock_rv
 

+ 

          mock_rv = Mock()
 

          mock_rv.ok = True
 

          mock_rv.json.return_value = {'issue': {'id': 2}}

Prevent opening an issue for 'fedora-scm-requests' team to create
a new branch when the branch already exists.

JIRA: RHELCMP-9404
Fixes: #481

Signed-off-by: Ondrej Nosek onosek@redhat.com

Pull-Request has been merged by onosek
2 years ago
Metadata
None
No Tags
Changes Summary 3
+26 -12
file changed
fedpkg/cli.py
+47 -2
file changed
fedpkg/utils.py
+109 -11
file changed
test/test_cli.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.