Merge lp:~jelmer/brz/credentials-error into lp:brz

[Jelmer Vernooij]

Raise a clearer error when not logged into e.g. GitHub.

[Martin Packman (gz)]

Not thrilled about this decorator usage, but do see it's convenient. Probably need to refactor the innards to avoid needing an outer wrapper like this.

review: Approve

=== modified file 'breezy/plugins/propose/github.py'
--- breezy/plugins/propose/github.py	2019-02-02 17:36:19 +0000
+++ breezy/plugins/propose/github.py	2019-02-10 18:54:22 +0000
@@ -22,6 +22,7 @@
 
 from .propose import (
     Hoster,
+    HosterLoginRequired,
     MergeProposal,
     MergeProposalBuilder,
     MergeProposalExists,
@@ -74,6 +75,11 @@
         self.url = url
 
 
+class GitHubLoginRequired(HosterLoginRequired):
+
+    _fmt = "Action requires GitHub login."
+
+
 def connect_github():
     """Connect to GitHub.
     """
@@ -145,6 +151,18 @@
         git_url_to_bzr_url(url), {"branch": branch_name})
 
 
+def convert_github_error(fn):
+    def convert(self, *args, **kwargs):
+        import github
+        try:
+            return fn(self, *args, **kwargs)
+        except github.GithubException as e:
+            if e.args[0] == 401:
+                raise GitHubLoginRequired(self)
+            raise
+    return convert
+
+
 class GitHub(Hoster):
 
     name = 'github'
@@ -163,6 +181,7 @@
     def __init__(self):
         self.gh = connect_github()
 
+    @convert_github_error
     def publish_derived(self, local_branch, base_branch, name, project=None,
                         owner=None, revision_id=None, overwrite=False,
                         allow_lossy=True):
@@ -201,11 +220,13 @@
         return push_result.target_branch, github_url_to_bzr_url(
             remote_repo.html_url, name)
 
+    @convert_github_error
     def get_push_url(self, branch):
         owner, project, branch_name = parse_github_url(branch)
         repo = self.gh.get_repo('%s/%s' % (owner, project))
         return github_url_to_bzr_url(repo.ssh_url, branch_name)
 
+    @convert_github_error
     def get_derived_branch(self, base_branch, name, project=None, owner=None):
         import github
         base_owner, base_project, base_branch_name = parse_github_url(base_branch)
@@ -221,9 +242,11 @@
         except github.UnknownObjectException:
             raise errors.NotBranchError('https://github.com/%s/%s' % (owner, project))
 
+    @convert_github_error
     def get_proposer(self, source_branch, target_branch):
         return GitHubMergeProposalBuilder(self.gh, source_branch, target_branch)
 
+    @convert_github_error
     def iter_proposals(self, source_branch, target_branch, status='open'):
         (source_owner, source_repo_name, source_branch_name) = (
             parse_github_url(source_branch))
@@ -269,6 +292,7 @@
     def iter_instances(cls):
         yield cls()
 
+    @convert_github_error
     def iter_my_proposals(self, status='open'):
         query = ['is:pr']
         if status == 'open':

=== modified file 'breezy/plugins/propose/propose.py'
--- breezy/plugins/propose/propose.py	2019-02-01 00:18:37 +0000
+++ breezy/plugins/propose/propose.py	2019-02-10 18:54:22 +0000
@@ -83,6 +83,16 @@
         self.hoster = hoster
 
 
+class HosterLoginRequired(errors.BzrError):
+    """Action requires hoster login credentials."""
+
+    _fmt = "Action requires credentials for hosting site %(hoster)r."""
+
+    def __init__(self, hoster):
+        errors.BzrError.__init__(self)
+        self.hoster = hoster
+
+
 class MergeProposal(object):
     """A merge proposal.
 
@@ -175,6 +185,8 @@
         :param base_branch: branch to derive the new branch from
         :param new_branch: branch to publish
         :return: resulting branch, public URL
+        :raise HosterLoginRequired: Action requires a hoster login, but none is
+            known.
         """
         raise NotImplementedError(self.publish)
 
@@ -225,6 +237,8 @@
         :param status: Only yield proposals with this status
             (one of: 'open', 'closed', 'merged', 'all')
         :return: Iterator over MergeProposal objects
+        :raise HosterLoginRequired: Action requires a hoster login, but none is
+            known.
         """
         raise NotImplementedError(self.iter_my_proposals)