Merge lp:~jelmer/brz/gitlab-422 into lp:brz/3.1

[Jelmer Vernooij]

Include response data when raising UnexpectedHttpStatus.

Raise more specific error when source branch doesn't derived from target branch.

Include response data when raising UnexpectedHttpStatus.

[Jelmer Vernooij (jelmer)]

review: Approve

=== modified file 'breezy/errors.py'
--- breezy/errors.py	2020-07-26 15:29:07 +0000
+++ breezy/errors.py	2020-08-02 03:22:50 +0000
@@ -1302,15 +1302,15 @@
 
 class UnexpectedHttpStatus(InvalidHttpResponse):
 
-    _fmt = "Unexpected HTTP status %(code)d for %(path)s"
+    _fmt = "Unexpected HTTP status %(code)d for %(path)s: %(extra)s"
 
-    def __init__(self, path, code, msg=None):
+    def __init__(self, path, code, extra=None):
         self.path = path
         self.code = code
-        self.msg = msg
+        self.extra = extra or ''
         full_msg = 'status code %d unexpected' % code
-        if msg is not None:
-            full_msg += ': ' + msg
+        if extra is not None:
+            full_msg += ': ' + extra
         InvalidHttpResponse.__init__(
             self, path, full_msg)
 

=== modified file 'breezy/plugins/gitlab/hoster.py'
--- breezy/plugins/gitlab/hoster.py	2020-07-26 17:39:41 +0000
+++ breezy/plugins/gitlab/hoster.py	2020-08-02 03:22:50 +0000
@@ -77,6 +77,14 @@
         self.url = url
 
 
+class GitLabUnprocessable(errors.BzrError):
+
+    _fmt = "GitLab can not process request: %(error)s."
+
+    def __init__(self, error):
+        errors.BzrError.__init__(self, error=error)
+
+
 class DifferentGitLabInstances(errors.BzrError):
 
     _fmt = ("Can't create merge proposals across GitLab instances: "
@@ -196,6 +204,11 @@
     return host, project_name, int(parts[-1])
 
 
+def _unexpected_status(path, response):
+    raise errors.UnexpectedHttpStatus(
+        path, response.status, response.data.decode('utf-8', 'replace'))
+
+
 class GitLabMergeProposal(MergeProposal):
 
     def __init__(self, gl, mr):
@@ -346,7 +359,7 @@
             raise KeyError('no such user %s' % username)
         if response.status == 200:
             return json.loads(response.data)
-        raise errors.UnexpectedHttpStatus(path, response.status)
+        _unexpected_status(path, response)
 
     def _get_user_by_email(self, email):
         path = 'users?search=%s' % urlutils.quote(str(email), '')
@@ -358,7 +371,7 @@
             if len(ret) != 1:
                 raise ValueError('unexpected number of results; %r' % ret)
             return ret[0]
-        raise errors.UnexpectedHttpStatus(path, response.status)
+        _unexpected_status(path, response)
 
     def _get_project(self, project_name):
         path = 'projects/%s' % urlutils.quote(str(project_name), '')
@@ -367,7 +380,7 @@
             raise NoSuchProject(project_name)
         if response.status == 200:
             return json.loads(response.data)
-        raise errors.UnexpectedHttpStatus(path, response.status)
+        _unexpected_status(path, response)
 
     def _fork_project(self, project_name, timeout=50, interval=5, owner=None):
         path = 'projects/%s/fork' % urlutils.quote(str(project_name), '')
@@ -381,7 +394,7 @@
             resp = json.loads(response.data)
             raise GitLabConflict(resp.get('message'))
         if response.status not in (200, 201):
-            raise errors.UnexpectedHttpStatus(path, response.status)
+            _unexpected_status(path, response)
         # The response should be valid JSON, but let's ignore it
         project = json.loads(response.data)
         # Spin and wait until import_status for new project
@@ -417,7 +430,7 @@
             if response.status == 403:
                 raise errors.PermissionDenied(response.text)
             if response.status != 200:
-                raise errors.UnexpectedHttpStatus(path, response.status)
+                _unexpected_status(path, response)
             page = response.getheader("X-Next-Page")
             for entry in json.loads(response.data):
                 yield entry
@@ -440,7 +453,7 @@
         if response.status == 403:
             raise errors.PermissionDenied(response.text)
         if response.status != 200:
-            raise errors.UnexpectedHttpStatus(path, response.status)
+            _unexpected_status(path, response)
         return json.loads(response.data)
 
     def _list_projects(self, owner):
@@ -454,7 +467,7 @@
         response = self._api_request('PUT', path, fields=mr)
         if response.status == 200:
             return json.loads(response.data)
-        raise errors.UnexpectedHttpStatus(path, response.status)
+        _unexpected_status(path, response)
 
     def _post_merge_request_note(self, project_id, iid, kwargs):
         path = 'projects/%s/merge_requests/%s/notes' % (
@@ -463,7 +476,7 @@
         if response.status == 201:
             json.loads(response.data)
             return
-        raise errors.UnexpectedHttpStatus(path, response.status)
+        _unexpected_status(path, response)
 
     def _create_mergerequest(
             self, title, source_project_id, target_project_id,
@@ -485,8 +498,11 @@
             raise errors.PermissionDenied(response.text)
         if response.status == 409:
             raise MergeRequestExists()
+        if response.status == 422:
+            data = json.loads(response.data)
+            raise GitLabUnprocessable(data['error'])
         if response.status != 201:
-            raise errors.UnexpectedHttpStatus(path, response.status)
+            _unexpected_status(path, response)
         return json.loads(response.data)
 
     def get_push_url(self, branch):
@@ -634,7 +650,7 @@
         if response.status == 404:
             raise NoSuchProject(project)
         if response.status != 202:
-            raise errors.UnexpectedHttpStatus(path, response.status)
+            _unexpected_status(path, response)
 
 
 class GitlabMergeProposalBuilder(MergeProposalBuilder):
@@ -703,6 +719,11 @@
             merge_request = self.gl._create_mergerequest(**kwargs)
         except MergeRequestExists:
             raise MergeProposalExists(self.source_branch.user_url)
+        except GitLabUnprocessable as e:
+            if e.error == [
+                    "Source project is not a fork of the target project"]:
+                raise SourceNotDerivedFromTarget(
+                    self.source_branch, self.target_branch)
         return GitLabMergeProposal(self.gl, merge_request)
 
 

=== modified file 'breezy/propose.py'
--- breezy/propose.py	2020-07-24 17:47:14 +0000
+++ breezy/propose.py	2020-08-02 03:22:50 +0000
@@ -99,6 +99,18 @@
         self.hoster = hoster
 
 
+class SourceNotDerivedFromTarget(errors.BzrError):
+    """Source branch is not derived from target branch."""
+
+    _fmt = ("Source %(source_branch)r not derived from "
+            "target %(target_branch)r.")
+
+    def __init__(self, source_branch, target_branch):
+        errors.BzrError.__init__(
+            self, source_branch=source_branch,
+            target_branch=target_branch)
+
+
 class MergeProposal(object):
     """A merge proposal.