Repository: cloudinary/pycloudinary

Issue:
Uploading large files from a stream
Hello,

I've been using cloudinary for quite some time and it's been great. Thanks.

Now, I need to be able to upload large files (generated reports) that might go beyond 100 MB. According to docs, I need to use the [upload_large](http://cloudinary.com/documentation/upload_images#chunked_image_upload). My issue is that I have a stringio object and I couldn't use the upload_large function to upload it to cloudinary (stringio object worked fine with cloudinary.uploader.upload). I'm getting this:
`TypeError: coercing to Unicode: need string or buffer, instance found`
Looking at the [implementation of upload_large](https://github.com/cloudinary/pycloudinary/blob/master/cloudinary/uploader.py#L60), I see that it's invoking `open` on the file argument which means it expects a filename/path of a file on the file system which is inconsistent compared to cloudinary.uploader.upload.

So, I want to ask:
- Is there a way to use a stream (maybe sth other than stringio) with `upload_large` ?
- Also, I want to check if there is a constraint that needs the "large" files to be on the file system.

Of course I have no file size issue as the paid cloudinary plan is customized to support uploading up to 200MB per file.

Thank you.

Discussion:

Patch:
diff --git a/cloudinary/uploader.py b/cloudinary/uploader.py
index 0f58884..66342e5 100644
--- a/cloudinary/uploader.py
+++ b/cloudinary/uploader.py
@@ -1,14 +1,12 @@
 # Copyright Cloudinary
 import json
-import re
 import socket
-from os.path import getsize
 
+import certifi
 from six import string_types
 from urllib3 import PoolManager
 from urllib3.exceptions import HTTPError
 
-import certifi
 import cloudinary
 from cloudinary import utils
 from cloudinary.api import Error
@@ -34,6 +32,8 @@ else:
         ca_certs=certifi.where()
     )
 
+UPLOAD_LARGE_CHUNK_SIZE = 20000000
+
 
 def upload(file, **options):
     params = utils.build_upload_params(**options)
@@ -64,32 +64,44 @@ def upload_large(file, **options):
     if utils.is_remote_url(file):
         return upload(file, **options)
 
-    upload_id = utils.random_public_id()
-    with open(file, 'rb') as file_io:
-        results = None
+    if hasattr(file, 'read') and callable(file.read):
+        file_io = file
+    else:
+        file_io = open(file, 'rb')
+
+    upload_result = None
+
+    with file_io:
+        upload_id = utils.random_public_id()
         current_loc = 0
-        chunk_size = options.get("chunk_size", 20000000)
-        file_size = getsize(file)
+        chunk_size = options.get("chunk_size", UPLOAD_LARGE_CHUNK_SIZE)
+        file_size = utils.file_io_size(file_io)
+
+        file_name = file.name if hasattr(file, 'name') and isinstance(file.name, str) else "stream"
+
         chunk = file_io.read(chunk_size)
+
         while chunk:
-            range = "bytes {0}-{1}/{2}".format(current_loc, current_loc + len(chunk) - 1, file_size)
+            content_range = "bytes {0}-{1}/{2}".format(current_loc, current_loc + len(chunk) - 1, file_size)
             current_loc += len(chunk)
+            http_headers = {"Content-Range": content_range, "X-Unique-Upload-Id": upload_id}
+
+            upload_result = upload_large_part((file_name, chunk), http_headers=http_headers, **options)
+
+            options["public_id"] = upload_result.get("public_id")
 
-            results = upload_large_part(
-                (file, chunk),
-                http_headers={"Content-Range": range,
-                              "X-Unique-Upload-Id": upload_id},
-                **options)
-            options["public_id"] = results.get("public_id")
             chunk = file_io.read(chunk_size)
-        return results
+
+    return upload_result
 
 
 def upload_large_part(file, **options):
     """ Upload large files. """
     params = utils.build_upload_params(**options)
+
     if 'resource_type' not in options:
         options['resource_type'] = "raw"
+
     return call_api("upload", params, file=file, **options)
 
 
diff --git a/cloudinary/utils.py b/cloudinary/utils.py
index a112aa6..9a6fd1f 100644
--- a/cloudinary/utils.py
+++ b/cloudinary/utils.py
@@ -3,6 +3,7 @@ import base64
 import copy
 import hashlib
 import json
+import os
 import random
 import re
 import string
@@ -988,3 +989,19 @@ def __json_serializer(obj):
 def is_remote_url(file):
     """Basic URL scheme check to define if it's remote URL"""
     return isinstance(file, string_types) and re.match(REMOTE_URL_RE, file)
+
+
+def file_io_size(file_io):
+    """
+    Helper function for getting file-like object size(suitable for both files and streams)
+
+    :param file_io: io.IOBase
+
+    :return: size
+    """
+    initial_position = file_io.tell()
+    file_io.seek(0, os.SEEK_END)
+    size = file_io.tell()
+    file_io.seek(initial_position, os.SEEK_SET)
+
+    return size


Test patch:
diff --git a/test/helper_test.py b/test/helper_test.py
index 19c7060..4cb704e 100644
--- a/test/helper_test.py
+++ b/test/helper_test.py
@@ -1,6 +1,5 @@
 import os
 import random
-
 import re
 from datetime import timedelta, tzinfo
 
@@ -8,6 +7,8 @@ import six
 from urllib3 import HTTPResponse
 from urllib3._collections import HTTPHeaderDict
 
+from cloudinary import utils
+
 SUFFIX = os.environ.get('TRAVIS_JOB_ID') or random.randint(10000, 99999)
 
 REMOTE_TEST_IMAGE = "http://cloudinary.com/images/old_logo.png"
@@ -92,3 +93,21 @@ def api_response_mock():
 
 def uploader_response_mock():
     return http_response_mock('{"foo":"bar"}')
+
+
+def populate_large_file(file_io, size, chunk_size=4096):
+    file_io.write(b"BMJ\xB9Y\x00\x00\x00\x00\x00\x8A\x00\x00\x00|\x00\x00\x00x\x05\x00\x00x\x05\x00\x00\x01\
+\x00\x18\x00\x00\x00\x00\x00\xC0\xB8Y\x00a\x0F\x00\x00a\x0F\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xFF\x00\
+\x00\xFF\x00\x00\xFF\x00\x00\x00\x00\x00\x00\xFFBGRs\x00\x00\x00\x00\x00\x00\x00\x00T\xB8\x1E\xFC\x00\x00\x00\x00\
+\x00\x00\x00\x00fff\xFC\x00\x00\x00\x00\x00\x00\x00\x00\xC4\xF5(\xFF\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\
+\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
+
+    remaining_size = size - utils.file_io_size(file_io)
+
+    while remaining_size > 0:
+        curr_chunk_size = min(remaining_size, chunk_size)
+        file_io.write(b"\xFF" * curr_chunk_size)
+        remaining_size -= chunk_size
+
+    file_io.flush()
+    file_io.seek(0)
diff --git a/test/test_uploader.py b/test/test_uploader.py
index a897d9a..e03b5b9 100644
--- a/test/test_uploader.py
+++ b/test/test_uploader.py
@@ -7,13 +7,13 @@ from datetime import datetime
 
 import six
 from mock import patch
-import cloudinary
-from cloudinary import api, uploader, utils
-
 from urllib3 import disable_warnings
 from urllib3.util import parse_url
+
+import cloudinary
+from cloudinary import api, uploader, utils
 from test.helper_test import uploader_response_mock, SUFFIX, TEST_IMAGE, get_params, TEST_ICON, TEST_DOC, \
-    REMOTE_TEST_IMAGE, UTC
+    REMOTE_TEST_IMAGE, UTC, populate_large_file
 
 MOCK_RESPONSE = uploader_response_mock()
 
@@ -37,6 +37,11 @@ TEST_ID2 = "uploader_test_{}2".format(SUFFIX)
 
 disable_warnings()
 
+LARGE_FILE_SIZE = 5880138
+LARGE_CHUNK_SIZE = 5243000
+LARGE_FILE_WIDTH = 1400
+LARGE_FILE_HEIGHT = 1400
+
 
 class UploaderTest(unittest.TestCase):
 
@@ -72,11 +77,12 @@ class UploaderTest(unittest.TestCase):
     @unittest.skipUnless(cloudinary.config().api_secret, "requires api_key/api_secret")
     def test_upload_file_io_without_filename(self):
         """should successfully upload FileIO file """
-        with io.BytesIO() as temp_file, \
-                open(TEST_IMAGE, 'rb') as input_file:
+        with io.BytesIO() as temp_file, open(TEST_IMAGE, 'rb') as input_file:
             temp_file.write(input_file.read())
             temp_file.seek(0)
+
             result = uploader.upload(temp_file, tags=[UNIQUE_TAG])
+
         self.assertEqual(result["width"], TEST_IMAGE_WIDTH)
         self.assertEqual(result["height"], TEST_IMAGE_HEIGHT)
         self.assertEqual('stream', result["original_filename"])
@@ -325,44 +331,57 @@ P9/AFGGFyjOXZtQAAAAAElFTkSuQmCC\
     @unittest.skipUnless(cloudinary.config().api_secret, "requires api_key/api_secret")
     def test_upload_large(self):
         """ should support uploading large files """
-        temp_file = tempfile.NamedTemporaryFile()
-        temp_file_name = temp_file.name
-        temp_file.write(b"BMJ\xB9Y\x00\x00\x00\x00\x00\x8A\x00\x00\x00|\x00\x00\x00x\x05\x00\x00x\x05\x00\x00\x01\
-\x00\x18\x00\x00\x00\x00\x00\xC0\xB8Y\x00a\x0F\x00\x00a\x0F\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xFF\x00\
-\x00\xFF\x00\x00\xFF\x00\x00\x00\x00\x00\x00\xFFBGRs\x00\x00\x00\x00\x00\x00\x00\x00T\xB8\x1E\xFC\x00\x00\x00\x00\
-\x00\x00\x00\x00fff\xFC\x00\x00\x00\x00\x00\x00\x00\x00\xC4\xF5(\xFF\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\
-\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
-        for i in range(0, 588000):
-            temp_file.write(b"\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF")
-
-        temp_file.flush()
-        self.assertEqual(5880138, os.path.getsize(temp_file_name))
-
-        resource = uploader.upload_large(temp_file_name, chunk_size=5243000, tags=["upload_large_tag", UNIQUE_TAG])
-        self.assertEqual(resource["tags"], ["upload_large_tag", UNIQUE_TAG])
-        self.assertEqual(resource["resource_type"], "raw")
-
-        resource2 = uploader.upload_large(temp_file_name, chunk_size=5243000, tags=["upload_large_tag", UNIQUE_TAG],
-                                          resource_type="image")
-        self.assertEqual(resource2["tags"], ["upload_large_tag", UNIQUE_TAG])
-        self.assertEqual(resource2["resource_type"], "image")
-        self.assertEqual(resource2["width"], 1400)
-        self.assertEqual(resource2["height"], 1400)
-
-        resource3 = uploader.upload_large(temp_file_name, chunk_size=5880138, tags=["upload_large_tag", UNIQUE_TAG])
-        self.assertEqual(resource3["tags"], ["upload_large_tag", UNIQUE_TAG])
-        self.assertEqual(resource3["resource_type"], "raw")
-
-        # should allow fallback of upload large with remote url to regular upload
+        with tempfile.NamedTemporaryFile() as temp_file:
+            populate_large_file(temp_file, LARGE_FILE_SIZE)
+            temp_file_name = temp_file.name
+
+            self.assertEqual(LARGE_FILE_SIZE, os.path.getsize(temp_file_name))
+
+            resource = uploader.upload_large(temp_file_name, chunk_size=LARGE_CHUNK_SIZE,
+                                             tags=["upload_large_tag", UNIQUE_TAG])
+
+            self.assertEqual(resource["tags"], ["upload_large_tag", UNIQUE_TAG])
+            self.assertEqual(resource["resource_type"], "raw")
+
+            resource2 = uploader.upload_large(temp_file_name, chunk_size=LARGE_CHUNK_SIZE,
+                                              tags=["upload_large_tag", UNIQUE_TAG], resource_type="image")
+
+            self.assertEqual(resource2["tags"], ["upload_large_tag", UNIQUE_TAG])
+            self.assertEqual(resource2["resource_type"], "image")
+            self.assertEqual(resource2["width"], LARGE_FILE_WIDTH)
+            self.assertEqual(resource2["height"], LARGE_FILE_HEIGHT)
+
+            resource3 = uploader.upload_large(temp_file_name, chunk_size=LARGE_FILE_SIZE,
+                                              tags=["upload_large_tag", UNIQUE_TAG])
+
+            self.assertEqual(resource3["tags"], ["upload_large_tag", UNIQUE_TAG])
+            self.assertEqual(resource3["resource_type"], "raw")
+
+    @unittest.skipUnless(cloudinary.config().api_secret, "requires api_key/api_secret")
+    def test_upload_large_url(self):
+        """Should allow fallback of upload large with remote url to regular upload"""
         resource4 = uploader.upload_large(REMOTE_TEST_IMAGE, tags=[UNIQUE_TAG])
+
         self.assertEqual(resource4["width"], TEST_IMAGE_WIDTH)
         self.assertEqual(resource4["height"], TEST_IMAGE_HEIGHT)
-        expected_signature = utils.api_sign_request(dict(public_id=resource4["public_id"],
-                                                         version=resource4["version"]),
-                                                    cloudinary.config().api_secret)
+
+        expected_signature = utils.api_sign_request(
+            dict(public_id=resource4["public_id"], version=resource4["version"]), cloudinary.config().api_secret)
+
         self.assertEqual(resource4["signature"], expected_signature)
 
-        temp_file.close()
+    @unittest.skipUnless(cloudinary.config().api_secret, "requires api_key/api_secret")
+    def test_upload_large_file_io(self):
+        """Should support uploading large streams"""
+        with io.BytesIO() as temp_file:
+            populate_large_file(temp_file, LARGE_FILE_SIZE)
+            resource = uploader.upload_large(temp_file, chunk_size=LARGE_CHUNK_SIZE,
+                                             tags=["upload_large_tag", UNIQUE_TAG], resource_type="image")
+
+            self.assertEqual(resource["tags"], ["upload_large_tag", UNIQUE_TAG])
+            self.assertEqual(resource["resource_type"], "image")
+            self.assertEqual(resource["width"], LARGE_FILE_WIDTH)
+            self.assertEqual(resource["height"], LARGE_FILE_HEIGHT)
 
     @unittest.skipUnless(cloudinary.config().api_secret, "requires api_key/api_secret")
     def test_upload_preset(self):
diff --git a/test/test_utils.py b/test/test_utils.py
index ccb82fc..d5537f0 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -1,10 +1,12 @@
 # -*- coding: utf-8 -*-
-
+import io
 import re
+import tempfile
 import unittest
 from collections import OrderedDict
 from datetime import datetime, date
 from fractions import Fraction
+from os.path import getsize
 
 import six
 from mock import patch
@@ -13,7 +15,6 @@ import cloudinary.utils
 from cloudinary.utils import build_list_of_dicts, json_encode, encode_unicode_url
 from test.helper_test import TEST_IMAGE, REMOTE_TEST_IMAGE
 
-
 DEFAULT_ROOT_PATH = 'http://res.cloudinary.com/test123/'
 DEFAULT_UPLOAD_PATH = 'http://res.cloudinary.com/test123/image/upload/'
 VIDEO_UPLOAD_PATH = 'http://res.cloudinary.com/test123/video/upload/'
@@ -824,6 +825,32 @@ class TestUtils(unittest.TestCase):
         self.assertFalse(cloudinary.utils.is_remote_url(TEST_IMAGE))
         self.assertTrue(cloudinary.utils.is_remote_url(REMOTE_TEST_IMAGE))
 
+    def test_file_io_size(self):
+        """Should return correct file size"""
+        test_data = b"Test data"
+        test_data_len = len(test_data)
+
+        with tempfile.NamedTemporaryFile() as temp_file:
+            temp_file.write(test_data)
+
+            actual_size = cloudinary.utils.file_io_size(temp_file)
+
+            filesystem_size = getsize(temp_file.name)
+
+            self.assertEqual(test_data_len, filesystem_size)
+            self.assertEqual(test_data_len, actual_size)
+
+        with io.BytesIO() as temp_stream:
+            temp_stream.write(test_data)
+
+            actual_size = cloudinary.utils.file_io_size(temp_stream)
+
+            self.assertEqual(test_data_len, actual_size)
+
+        with tempfile.NamedTemporaryFile() as empty_file:
+            actual_size = cloudinary.utils.file_io_size(empty_file)
+            self.assertEqual(0, actual_size)
+
 
 if __name__ == '__main__':
     unittest.main()