Merge lp:~jelmer/brz/custom-github into lp:brz

[Jelmer Vernooij]

Use local REST implementation to access GitHub API.

Use local REST implementation to access GitHub API.

[Martin Packman (gz)]

Unrelated to PR content, REST when talking about apis is always all caps, ReST is only used for the python markup format?

Change looks good, thanks!

review: Approve

=== modified file 'breezy/plugins/propose/github.py'
--- breezy/plugins/propose/github.py	2019-05-21 01:25:23 +0000
+++ breezy/plugins/propose/github.py	2019-05-21 01:25:23 +0000
@@ -26,6 +26,7 @@
     MergeProposal,
     MergeProposalBuilder,
     MergeProposalExists,
+    NoSuchProject,
     PrerequisiteBranchUnsupported,
     UnsupportedHoster,
     )
@@ -39,15 +40,18 @@
     version_string as breezy_version,
     )
 from ...config import AuthenticationConfig, GlobalStack, config_dir
+from ...errors import InvalidHttpResponse
 from ...git.urls import git_url_to_bzr_url
 from ...i18n import gettext
 from ...sixish import PY3
 from ...trace import note
+from ...transport import get_transport
 from ...transport.http import default_user_agent
-from ...lazy_import import lazy_import
-lazy_import(globals(), """
-from github import Github
-""")
+
+
+GITHUB_HOST = 'github.com'
+WEB_GITHUB_URL = 'https://github.com'
+API_GITHUB_URL = 'https://api.github.com'
 
 
 def store_github_token(scheme, host, token):
@@ -87,13 +91,12 @@
     user_agent = default_user_agent()
     auth = AuthenticationConfig()
 
-    credentials = auth.get_credentials('https', 'github.com')
+    credentials = auth.get_credentials('https', GITHUB_HOST)
     if credentials is not None:
         return Github(credentials['user'], credentials['password'],
                       user_agent=user_agent)
 
-    # TODO(jelmer): token = auth.get_token('https', 'github.com')
-    token = retrieve_github_token('https', 'github.com')
+    # TODO(jelmer): token = auth.get_token('https', GITHUB_HOST)
     if token is not None:
         return Github(token, user_agent=user_agent)
     else:
@@ -108,25 +111,25 @@
 
     @property
     def url(self):
-        return self._pr.html_url
+        return self._pr['html_url']
 
     def _branch_from_part(self, part):
-        return github_url_to_bzr_url(part.repo.html_url, part.ref)
+        return github_url_to_bzr_url(part['repo']['html_url'], part['ref'])
 
     def get_source_branch_url(self):
-        return self._branch_from_part(self._pr.head)
+        return self._branch_from_part(self._pr['head'])
 
     def get_target_branch_url(self):
-        return self._branch_from_part(self._pr.base)
+        return self._branch_from_part(self._pr['base'])
 
     def get_description(self):
-        return self._pr.body
+        return self._pr['body']
 
     def set_description(self, description):
         self._pr.edit(body=description, title=determine_title(description))
 
     def is_merged(self):
-        return self._pr.merged
+        return self._pr['merged']
 
     def close(self):
         self._pr.edit(state='closed')
@@ -135,7 +138,7 @@
 def parse_github_url(url):
     (scheme, user, password, host, port, path) = urlutils.parse_url(
         url)
-    if host != 'github.com':
+    if host != GITHUB_HOST:
         raise NotGitHubUrl(url)
     (owner, repo_name) = path.strip('/').split('/')
     if repo_name.endswith('.git'):
@@ -156,18 +159,6 @@
         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'
@@ -177,41 +168,91 @@
     def __repr__(self):
         return "GitHub()"
 
+    def _api_request(self, method, path):
+        headers = {
+            'Accept': 'application/vnd.github.v3+json'}
+        if self._token:
+            headers['Authorization'] = 'token %s' % self._token
+        response = self.transport.request(
+            method, urlutils.join(self.transport.base, path),
+            headers=headers)
+        if response.status == 401:
+            raise GitHubLoginRequired(self)
+        return response
+
+    def _get_repo(self, path):
+        path = 'repos/' + path
+        response = self._api_request('GET', path)
+        if response.status == 404:
+            raise NoSuchProject(path)
+        if response.status == 200:
+            return response.json
+        raise InvalidHttpResponse(path, response.text)
+
+    def _get_user(self, username=None):
+        if username:
+            path = 'users/:%s' % username
+        else:
+            path = 'user'
+        response = self._api_request('GET', path)
+        if response.status != 200:
+            raise InvalidHttpResponse(path, response.text)
+        return response.json
+
+    def _get_organization(self, name):
+        path = 'orgs/:%s' % name
+        response = self._api_request('GET', path)
+        if response.status != 200:
+            raise InvalidHttpResponse(path, response.text)
+        return response.json
+
+    def _search_issues(self, query):
+        path = 'search/issues'
+        response = self._api_request(
+            'GET', path + '?q=' + urlutils.quote(query))
+        if response.status != 200:
+            raise InvalidHttpResponse(path, response.text)
+        return response.json
+
+    def _create_fork(self, repo, owner=None):
+        (orig_owner, orig_repo) = repo.split('/')
+        path = '/repos/:%s/:%s/forks' % (orig_owner, orig_repo)
+        if owner:
+            path += '?organization=%s' % owner
+        response = self._api_request('POST', path)
+        if response != 202:
+            raise InvalidHttpResponse(path, response.text)
+        return response.json
+
     @property
     def base_url(self):
-        # TODO(jelmer): Can we get the default URL from the Python API package
-        # somehow?
-        return "https://github.com"
-
-    def __init__(self):
-        self.gh = connect_github()
-
-    @convert_github_error
+        return WEB_GITHUB_URL
+
+    def __init__(self, transport):
+        self._token = retrieve_github_token('https', GITHUB_HOST)
+        self.transport = transport
+        self._current_user = self._get_user()
+
     def publish_derived(self, local_branch, base_branch, name, project=None,
                         owner=None, revision_id=None, overwrite=False,
                         allow_lossy=True):
         import github
         base_owner, base_project, base_branch_name = parse_github_branch_url(base_branch)
-        base_repo = self.gh.get_repo('%s/%s' % (base_owner, base_project))
+        base_repo = self._get_repo('%s/%s' % (base_owner, base_project))
         if owner is None:
-            owner = self.gh.get_user().login
+            owner = self._current_user['login']
         if project is None:
-            project = base_repo.name
+            project = base_repo['name']
         try:
-            remote_repo = self.gh.get_repo('%s/%s' % (owner, project))
-            remote_repo.id
+            remote_repo = self._get_repo('%s/%s' % (owner, project))
         except github.UnknownObjectException:
-            base_repo = self.gh.get_repo('%s/%s' % (base_owner, base_project))
-            if owner == self.gh.get_user().login:
-                owner_obj = self.gh.get_user()
-            else:
-                owner_obj = self.gh.get_organization(owner)
-            remote_repo = owner_obj.create_fork(base_repo)
+            base_repo = self._get_repo('%s/%s' % (base_owner, base_project))
+            remote_repo = self._create_fork(base_repo, owner)
             note(gettext('Forking new repository %s from %s') %
-                 (remote_repo.html_url, base_repo.html_url))
+                 (remote_repo['html_url'], base_repo['html_url']))
         else:
-            note(gettext('Reusing existing repository %s') % remote_repo.html_url)
-        remote_dir = controldir.ControlDir.open(git_url_to_bzr_url(remote_repo.ssh_url))
+            note(gettext('Reusing existing repository %s') % remote_repo['html_url'])
+        remote_dir = controldir.ControlDir.open(git_url_to_bzr_url(remote_repo['ssh_url']))
         try:
             push_result = remote_dir.push_branch(
                 local_branch, revision_id=revision_id, overwrite=overwrite,
@@ -223,41 +264,37 @@
                 local_branch, revision_id=revision_id,
                 overwrite=overwrite, name=name, lossy=True)
         return push_result.target_branch, github_url_to_bzr_url(
-            remote_repo.html_url, name)
+            remote_repo['html_url'], name)
 
-    @convert_github_error
     def get_push_url(self, branch):
         owner, project, branch_name = parse_github_branch_url(branch)
-        repo = self.gh.get_repo('%s/%s' % (owner, project))
-        return github_url_to_bzr_url(repo.ssh_url, branch_name)
+        repo = self._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_branch_url(base_branch)
-        base_repo = self.gh.get_repo('%s/%s' % (base_owner, base_project))
+        base_repo = self._get_repo('%s/%s' % (base_owner, base_project))
         if owner is None:
-            owner = self.gh.get_user().login
+            owner = self._current_user['login']
         if project is None:
-            project = base_repo.name
+            project = base_repo['name']
         try:
-            remote_repo = self.gh.get_repo('%s/%s' % (owner, project))
-            full_url = github_url_to_bzr_url(remote_repo.ssh_url, name)
+            remote_repo = self._get_repo('%s/%s' % (owner, project))
+            full_url = github_url_to_bzr_url(remote_repo['ssh_url'], name)
             return _mod_branch.Branch.open(full_url)
         except github.UnknownObjectException:
-            raise errors.NotBranchError('https://github.com/%s/%s' % (owner, project))
+            raise errors.NotBranchError('%s/%s/%s' % (WEB_GITHUB_URL, owner, project))
 
-    @convert_github_error
     def get_proposer(self, source_branch, target_branch):
-        return GitHubMergeProposalBuilder(self.gh, source_branch, target_branch)
+        return GitHubMergeProposalBuilder(self, 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_branch_url(source_branch))
         (target_owner, target_repo_name, target_branch_name) = (
             parse_github_branch_url(target_branch))
-        target_repo = self.gh.get_repo(
+        target_repo = self._get_repo(
             "%s/%s" % (target_owner, target_repo_name))
         state = {
             'open': 'open',
@@ -294,13 +331,14 @@
             parse_github_url(url)
         except NotGitHubUrl:
             raise UnsupportedHoster(url)
-        return cls()
+        transport = get_transport(
+            API_GITHUB_URL, possible_transports=possible_transports)
+        return cls(transport)
 
     @classmethod
     def iter_instances(cls):
-        yield cls()
+        yield cls(get_transport(API_GITHUB_URL))
 
-    @convert_github_error
     def iter_my_proposals(self, status='open'):
         query = ['is:pr']
         if status == 'open':
@@ -312,9 +350,10 @@
             query.append('is:closed')
         elif status == 'merged':
             query.append('is:merged')
-        query.append('author:%s' % self.gh.get_user().login)
-        for issue in self.gh.search_issues(query=' '.join(query)):
-            yield GitHubMergeProposal(issue.as_pull_request())
+        query.append('author:%s' % self._current_user['login'])
+        for issue in self._search_issues(query=' '.join(query))['items']:
+            yield GitHubMergeProposal(
+                self.transport.request('GET', issue['pull_request']['url']).json)
 
 
 class GitHubMergeProposalBuilder(MergeProposalBuilder):
@@ -354,7 +393,7 @@
         # TODO(jelmer): Probe for right repo name
         if self.target_repo_name.endswith('.git'):
             self.target_repo_name = self.target_repo_name[:-4]
-        target_repo = self.gh.get_repo("%s/%s" % (self.target_owner, self.target_repo_name))
+        target_repo = self.gh._get_repo("%s/%s" % (self.target_owner, self.target_repo_name))
         # TODO(jelmer): Allow setting title explicitly?
         title = determine_title(description)
         # TOOD(jelmer): Set maintainers_can_modify?
@@ -370,7 +409,7 @@
         if reviewers:
             for reviewer in reviewers:
                 pull_request.assignees.append(
-                    self.gh.get_user(reviewer))
+                    self.gh._get_user(reviewer)['login'])
         if labels:
             for label in labels:
                 pull_request.issue.labels.append(label)