Documents List API (DocsService) download methods never raise errors

[GoogleCodeExporter]

DocsService.DownloadDocument .DownloadSpreadsheet and .DownloadPresentation
never raise errors, even if they completely fail to download a document (eg
if access is denied).

Steps to reproduce - using the API, and authenticating with ClientLogin,
get a document entry. Attempt to download the document using one of the
three methods, without authenticating with OAuth or AuthSub. The download
will fail silently, and will even write to the file as it is supposed to,
but it will just write whatever the http response is (which is not the
document).

Original issue reported on code.google.com by andrew...@gmail.com on 6 Apr 2009 at 12:33

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Fixes issue, simply by raising an error if the the status of the server's 
response is
not 200.

Original comment by andrew...@gmail.com on 9 Apr 2009 at 8:38

Attachments:

• issue240_diff

[GoogleCodeExporter]

Original comment by ericbide...@gmail.com on 9 Apr 2009 at 2:56

[GoogleCodeExporter]

LGTM.  +1

I've added other small changes.  The most significant is the adding the 
appropriate 
DocumentListEntryFromString converter to several methods the use of 
DocumentListEntry 
objects instead of GDataEntry.

Original comment by ericbide...@gmail.com on 9 Apr 2009 at 5:54

Attachments:

• doclist.patch

[GoogleCodeExporter]

LGTM

I noticed one place where I think a small improvement could be made. In the 
below
section of code, the entire file contents are read at once and written out. For 
large
files it might be better to read and write a chunk at a time. Also this method 
takes
a file name and calls open itself, but it might also be useful to be able pass 
in a
file handle (already opened, just call write) in case the user doesn't want to 
write
the file to disk. For example, you could pass in a StringIO object to hold the
downloaded file in memory. (On App Engine, you can't write to the local disk.)

     """Downloads a file from the Document List.

     Args:
-      uri: string The full Export URL to download the document from.
-      file_path: string The full path to save the file to.  The export
-          format is inferred from the the file extension.
+      uri: string The full Export URL to download the file from.
+      file_path: string The full path to save the file to.
     """
-    media_source = self.GetMedia(uri)
+    server_response = self.request('GET', uri)
+    response_body = server_response.read()
+    if server_response.status != 200:
+      raise gdata.service.RequestError, {'status': server_response.status,
+                                         'reason': server_response.reason,
+                                         'body': response_body}
     f = open(file_path, 'wb')
-    f.write(media_source.file_handle.read())
+    f.write(response_body)
     f.flush()
     f.close()

Original comment by jscud.w...@gmail.com on 9 Apr 2009 at 6:03

[GoogleCodeExporter]

I will commit these changes as is and break out that 
good idea/feature request into a sep. issue.

Thx!

Original comment by ericbide...@gmail.com on 9 Apr 2009 at 9:43

[GoogleCodeExporter]

Original comment by ericbide...@gmail.com on 9 Apr 2009 at 9:45

• Changed state: Fixed