Flesh out API for pull file download

sigmavirus24 · sigmavirus24 · commit 5695b1fadcc4 · 2015-02-01T23:23:09.000-06:00
diff --git a/LATEST_VERSION_NOTES.rst b/LATEST_VERSION_NOTES.rst
@@ -1,5 +1,22 @@
 .. vim: set tw=100
 
+1.0.0a2: 2015-02-01
+~~~~~~~~~~~~~~~~~~~
+
+Breaking Changes (since 1.0.0a1)
+````````````````````````````````
+
+- When you download a release asset, instead of returning ``True`` or
+  ``False``, it will return the name of the file in which it saved the asset.
+
+Features Added (since 1.0.0a1)
+``````````````````````````````
+
+- You can now download a file in a pull request to a file on disk.
+
+- You can retrieve the contents of the file in a pull request as bytes.
+
+
 1.0.0a1: 2014-12-07
 ~~~~~~~~~~~~~~~~~~~
 
diff --git a/github3/pulls.py b/github3/pulls.py
@@ -10,15 +10,17 @@
 
 from re import match
 from json import dumps
+
+from . import models
+from . import utils
 from .repos.commit import RepoCommit
-from .models import GitHubObject, GitHubCore, BaseComment
 from .users import User
 from .decorators import requires_auth
 from .issues.comment import IssueComment
 from uritemplate import URITemplate
 
 
-class PullDestination(GitHubCore):
+class PullDestination(models.GitHubCore):
 
     """The :class:`PullDestination <PullDestination>` object.
 
@@ -50,7 +52,7 @@ def _repr(self):
         return '<{0} [{1}]>'.format(self.direction, self.label)
 
 
-class PullFile(GitHubCore):
+class PullFile(models.GitHubCore):
 
     """The :class:`PullFile <PullFile>` object.
 
@@ -80,16 +82,36 @@ def _update_attributes(self, pfile):
     def _repr(self):
         return '<Pull Request File [{0}]>'.format(self.filename)
 
-    def contents(self, stream=False):
-        """Return the contents of the raw file.
+    def download(self, path=None):
+        """Download the contents for this file to disk.
+
+        :param path: (optional), path where the file should be saved
+            to, default is the filename provided in the headers and will be
+            written in the current directory.
+            it can take a file-like object as well
+        :type path: str, file-like object
+        :returns: bool -- True if successful, False otherwise
+        """
+        headers = {'Accept': 'application/octet-stream'}
+        resp = self._get(self.raw_url, stream=True, headers=headers)
+        if self._boolean(resp, 200, 404):
+            return utils.stream_response_to_file(resp, path)
+        return None
+
+    def contents(self):
+        """Return the contents of the file as bytes.
 
         :param stream: When true, the resulting object can be iterated over via
             ``iter_contents``.
         """
-        return self.session.get(self.raw_url, stream=stream)
+        headers = {'Accept': 'application/octect-stream'}
+        resp = self._get(self.raw_url, headers=headers)
+        if self._boolean(resp, 200, 404):
+            return resp.content
+        return b''
 
 
-class PullRequest(GitHubCore):
+class PullRequest(models.GitHubCore):
 
     """The :class:`PullRequest <PullRequest>` object.
 
@@ -349,7 +371,7 @@ def update(self, title=None, body=None, state=None):
         return False
 
 
-class ReviewComment(BaseComment):
+class ReviewComment(models.BaseComment):
 
     """The :class:`ReviewComment <ReviewComment>` object.
 
diff --git a/github3/repos/release.py b/github3/repos/release.py
@@ -5,7 +5,7 @@
 from ..decorators import requires_auth
 from ..exceptions import error_for
 from ..models import GitHubCore
-from ..utils import stream_response_to_file
+from .. import utils
 from uritemplate import URITemplate
 
 
@@ -188,7 +188,8 @@ def download(self, path=''):
             written in the current directory.
             it can take a file-like object as well
         :type path: str, file
-        :returns: bool -- True if successful, False otherwise
+        :returns: name of the file, if successful otherwise ``None``
+        :rtype: str
         """
         headers = {
             'Accept': 'application/octet-stream'
@@ -207,9 +208,8 @@ def download(self, path=''):
                                  headers=headers)
 
         if self._boolean(resp, 200, 404):
-            stream_response_to_file(resp, path)
-            return True
-        return False
+            return utils.stream_response_to_file(resp, path)
+        return None
 
     def edit(self, name, label=None):
         """Edit this asset.
diff --git a/github3/utils.py b/github3/utils.py
@@ -1,10 +1,11 @@
 # -*- coding: utf-8 -*-
 """A collection of useful utilities."""
-from collections import Callable
-from datetime import datetime, timedelta, tzinfo
-from requests.compat import basestring
+import collections
+import datetime
 import re
 
+from requests import compat
+
 # with thanks to https://code.google.com/p/jquery-localtime/issues/detail?id=4
 ISO_8601 = re.compile("^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[0-1]|0"
                       "[1-9]|[1-2][0-9])(T(2[0-3]|[0-1][0-9]):([0-5][0-9]):([0"
@@ -32,10 +33,10 @@ def timestamp_parameter(timestamp, allow_none=True):
             return None
         raise ValueError("Timestamp value cannot be None")
 
-    if isinstance(timestamp, datetime):
+    if isinstance(timestamp, datetime.datetime):
         return timestamp.isoformat() + 'Z'
 
-    if isinstance(timestamp, basestring):
+    if isinstance(timestamp, compat.basestring):
         if not ISO_8601.match(timestamp):
             raise ValueError(("Invalid timestamp: %s is not a valid ISO-8601"
                               " formatted date") % timestamp)
@@ -44,12 +45,12 @@ def timestamp_parameter(timestamp, allow_none=True):
     raise ValueError("Cannot accept type %s for timestamp" % type(timestamp))
 
 
-class UTC(tzinfo):
+class UTC(datetime.tzinfo):
 
     """Yet another UTC reimplementation, to avoid a dependency on pytz or
     dateutil."""
 
-    ZERO = timedelta(0)
+    ZERO = datetime.timedelta(0)
 
     def __repr__(self):
         return 'UTC()'
@@ -73,22 +74,30 @@ def stream_response_to_file(response, path=None):
     :param response: A Response object from requests
     :type response: requests.models.Response
     :param str path: The full path and file name used to save the response
+    :return: path to the file
+    :rtype: str
     """
     pre_opened = False
     fd = None
+    filename = None
     if path:
-        if isinstance(getattr(path, 'write', None), Callable):
+        if isinstance(getattr(path, 'write', None), collections.Callable):
             pre_opened = True
             fd = path
+            filename = getattr(fd, 'name', None)
         else:
             fd = open(path, 'wb')
+            filename = path
     else:
         header = response.headers['content-disposition']
         i = header.find('filename=') + len('filename=')
-        fd = open(header[i:], 'wb')
+        filename = header[i:]
+        fd = open(filename, 'wb')
 
     for chunk in response.iter_content(chunk_size=512):
         fd.write(chunk)
 
     if not pre_opened:
         fd.close()
+
+    return filename
