Merge lp:~albaguirre/jenkins-launchpad-plugin/support-query-git-mps into lp:jenkins-launchpad-plugin

[Alberto Aguirre]

Add support to query merge proposals from launchpad git repos

Needs lp_version: devel in the configuration to be able to run.

Example:
alberto@skynet97:~/source/jenkins-launchpad-plugin$ ./getMergeProposals.py --merge_type "Needs review" --repo_type git repowerd
DEBUG: [repowerd] merge proposals: [<branch_merge_proposal at https://api.launchpad.net/devel/~albaguirre/repowerd/+git/repowerd/+merge/301919>]
DEBUG: count: 1

[lp:~albaguirre/jenkins-launchpad-plugin/support-query-git-mps]

132. By Alberto Aguirre on 2016-08-09

Small cleanup, some more tests

133. By Alberto Aguirre on 2016-08-09

Fix too much indent on doc comment

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Francis Ginther (fginther)]

Thanks for working on adding git support. I'd like to get these three MPs merged, but I think this can be done without requiring an explicit repo_type option for every git repo. I think it's reasonable to have the code attempt to lookup both a bazaar branch and git repo based on the name and select the branch or repo that isn't 'None'. For example, the following can be used to convert the user provided name ('lp:jenkins-launchpad-plugin' or 'lp:some-git-repo') into a branch object or a repo_type dynamically:

def get_branch_handle_for(lp_handle, name, repo_type='auto'):
    """ Return a branch/repo handle for the given name.

    Returns the repo_type and a launchpad branch or git repository
      handle for the given name.
    :param lp_handle: launchpad API handle/instance
    :param name: name of the branch or git repository
    :param repo_type: Type of repository [bazaar, git or auto]
    """
    logger.debug('Provided branch name: {}'.format(name))
    branch = lp_handle.branches.getByUrl(url=name)
    logger.debug('bazaar branch: {}'.format(branch))
    if repo_type == 'bazaar':
        return repo_type, branch
    try:
        repo = lp_handle.git_repositories.getByPath(
            path=name.replace('lp:', ''))
    except AttributeError:
        logger.debug('git_repositories.getByPath was not found. You may need '
                     'to set lp_version:devel in the config')
        if repo_type == 'git':
            raise RuntimeError('Git support is incomplete, '
                               'set lp_version:devel in the config.')
    logger.debug('git repo: {}'.format(repo))
    if repo_type == 'git':
        return repo_type, repo
    elif repo_type == 'auto':
        if branch is not None and repo is not None:
            # Ambiguous name, can't automatically select
            raise RuntimeError('Ambiguous branch name: {} resolves to '
                               '{} and {}'.format(name, branch, repo))
        if branch is not None:
            return 'bazaar', branch
        elif repo is not None:
            return 'git', repo
        else:
            raise RuntimeError('Could not determine branch or repo from '
                               '{}'.format(name))
    else:
        logger.debug('Repo type "' + repo_type + '" is not valid')

    return None, None

This then returns the repo_typo that was discovered along with the branch or repo object. Unfortunately, launchpad allows git repos and bazaar branches to have the same name, so it's possible to encounter cases where this lookup will return both, which is why this method raises an error when that happens. However, I think in practice this will almost never happen as teams will either choose bazaar or git for their project and not use both. To deal with this case, I do think it's ok to have an optional 'repo_type' that can be either 'bazaar', 'git' or 'auto'. Using 'bazaar' or 'git' will force it to use one of those methods. 'auto', the default would perform the lookup.

This is my only comment for the set of three git MPs. If you are no longer interested in working these, I'd like to take them over and move them forward to merge them into trunk.

review: Needs Fixing

[Alberto Aguirre (albaguirre)]

" I think in practice this will almost never happen as teams will either choose bazaar or git for their project"

The original reason is that I was doing these changes to add jenkaas support for the repowerd project which indeed does have git repos and bazaar branches, but yeah an auto option would be reasonable.

" If you are no longer interested in working these, I'd like to take them over and move them forward to merge them into trunk."

Cool! I probably won't have time to work on these for a while, so it would be appreciated.

[Joshua Powers (powersj)]

Any update on this MR? I am more than willing to help out as we really miss having reviews of cloud-init.

=== modified file 'jlp.config'
--- jlp.config	2015-12-14 04:36:05 +0000
+++ jlp.config	2016-08-09 21:56:43 +0000
@@ -57,6 +57,10 @@
 #you don't need to change this
 lp_env: production
 
+#which launchpad api version to use
+#devel version includes additional querying support for git repos
+#lp_version: devel
+
 #URL of your public jenkins in case you are publishing your jobs to some
 #other jenkins
 public_jenkins_url: http://jenkins.qa.ubuntu.com

=== modified file 'jlp/commands/getMergeProposals.py'
--- jlp/commands/getMergeProposals.py	2015-09-24 16:09:07 +0000
+++ jlp/commands/getMergeProposals.py	2016-08-09 21:56:43 +0000
@@ -5,7 +5,7 @@
 from shutil import rmtree
 
 
-def getMergeCount(branch, merge_type):
+def getMergeCount(branch, merge_type, repo_type):
     """Returns the total number of outstanding merge requests for the given
     type."""
     count = 0
@@ -23,7 +23,7 @@
                     ignore_errors=True)
     lp_handle = get_launchpad(launchpadlib_dir=launchpad_cachedir)
     for b in branch:
-        mps = launchpadutils.get_merge_proposals(lp_handle, b, [merge_type])
+        mps = launchpadutils.get_merge_proposals(lp_handle, b, [merge_type], repo_type)
         logger.debug("[%s] merge proposals: %s" % (b, mps))
         logger.debug("count: %d" % (len(mps)))
         count += len(mps)
@@ -35,6 +35,8 @@
                             "proposals.")
     parser.add_argument("-t", "--merge_type", default="Approved",
                         help="Count only this type of merge proposals.")
+    parser.add_argument("-r", "--repo_type", default="bzr",
+                        help="Repository type [bzr or git]")
     parser.add_argument("branch", nargs="+",
                         help="Branch to check merge count.")
     args = vars(parser.parse_args())

=== modified file 'jlp/launchpadagent.py'
--- jlp/launchpadagent.py	2013-05-10 14:10:52 +0000
+++ jlp/launchpadagent.py	2016-08-09 21:56:43 +0000
@@ -61,8 +61,14 @@
         get_config_option('credential_store_path'))
     lp_app = get_config_option('lp_app')
     lp_env = get_config_option('lp_env')
+    try:
+        lp_version = get_config_option('lp_version')
+    except KeyError:
+        lp_version = Launchpad.DEFAULT_VERSION;
+
     authorization_engine = AuthorizeRequestTokenWithConsole(lp_env, lp_app)
     return Launchpad.login_with(lp_app, lp_env,
                                 credential_store=store,
                                 authorization_engine=authorization_engine,
-                                launchpadlib_dir=launchpadlib_dir)
+                                launchpadlib_dir=launchpadlib_dir,
+                                version=lp_version)

=== modified file 'jlp/launchpadutils.py'
--- jlp/launchpadutils.py	2015-01-07 16:27:30 +0000
+++ jlp/launchpadutils.py	2016-08-09 21:56:43 +0000
@@ -392,20 +392,44 @@
     else:
         raise UpdateMergeProposalException("unrecognized parameters")
 
-
-def get_merge_proposals(lp_handle, branch_name, state):
-    """ Return all merge proposal for a given branch.
+def get_branch_handle_for(lp_handle, name, repo_type):
+    """ Return a branch/repo handle for the given name.
+
+    Returns a launchpad branch or git repository handle for the given name.
+    :param lp_handle: launchpad API handle/instance
+    :param name: name of the branch or git repository
+    :param repo_type: Type of repository [bzr or git]
+    """
+
+    if 'bzr' == repo_type:
+        return lp_handle.branches.getByUrl(url=name)
+    elif 'git' == repo_type:
+        name = name.replace('lp:', '');
+        try:
+            return lp_handle.git_repositories.getByPath(path=name)
+        except AttributeError:
+            logger.debug('git_repositories.getByPath was not found. You may need to set lp_version:devel in the config')
+            return None
+    else:
+        logger.debug('Repo type "' + repo_type + '" is not valid')
+        return None
+
+def get_merge_proposals(lp_handle, name, state, repo_type='bzr'):
+    """ Return all merge proposals for a given branch.
 
     Returns a list of launchpad handles for merge proposals.
     :param lp_handle: launchpad API handle/instance
-    :param branch_name: name of the branch
+    :param name: name of the branch or git repository
     :param state: list of states I'm interested in (usually something like
     ['Approved'] or ['Needs review'])
+    :param repo_type: Type of repository [bzr or git]
     """
-    branch = lp_handle.branches.getByUrl(url=branch_name)
+
+    branch = get_branch_handle_for(lp_handle, name, repo_type)
     if not branch:
-        logger.debug('Branch "' + branch_name + '" doesn\'t exist')
+        logger.debug('Branch "' + name + '" doesn\'t exist')
         return []
+
     mps = []
     for mp in branch.landing_candidates:
         if mp.queue_status in state:

=== modified file 'tests/test_launchpadutils.py'
--- tests/test_launchpadutils.py	2015-01-07 21:25:30 +0000
+++ tests/test_launchpadutils.py	2016-08-09 21:56:43 +0000
@@ -565,6 +565,53 @@
         self.assertEqual(len(mps), 1)
         self.assertEqual(mps[0].queue_status, 2)
 
+class TestGetGitMergeProposals(unittest.TestCase):
+    git_repo_with_mps = None
+    mp_status = range(1,10)
+
+    def setUp(self):
+        repo = MagicMock()
+        repo.landing_candidates = []
+        for i in self.mp_status:
+            mp = MagicMock()
+            mp.queue_status = i
+            repo.landing_candidates.append(mp)
+        self.git_repo_with_mps = repo
+
+    def _get_launchpad(self, getByPath_result=None):
+        launchpad = MagicMock()
+        launchpad.git_repositories.getByPath = lambda path: getByPath_result
+        return launchpad
+
+    def raise_attribute_error(self):
+        raise AttributeError
+
+    def _get_launchpad_without_git(self):
+        launchpad = MagicMock()
+        launchpad.git_repositories = lambda path: self.raise_attribute_error
+        return launchpad
+
+    def test_get_git_merge_proposals(self):
+        launchpad = self._get_launchpad(self.git_repo_with_mps)
+        mps = launchpadutils.get_merge_proposals(launchpad, '', self.mp_status, repo_type='git')
+        self.assertEqual(len(mps), len(self.mp_status))
+        for mp, expected_status in zip(mps, self.mp_status):
+            self.assertEqual(mp.queue_status, expected_status)
+
+    def test_get_git_merge_proposals_with_invalid_branch(self):
+        launchpad = self._get_launchpad(None)
+        ret = launchpadutils.get_merge_proposals(launchpad, '', '', repo_type='git')
+        self.assertEqual(ret, [])
+
+    def test_get_git_merge_proposals_with_wrong_launchpad_version(self):
+        launchpad = self._get_launchpad_without_git()
+        ret = launchpadutils.get_merge_proposals(launchpad, '', '', repo_type='git')
+        self.assertEqual(ret, [])
+
+    def test_invalid_repo_type(self):
+        launchpad = self._get_launchpad(None)
+        ret = launchpadutils.get_merge_proposals(launchpad, '', '', repo_type='test')
+        self.assertEqual(ret, [])
 
 class TestGetMpHandleFromURL(unittest.TestCase):
     @patch("jlp.launchpadutils.get_branch_from_mp", new=lambda x: None)
@@ -598,3 +645,30 @@
         launchpad.branches.getByUrl = lambda url: branch
         ret = launchpadutils.get_mp_handle_from_url(launchpad, mp_url)
         self.assertEquals(ret, mp)
+
+class TestGetBranchHandle(unittest.TestCase):
+    def test_uses_bzr_api_for_bzr_type(self):
+        launchpad = MagicMock()
+        name = 'lp:dummy'
+        getByUrlMock = MagicMock()
+        dummyHandle = MagicMock()
+        getByUrlMock.return_value = dummyHandle
+        launchpad.branches.getByUrl = getByUrlMock
+
+        result = launchpadutils.get_branch_handle_for(launchpad, name, 'bzr');
+
+        launchpad.branches.getByUrl.assert_called_once_with(url=name)
+        self.assertEqual(result, dummyHandle)
+
+    def test_uses_git_api_for_git_type(self):
+        launchpad = MagicMock()
+        name = 'lp:dummy'
+        getByPathMock = MagicMock()
+        dummyHandle = MagicMock()
+        getByPathMock.return_value = dummyHandle
+        launchpad.git_repositories.getByPath = getByPathMock
+
+        result = launchpadutils.get_branch_handle_for(launchpad, name, 'git');
+
+        launchpad.git_repositories.getByPath.assert_called_once_with(path='dummy')
+        self.assertEqual(result, dummyHandle)
\ No newline at end of file