Przeglądaj źródła

[notebook] Update github_fetch handling and GithubClient

github_fetch returns 400 on malformed requests
GithubClient better regex parsing of URLs and better error handling
Jenny Kim 10 lat temu
rodzic
commit
d35f6c8

+ 6 - 0
desktop/conf/pseudo-distributed.ini.tmpl

@@ -600,6 +600,12 @@
   ## Main flag to override the automatic starting of the DBProxy server.
   # enable_dbproxy_server=true
 
+  ## Base URL to Remote Github Server
+  # github_remote_url=https://github.com
+
+  ## Base URL to Github API
+  # github_api_url=https://api.github.com
+
 
 ###########################################################################
 # Settings to configure your Hadoop cluster.

+ 3 - 3
desktop/libs/notebook/src/notebook/api.py

@@ -18,6 +18,7 @@
 import json
 import logging
 
+from django.http import HttpResponseBadRequest
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_GET, require_POST
 
@@ -30,8 +31,6 @@ from notebook.decorators import api_error_handler, check_document_modify_permiss
 from notebook.github import GithubClient, GithubClientException
 from notebook.models import escape_rows
 
-import requests
-
 
 LOG = logging.getLogger(__name__)
 
@@ -301,6 +300,7 @@ def autocomplete(request, database=None, table=None, column=None, nested=None):
   return JsonResponse(response)
 
 
+@require_GET
 @api_error_handler
 def github_fetch(request):
   response = {'status': -1}
@@ -316,6 +316,6 @@ def github_fetch(request):
     content = api.get_file_contents(owner, repo, filepath, branch)
     response['content'] = json.loads(content)
   else:
-    response['message'] = _('fetch_github requires full URL to Github file.')
+    return HttpResponseBadRequest(_('url param is required'))
 
   return JsonResponse(response)

+ 17 - 1
desktop/libs/notebook/src/notebook/conf.py

@@ -18,7 +18,7 @@
 from django.utils.translation import ugettext_lazy as _t
 
 from desktop.lib.conf import Config, UnspecifiedConfigSection, ConfigSection,\
-  coerce_json_dict
+  coerce_json_dict, coerce_string
 
 
 def get_interpreters():
@@ -64,3 +64,19 @@ ENABLE_DBPROXY_SERVER = Config(
   help=_t("Main flag to override the automatic starting of the DBProxy server."),
   type=bool,
   default=True)
+
+
+GITHUB_REMOTE_URL = Config(
+    "github_remote_url",
+    help="Base URL to Github Remote Server",
+    default='https://github.com',
+    type=coerce_string,
+)
+
+
+GITHUB_API_URL = Config(
+    "github_api_url",
+    help="Base URL to Github API",
+    default='https://api.github.com',
+    type=coerce_string,
+)

+ 58 - 55
desktop/libs/notebook/src/notebook/github.py

@@ -25,6 +25,8 @@ import urllib
 from desktop.lib.rest.http_client import HttpClient, RestException
 from desktop.lib.rest import resource
 
+from notebook.conf import GITHUB_REMOTE_URL, GITHUB_API_URL
+
 
 LOG = logging.getLogger(__name__)
 
@@ -38,55 +40,53 @@ class GithubClient(object):
   https://developer.github.com/v3/
   """
 
-  BASE_URL = 'https://api.github.com'
-
-  REPO_URL_RE = re.compile("http[s]?://(www.)?github.com/([a-z0-9](?:-?[a-z0-9]){0,38}).([\w\.@\:\-~]+)/blob/([\w\.@\:\-~_]+)/(.+)?")
+  OWNER_RE = "(?P<owner>[a-z0-9](?:-?[a-z0-9]){0,38})"
+  REPO_RE = "(?P<repo>[\w\.@\:\-~]+)"
+  BRANCH_RE = "(?P<branch>[\w\.@\:\-~]+)"
+  FILEPATH_RE = "(?P<filepath>.+)"
 
 
   def __init__(self, **options):
-    # TODO: Add support for access token and authenticated API access
-    self._client = HttpClient(self.BASE_URL, logger=LOG)
-    self._root = resource.Resource(self._client)
+    self._github_base_url = options.get('github_remote_url', GITHUB_REMOTE_URL.get()).strip('/')
+    self._api_url = options.get('github_api_url', GITHUB_API_URL.get()).strip('/')
 
-
-  @classmethod
-  def parse_github_url(cls, url):
-    """
-    Given a base URL to a Github repository, return a tuple of the owner, repo, branch, and filepath
-    :param url: base URL to repo (e.g. - https://github.com/cloudera/hue/blob/master/README.rst)
-    :return: tuple of strings (e.g. - ('cloudera', 'hue', 'master', 'README.rst'))
-    """
-    match = cls.REPO_URL_RE.search(url)
-    if match:
-      return match.group(2), match.group(3), match.group(4), match.group(5)
-    else:
-      raise ValueError('Github URL is not formatted correctly: %s' % url)
+    self._client = HttpClient(self._api_url, logger=LOG)
+    self._root = resource.Resource(self._client)
+    self.__headers = {}
+    self.__params = ()
 
 
-  def _get_headers(self):
-    return {}
+  @property
+  def github_url_regex(self):
+    return re.compile('%s/%s/%s/blob/%s/%s' % (self._github_base_url, self.OWNER_RE, self.REPO_RE, self.BRANCH_RE, self.FILEPATH_RE))
 
 
-  def _get_params(self):
-    return ()
+  def _clean_path(self, filepath):
+    cleaned_path = filepath.strip('/')
+    cleaned_path = urllib.unquote(cleaned_path)
+    return cleaned_path
 
 
-  def _get_json(self, response):
+  def _get_json(cls, response):
     if type(response) != dict:
-      # Got 'plain/text' mimetype instead of 'application/json'
       try:
         response = json.loads(response)
-      except ValueError, e:
-        # Got some null bytes in the response
-        LOG.error('%s: %s' % (unicode(e), repr(response)))
-        response = json.loads(response.replace('\x00', ''))
+      except ValueError:
+        raise GithubClientException('Github API did not return JSON response')
     return response
 
 
-  def _clean_path(self, filepath):
-    cleaned_path = filepath.strip('/')
-    cleaned_path = urllib.unquote(cleaned_path)
-    return cleaned_path
+  def parse_github_url(self, url):
+    """
+    Given a base URL to a Github repository, return a tuple of the owner, repo, branch, and filepath
+    :param url: base URL to repo (e.g. - https://github.com/cloudera/hue/blob/master/README.rst)
+    :return: tuple of strings (e.g. - ('cloudera', 'hue', 'master', 'README.rst'))
+    """
+    match = self.github_url_regex.search(url)
+    if match:
+      return match.group('owner'), match.group('repo'), match.group('branch'), match.group('filepath')
+    else:
+      raise ValueError('Github URL is not formatted correctly: %s' % url)
 
 
   def get_file_contents(self, owner, repo, filepath, branch='master'):
@@ -96,27 +96,30 @@ class GithubClient(object):
       blob = self.get_blob(owner, repo, sha)
       content = blob['content'].decode('base64')
       return content
-    except binascii.Error:
-      raise GithubClientException('Failed to decode file contents, check if file content is properly base64-encoded.')
-    except GithubClientException, e:
-      raise e
-    except Exception, e:
-      raise GithubClientException('Failed to get file contents: %s' % str(e))
+    except binascii.Error, e:
+      raise GithubClientException('Failed to decode file contents, check if file content is properly base64-encoded: %s' % e)
+    except KeyError, e:
+      raise GithubClientException('Failed to find expected content object in blob object: %s' % e)
 
 
   def get_sha(self, owner, repo, filepath, branch='master'):
+    """
+    Return the sha for a given filepath by recursively calling Trees API for each level of the path
+    """
     filepath = self._clean_path(filepath)
-    try:
-      sha = branch
-      path_tokens = filepath.split('/')
-      for token in path_tokens:
-        tree = self.get_tree(owner, repo, sha, recursive=False)
-        sha = next(elem['sha'] for elem in tree['tree'] if elem['path'] == token)
-      return sha
-    except StopIteration, e:
-      raise GithubClientException('Could not find sha for: %s/%s/%s/%s' % (owner, repo, branch, filepath))
-    except RestException, e:
-      raise e
+    sha = branch
+    path_tokens = filepath.split('/')
+
+    for token in path_tokens:
+      tree = self.get_tree(owner, repo, sha, recursive=False)
+      for elem in tree['tree']:
+        if elem['path'] == token:
+          sha = elem['sha']
+          break
+      else:
+        raise GithubClientException('Could not find sha for: %s/%s/%s/%s' % (owner, repo, branch, filepath))
+
+    return sha
 
 
   def get_tree(self, owner, repo, sha='master', recursive=True):
@@ -125,15 +128,15 @@ class GithubClient(object):
     https://developer.github.com/v3/git/trees/#get-a-tree
     """
     try:
-      params = self._get_params()
+      params = self.__params
       if recursive:
         params += (
             ('recursive', 1),
         )
-      response = self._root.get('repos/%s/%s/git/trees/%s' % (owner, repo, sha), headers=self._get_headers(), params=params)
+      response = self._root.get('repos/%s/%s/git/trees/%s' % (owner, repo, sha), headers=self.__headers, params=self.__params)
       return self._get_json(response)
     except RestException, e:
-      raise GithubClientException('Could not find Github object, check owner, repo and filepath or permissions: %s' % str(e))
+      raise GithubClientException('Could not find Github object, check owner, repo and filepath or permissions: %s' % e)
 
 
   def get_blob(self, owner, repo, sha):
@@ -142,7 +145,7 @@ class GithubClient(object):
     https://developer.github.com/v3/git/blobs/#get-a-blob
     """
     try:
-      response = self._root.get('repos/%s/%s/git/blobs/%s' % (owner, repo, sha), headers=self._get_headers(), params=self._get_params())
+      response = self._root.get('repos/%s/%s/git/blobs/%s' % (owner, repo, sha), headers=self.__headers, params=self.__params)
       return self._get_json(response)
     except RestException, e:
-      raise GithubClientException('Could not find Github object, check owner, repo and sha or permissions: %s' % str(e))
+      raise GithubClientException('Could not find Github object, check owner, repo and sha or permissions: %s' % e)