From f57ec01ef40f2c9bc2901c9b45638a4728369fab Mon Sep 17 00:00:00 2001 From: Adam Goforth Date: Tue, 8 Jan 2013 22:06:28 -0500 Subject: [PATCH 1/6] Add file_path option to allow downloading directly to disk add new exception ShotgunDownloadFileError Add support for passing in dict to download_attachment() --- shotgun_api3/shotgun.py | 96 +++++++++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 32 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 86a90debc..3a92e4452 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -47,6 +47,7 @@ import urllib import urllib2 # used for image upload import urlparse +import shutil # used for attachment download # use relative import for versions >=2.5 and package import for python versions <2.5 if (sys.version_info[0] > 2) or (sys.version_info[0] == 2 and sys.version_info[1] >= 5): @@ -996,7 +997,7 @@ def share_thumbnail(self, entities, thumbnail_path=None, source_entity=None, try: resp = opener.open(url, params) result = resp.read() - # response heaers are in str(resp.info()).splitlines() + # response headers are in str(resp.info()).splitlines() except urllib2.HTTPError, e: if e.code == 500: raise ShotgunError("Server encountered an internal error. " @@ -1111,14 +1112,64 @@ def upload(self, entity_type, entity_id, path, field_name=None, attachment_id = int(str(result).split(":")[1].split("\n")[0]) return attachment_id - def download_attachment(self, attachment_id): - """Gets the returns binary content of the specified attachment. + def download_attachment(self, attachment_id, file_path=None): + """Download the file associated with the Shotgun Attachment - :param attachment_id: id of the attachment to get. + :param attachment_id: (int) id of the Shotgun Attachment entity to + download. - :returns: binary data as a string + :param file_path: (str) Optional. If provided, write the data directly + to local disk using the file_path. This avoids loading all of the data + in memory and saves the file locally which is probably what is desired + anyway. + + :returns: (str) If file_path is None, returns data of the Attachment + file as a string. If file_path is provided, returns file_path. + """ + if file_path: + fp = open(file_path, 'wb') + self.set_up_auth_cookie() + url = self.get_attachment_download_url(attachment_id) + + try: + request = urllib2.Request(url) + request.add_header('user-agent', "; ".join(self._user_agents)) + req = urllib2.urlopen(request) + if file_path: + shutil.copyfileobj(req, fp) + else: + attachment = req.read() + + # raised when Attachment id doesn't exist or is a local file + # todo: what happens when s3 url is invalid? + except urllib2.URLError, e: + if file_path: + fp.close() + err = "Failed to open %s" % url + err += "\n%s" % e + if hasattr(e, 'code'): + if e.code == 400: + err += "\nAttachment with id %d may not exist or is a "\ + "local file." % attachment_id + else: + err += "\nHTTP Status: %s" % e.code + # what cases set this and do we need to show it? + if hasattr(e, 'reason'): + err += "\n%s" % e.reason + raise ShotgunError(err) + + # NOTE: On older (< v5.1.0)Shotgun versions, non-downloadable files + # don't raise exceptions, they cause a server error which returns a + # 200 with the page content. + else: + if file_path: + return file_path + else: + return attachment + + def set_up_auth_cookie(self): + """Sets up urllib2 with a cookie for authentication. """ - # Cookie for auth sid = self._get_session_token() cj = cookielib.LWPCookieJar() c = cookielib.Cookie('0', '_session_id', sid, None, False, @@ -1129,35 +1180,16 @@ def download_attachment(self, attachment_id): opener = self._build_opener(cookie_handler) urllib2.install_opener(opener) + def get_attachment_download_url(self, attachment_id): + """Given an Attachment id, returns the URL for downloading that + Attachment. + + returns: (str) the download URL for Attachment + """ url = urlparse.urlunparse((self.config.scheme, self.config.server, "/file_serve/attachment/%s" % urllib.quote(str(attachment_id)), None, None, None)) - - try: - request = urllib2.Request(url) - request.add_header('User-agent', - "Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; "\ - "rv:1.9.0.7) Gecko/2009021906 Firefox/3.0.7") - attachment = urllib2.urlopen(request).read() - - except IOError, e: - err = "Failed to open %s" % url - if hasattr(e, 'code'): - err += "\nWe failed with error code - %s." % e.code - elif hasattr(e, 'reason'): - err += "\nThe error object has the following 'reason' "\ - "attribute : %s" % e.reason - err += "\nThis usually means the server doesn't exist, is "\ - "down, or we don't have an internet connection." - raise ShotgunError(err) - else: - if attachment.lstrip().startswith(' Date: Fri, 21 Jun 2013 16:48:39 -0500 Subject: [PATCH 2/6] add new exception ShotgunDownloadFileError --- shotgun_api3/__init__.py | 4 ++-- shotgun_api3/shotgun.py | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/shotgun_api3/__init__.py b/shotgun_api3/__init__.py index b79881707..ef9d6b6b2 100644 --- a/shotgun_api3/__init__.py +++ b/shotgun_api3/__init__.py @@ -1,4 +1,4 @@ -from shotgun import (Shotgun, ShotgunError, Fault, ProtocolError, ResponseError, - Error, __version__) +from shotgun import (Shotgun, ShotgunError, ShotgunFileDownloadError, Fault, + ProtocolError, ResponseError, Error, __version__) from shotgun import SG_TIMEZONE as sg_timezone diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 3a92e4452..0872c884e 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -79,6 +79,10 @@ class ShotgunError(Exception): """Base for all Shotgun API Errors""" pass +class ShotgunFileDownloadError(ShotgunError): + """Exception for file download-related errors""" + pass + class Fault(ShotgunError): """Exception when server side exception detected.""" pass From a4ae8de4cd2f01479499954b10ba3d2ef616fd9e Mon Sep 17 00:00:00 2001 From: KP Date: Fri, 21 Jun 2013 16:51:14 -0500 Subject: [PATCH 3/6] Add support for passing in dict to download_attachment() backwards compatible so still supports Attachment id. cleaned up error handling --- shotgun_api3/shotgun.py | 121 +++++++++++++++++++++++++++++----------- 1 file changed, 87 insertions(+), 34 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 0872c884e..9b4df1c50 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1116,25 +1116,59 @@ def upload(self, entity_type, entity_id, path, field_name=None, attachment_id = int(str(result).split(":")[1].split("\n")[0]) return attachment_id - def download_attachment(self, attachment_id, file_path=None): - """Download the file associated with the Shotgun Attachment - - :param attachment_id: (int) id of the Shotgun Attachment entity to - download. + def download_attachment(self, attachment=False, file_path=None, + attachment_id=None): + """Downloads the file associated with a Shotgun Attachment. + + NOTE: On older (< v5.1.0) Shotgun versions, non-downloadable files + on Shotgun don't raise exceptions, they cause a server error which + returns a 200 with the page content. + + :param attachment: (mixed) Usually a dict representing an Attachment. + The dict should have a 'url' key that specifies the download url. + Optionally, the dict can be a standard entity hash format with 'id' and + 'type' keys as long as 'type'=='Attachment'. This is only supported for + backwards compatibility (#22150). + If an int value is passed in, the Attachment with the matching id will + be downloaded from the Shotgun server. :param file_path: (str) Optional. If provided, write the data directly to local disk using the file_path. This avoids loading all of the data in memory and saves the file locally which is probably what is desired anyway. + :param attachment_id: (int) Optional. Deprecated in favor of passing in + Attachment hash to attachment param. This attachment_id exists only for + backwards compatibility for scripts specifying the parameter with + keywords. + :returns: (str) If file_path is None, returns data of the Attachment file as a string. If file_path is provided, returns file_path. """ + # backwards compatibility when passed via keyword argument + if attachment is False: + if type(attachment_id) == int: + attachment = attachment_id + else: + raise TypeError("Missing parameter 'attachment'. Expected a "\ + "dict, int, NoneType value or"\ + "an int for parameter attachment_id") + # write to disk if file_path: - fp = open(file_path, 'wb') - self.set_up_auth_cookie() - url = self.get_attachment_download_url(attachment_id) - + try: + fp = open(file_path, 'wb') + except IOError, e: + raise IOError("Unable to write Attachment to disk using "\ + "file_path. %s" % e) + + url = self.get_attachment_download_url(attachment) + if url is None: + return None + + # We only need to set the auth cookie for downloads from Shotgun server + if self.config.server in url: + self.set_up_auth_cookie() + try: request = urllib2.Request(url) request.add_header('user-agent', "; ".join(self._user_agents)) @@ -1143,28 +1177,16 @@ def download_attachment(self, attachment_id, file_path=None): shutil.copyfileobj(req, fp) else: attachment = req.read() - - # raised when Attachment id doesn't exist or is a local file - # todo: what happens when s3 url is invalid? + # 400 [sg] Attachment id doesn't exist or is a local file + # 403 [s3] link is invalid except urllib2.URLError, e: if file_path: fp.close() - err = "Failed to open %s" % url - err += "\n%s" % e + err = "Failed to open %s\n%s" % (url, e) if hasattr(e, 'code'): if e.code == 400: - err += "\nAttachment with id %d may not exist or is a "\ - "local file." % attachment_id - else: - err += "\nHTTP Status: %s" % e.code - # what cases set this and do we need to show it? - if hasattr(e, 'reason'): - err += "\n%s" % e.reason - raise ShotgunError(err) - - # NOTE: On older (< v5.1.0)Shotgun versions, non-downloadable files - # don't raise exceptions, they cause a server error which returns a - # 200 with the page content. + err += "\nAttachment may not exist or is a local file?" + raise ShotgunFileDownloadError(err) else: if file_path: return file_path @@ -1172,7 +1194,8 @@ def download_attachment(self, attachment_id, file_path=None): return attachment def set_up_auth_cookie(self): - """Sets up urllib2 with a cookie for authentication. + """Sets up urllib2 with a cookie for authentication on the Shotgun + instance. """ sid = self._get_session_token() cj = cookielib.LWPCookieJar() @@ -1184,15 +1207,45 @@ def set_up_auth_cookie(self): opener = self._build_opener(cookie_handler) urllib2.install_opener(opener) - def get_attachment_download_url(self, attachment_id): - """Given an Attachment id, returns the URL for downloading that - Attachment. + def get_attachment_download_url(self, attachment): + """Returns the URL for downloading provided Attachment. + + :param attachment: (mixed) If type is an int, construct url to download + Attachment with id from Shotgun. + If type is a dict, and a url key is present, use that url. + If type is a dict, and url key is not present, check if we have + an id and type keys and the type is 'Attachment' in which case we + construct url to download Attachment with id from Shotgun as if just + the id has been passed in. - returns: (str) the download URL for Attachment + :todo: Support for a standard entity hash should be removed: #22150 + + :returns: (str) the download URL for the Attachment or None if None was + passed to attachment param. This avoids raising an error when results + from a find() are passed off to a download_attachment() call. """ - url = urlparse.urlunparse((self.config.scheme, self.config.server, - "/file_serve/attachment/%s" % urllib.quote(str(attachment_id)), - None, None, None)) + attachment_id = None + if isinstance(attachment, int): + attachment_id = attachment + elif isinstance(attachment, dict): + try: + url = attachment['url'] + except KeyError: + if ('id' in attachment and 'type' in attachment and + attachment['type'] == 'Attachment'): + attachment_id = attachment['id'] + else: + raise ValueError("Missing 'url' key in Attachment dict") + elif attachment is None: + url = None + else: + raise TypeError("Unable to determine download url. Expected "\ + "dict, int, or NoneType. Instead got %s" % type(attachment)) + + if attachment_id: + url = urlparse.urlunparse((self.config.scheme, self.config.server, + "/file_serve/attachment/%s" % urllib.quote(str(attachment_id)), + None, None, None)) return url def authenticate_human_user(self, user_login, user_password): From 45aa081c51089c896bc536ee7b6f117d59655488 Mon Sep 17 00:00:00 2001 From: KP Date: Fri, 21 Jun 2013 16:51:37 -0500 Subject: [PATCH 4/6] more tests for download_attachment() --- tests/test_api.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 6af0df03c..a8ba111c3 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -127,7 +127,7 @@ def test_get_session_token(self): self.assertTrue(rv) def test_upload_download(self): - """Upload and download an attachment """ + """Upload and download an attachment tests""" # upload / download only works against a live server because it does # not use the standard http interface if 'localhost' in self.server_url: @@ -143,13 +143,67 @@ def test_upload_download(self): self.ticket['id'], path, 'attachments', tag_list="monkeys, everywhere, send, help") + # test download with attachment_id attach_file = self.sg.download_attachment(attach_id) self.assertTrue(attach_file is not None) self.assertEqual(size, len(attach_file)) + orig_file = open(path, "rb").read() + self.assertEqual(orig_file, attach_file) + # test download with attachment_id as keyword + attach_file = self.sg.download_attachment(attachment_id=attach_id) + self.assertTrue(attach_file is not None) + self.assertEqual(size, len(attach_file)) orig_file = open(path, "rb").read() self.assertEqual(orig_file, attach_file) + # test download with attachment_id (write to disk) + file_path = "%s/sg_logo_download.jpg" % os.path.dirname(os.path.realpath(__file__)) + result = self.sg.download_attachment(attach_id, file_path=file_path) + self.assertEqual(result, file_path) + fp = open(file_path) + attach_file = fp.read() + fp.close() + self.assertEqual(size, len(attach_file)) + self.assertEqual(orig_file, attach_file) + + # test download with attachment hash + ticket = self.sg.find_one('Ticket', [['id', 'is', self.ticket['id']]], + ['attachments']) + attach_file = self.sg.download_attachment(ticket['attachments'][0]) + self.assertTrue(attach_file is not None) + self.assertEqual(size, len(attach_file)) + self.assertEqual(orig_file, attach_file) + + # test download with attachment hash (write to disk) + result = self.sg.download_attachment(ticket['attachments'][0], + file_path=file_path) + self.assertEqual(result, file_path) + fp = open(file_path) + attach_file = fp.read() + fp.close() + self.assertTrue(attach_file is not None) + self.assertEqual(size, len(attach_file)) + self.assertEqual(orig_file, attach_file) + + # test invalid requests + INVALID_S3_URL = "https://sg-media-usor-01.s3.amazonaws.com/ada3de3ee3873875e1dd44f2eb0882c75ae36a4a/cd31346421dbeef781e0e480f259a3d36652d7f2/IMG_0465.MOV?AWSAccessKeyId=AKIAIQGOBSVN3FSQ5QFA&Expires=1371789959&Signature=SLbzv7DuVlZ8XAoOSQQAiGpF3u8%3D" + self.assertRaises(shotgun_api3.ShotgunFileDownloadError, + self.sg.download_attachment, + {"url": INVALID_S3_URL}) + INVALID_ATTACHMENT_ID = 99999999 + self.assertRaises(shotgun_api3.ShotgunFileDownloadError, + self.sg.download_attachment, + INVALID_ATTACHMENT_ID) + self.assertRaises(TypeError, self.sg.download_attachment, + "/path/to/some/file.jpg") + self.assertRaises(ValueError, self.sg.download_attachment, + {"id":123, "type":"Shot"}) + self.assertRaises(TypeError, self.sg.download_attachment) + + # cleanup + os.remove(file_path) + def test_upload_thumbnail_in_create(self): """Upload a thumbnail via the create method""" this_dir, _ = os.path.split(__file__) From 8bcadddbee231f337bed41fff6877092d14e477d Mon Sep 17 00:00:00 2001 From: Brandon Ashworth Date: Tue, 25 Jun 2013 10:44:40 -0700 Subject: [PATCH 5/6] Updated error messages for 403 errors when using download_attachment to access files in S3. --- shotgun_api3/shotgun.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 9b4df1c50..c1b737bbd 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1186,6 +1186,17 @@ def download_attachment(self, attachment=False, file_path=None, if hasattr(e, 'code'): if e.code == 400: err += "\nAttachment may not exist or is a local file?" + elif e.code == 403: + # Only parse the body if it is an Amazon S3 url. + if url.find('s3.amazonaws.com') != -1: + body = e.readlines() + if body and len(body) > 1: + xml = body[1] + # Once python 2.4 support is not needed we can think about using elementtree. + # The doc is pretty small so this shouldn't be an issue. + match = re.search('(.*)', xml) + if match: + err += ' - %s' % (match.group(1)) raise ShotgunFileDownloadError(err) else: if file_path: From 3a55a76f3d675548bd52ec87b758dd91c4ba7d38 Mon Sep 17 00:00:00 2001 From: Brandon Ashworth Date: Tue, 25 Jun 2013 11:29:41 -0700 Subject: [PATCH 6/6] A little bit of clean up for the 403 message. --- shotgun_api3/shotgun.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index c1b737bbd..764186825 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1188,10 +1188,11 @@ def download_attachment(self, attachment=False, file_path=None, err += "\nAttachment may not exist or is a local file?" elif e.code == 403: # Only parse the body if it is an Amazon S3 url. - if url.find('s3.amazonaws.com') != -1: + if url.find('s3.amazonaws.com') != -1 \ + and e.headers['content-type'] == 'application/xml': body = e.readlines() - if body and len(body) > 1: - xml = body[1] + if body: + xml = ''.join(body) # Once python 2.4 support is not needed we can think about using elementtree. # The doc is pretty small so this shouldn't be an issue. match = re.search('(.*)', xml)