From eaf3a239e48a503e4e5e4e81d3daf4a0f1f97efe Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 12:52:59 +0300 Subject: [PATCH 01/22] using botocore for s3 request signing: proof of concept --- scrapy/core/downloader/handlers/s3.py | 59 ++++++++++++++++++++------- tests/test_downloader_handlers.py | 39 +++++++++--------- 2 files changed, 65 insertions(+), 33 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 38cfd1e10..d3feb9815 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -1,7 +1,9 @@ +import six from six.moves.urllib.parse import unquote from scrapy.exceptions import NotConfigured from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.python import to_unicode from .http import HTTPDownloadHandler @@ -37,10 +39,6 @@ class S3DownloadHandler(object): def __init__(self, settings, aws_access_key_id=None, aws_secret_access_key=None, \ httpdownloadhandler=HTTPDownloadHandler, **kw): - _S3Connection = get_s3_connection() - if _S3Connection is None: - raise NotConfigured("missing boto library") - if not aws_access_key_id: aws_access_key_id = settings['AWS_ACCESS_KEY_ID'] if not aws_secret_access_key: @@ -53,10 +51,27 @@ class S3DownloadHandler(object): if anon is None and not aws_access_key_id and not aws_secret_access_key: kw['anon'] = True + self._signer = None try: - self.conn = _S3Connection(aws_access_key_id, aws_secret_access_key, **kw) - except Exception as ex: - raise NotConfigured(str(ex)) + import botocore.auth + import botocore.credentials + except ImportError: + if six.PY3: + raise NotConfigured("missing botocore library") + _S3Connection = get_s3_connection() + if _S3Connection is None: + raise NotConfigured("missing botocore or boto library") + try: + self.conn = _S3Connection( + aws_access_key_id, aws_secret_access_key, **kw) + except Exception as ex: + raise NotConfigured(str(ex)) + else: + SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] + # TODO - anon + self._signer = SignerCls(botocore.credentials.Credentials( + aws_access_key_id, aws_secret_access_key)) + self._download_http = httpdownloadhandler(settings).download_request def download_request(self, request, spider): @@ -65,12 +80,28 @@ class S3DownloadHandler(object): bucket = p.hostname path = p.path + '?' + p.query if p.query else p.path url = '%s://%s.s3.amazonaws.com%s' % (scheme, bucket, path) - signed_headers = self.conn.make_request( + if self._signer is not None: + import botocore.awsrequest + from botocore.vendored.requests.structures import CaseInsensitiveDict + print(url, request.headers) + awsrequest = botocore.awsrequest.AWSRequest( method=request.method, - bucket=bucket, - key=unquote(p.path), - query_args=unquote(p.query), - headers=request.headers, + url='%s://s3.amazonaws.com/%s%s' % (scheme, bucket, path), + # TODO - move to a header method + headers=CaseInsensitiveDict( + (to_unicode(key), to_unicode(b','.join(value))) + for key, value in request.headers.items()), data=request.body) - httpreq = request.replace(url=url, headers=signed_headers) - return self._download_http(httpreq, spider) + self._signer.add_auth(awsrequest) + request = request.replace( + url=url, headers=awsrequest.headers.items()) + else: + signed_headers = self.conn.make_request( + method=request.method, + bucket=bucket, + key=unquote(p.path), + query_args=unquote(p.query), + headers=request.headers, + data=request.body) + request = request.replace(url=url, headers=signed_headers) + return self._download_http(request, spider) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 56608bfc6..06e232503 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -432,13 +432,20 @@ class HttpDownloadHandlerMock(object): return request -class S3AnonTestCase(unittest.TestCase): - try: - import boto - except ImportError: - skip = 'missing boto library' +class BaseS3TestCase(unittest.TestCase): if six.PY3: - skip = 'S3 not supported on Py3' + try: + import botocore + except ImportError: + skip = 'missing botocore library' + else: + try: + import boto + except ImportError: + skip = 'missing boto library' + + +class S3AnonTestCase(BaseS3TestCase): def setUp(self): self.s3reqh = S3DownloadHandler(Settings(), @@ -457,12 +464,6 @@ class S3AnonTestCase(unittest.TestCase): class S3TestCase(unittest.TestCase): download_handler_cls = S3DownloadHandler - try: - import boto - except ImportError: - skip = 'missing boto library' - if six.PY3: - skip = 'S3 not supported on Py3' # test use same example keys than amazon developer guide # http://s3.amazonaws.com/awsdocs/S3/20060301/s3-dg-20060301.pdf @@ -484,7 +485,7 @@ class S3TestCase(unittest.TestCase): headers={'Date': 'Tue, 27 Mar 2007 19:36:42 +0000'}) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:xXjDGYUmKxnwqr5KXNPGldn5LbA=') + b'AWS 0PN5J17HBGZHT7JJ3X82:xXjDGYUmKxnwqr5KXNPGldn5LbA=') def test_request_signing2(self): # puts an object into the johnsmith bucket. @@ -495,7 +496,7 @@ class S3TestCase(unittest.TestCase): }) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:hcicpDDvL9SsO6AkvxqmIWkmOuQ=') + b'AWS 0PN5J17HBGZHT7JJ3X82:hcicpDDvL9SsO6AkvxqmIWkmOuQ=') def test_request_signing3(self): # lists the content of the johnsmith bucket. @@ -506,7 +507,7 @@ class S3TestCase(unittest.TestCase): }) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:jsRt/rhG+Vtp88HrYL706QhE4w4=') + b'AWS 0PN5J17HBGZHT7JJ3X82:jsRt/rhG+Vtp88HrYL706QhE4w4=') def test_request_signing4(self): # fetches the access control policy sub-resource for the 'johnsmith' bucket. @@ -514,7 +515,7 @@ class S3TestCase(unittest.TestCase): method='GET', headers={'Date': 'Tue, 27 Mar 2007 19:44:46 +0000'}) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:thdUi9VAkzhkniLj96JIrOPGi0g=') + b'AWS 0PN5J17HBGZHT7JJ3X82:thdUi9VAkzhkniLj96JIrOPGi0g=') def test_request_signing5(self): # deletes an object from the 'johnsmith' bucket using the @@ -526,7 +527,7 @@ class S3TestCase(unittest.TestCase): }) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=') + b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=') def test_request_signing6(self): # uploads an object to a CNAME style virtual hosted bucket with metadata. @@ -547,7 +548,7 @@ class S3TestCase(unittest.TestCase): }) httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ - 'AWS 0PN5J17HBGZHT7JJ3X82:C0FlOtU8Ylb9KDTpZqYkZPX91iI=') + b'AWS 0PN5J17HBGZHT7JJ3X82:C0FlOtU8Ylb9KDTpZqYkZPX91iI=') def test_request_signing7(self): # ensure that spaces are quoted properly before signing @@ -561,7 +562,7 @@ class S3TestCase(unittest.TestCase): httpreq = self.download_request(req, self.spider) self.assertEqual( httpreq.headers['Authorization'], - 'AWS 0PN5J17HBGZHT7JJ3X82:+CfvG8EZ3YccOrRVMXNaK2eKZmM=') + b'AWS 0PN5J17HBGZHT7JJ3X82:+CfvG8EZ3YccOrRVMXNaK2eKZmM=') class FTPTestCase(unittest.TestCase): From 467553cc2922d1d844fa136a036716eef3158eb9 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 13:30:22 +0300 Subject: [PATCH 02/22] fix anon test: in this case we do no signing, just change the url --- scrapy/core/downloader/handlers/s3.py | 7 +++++-- tests/test_downloader_handlers.py | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index d3feb9815..6d28f866e 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -47,9 +47,10 @@ class S3DownloadHandler(object): # If no credentials could be found anywhere, # consider this an anonymous connection request by default; # unless 'anon' was set explicitly (True/False). - anon = kw.get('anon', None) + anon = kw.get('anon') if anon is None and not aws_access_key_id and not aws_secret_access_key: kw['anon'] = True + self.anon = kw.get('anon') self._signer = None try: @@ -80,7 +81,9 @@ class S3DownloadHandler(object): bucket = p.hostname path = p.path + '?' + p.query if p.query else p.path url = '%s://%s.s3.amazonaws.com%s' % (scheme, bucket, path) - if self._signer is not None: + if self.anon: + request = request.replace(url=url) + elif self._signer is not None: import botocore.awsrequest from botocore.vendored.requests.structures import CaseInsensitiveDict print(url, request.headers) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 06e232503..0f79a208d 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -458,8 +458,10 @@ class S3AnonTestCase(BaseS3TestCase): def test_anon_request(self): req = Request('s3://aws-publicdatasets/') httpreq = self.download_request(req, self.spider) - self.assertEqual(hasattr(self.s3reqh.conn, 'anon'), True) - self.assertEqual(self.s3reqh.conn.anon, True) + self.assertEqual(hasattr(self.s3reqh, 'anon'), True) + self.assertEqual(self.s3reqh.anon, True) + self.assertEqual( + httpreq.url, 'http://aws-publicdatasets.s3.amazonaws.com/') class S3TestCase(unittest.TestCase): From 1b1092b7d073320531986b35461ca5633dc2829a Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 14:19:04 +0300 Subject: [PATCH 03/22] add Headers.to_native_string_dict - useful when interfacing with other libraries --- scrapy/core/downloader/handlers/s3.py | 7 +------ scrapy/http/headers.py | 7 +++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 6d28f866e..0903b84ad 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -85,15 +85,10 @@ class S3DownloadHandler(object): request = request.replace(url=url) elif self._signer is not None: import botocore.awsrequest - from botocore.vendored.requests.structures import CaseInsensitiveDict - print(url, request.headers) awsrequest = botocore.awsrequest.AWSRequest( method=request.method, url='%s://s3.amazonaws.com/%s%s' % (scheme, bucket, path), - # TODO - move to a header method - headers=CaseInsensitiveDict( - (to_unicode(key), to_unicode(b','.join(value))) - for key, value in request.headers.items()), + headers=request.headers.to_native_string_dict(), data=request.body) self._signer.add_auth(awsrequest) request = request.replace( diff --git a/scrapy/http/headers.py b/scrapy/http/headers.py index 13f0f0383..d0c4cd0fb 100644 --- a/scrapy/http/headers.py +++ b/scrapy/http/headers.py @@ -1,6 +1,7 @@ import six from w3lib.http import headers_dict_to_raw from scrapy.utils.datatypes import CaselessDict +from scrapy.utils.python import to_unicode class Headers(CaselessDict): @@ -78,6 +79,12 @@ class Headers(CaselessDict): def to_string(self): return headers_dict_to_raw(self) + def to_native_string_dict(self): + return CaselessDict( + (to_unicode(key, encoding=self.encoding), + to_unicode(b','.join(value), encoding=self.encoding)) + for key, value in self.items()) + def __copy__(self): return self.__class__(self) copy = __copy__ From c3fec83e7eaeaa1d59a479b70857ccaf98a54e35 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 14:46:56 +0300 Subject: [PATCH 04/22] use botocore by default, boto is still used in "precise" env --- tests/requirements-py3.txt | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index 73e73e651..2a89763a5 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -4,7 +4,7 @@ pytest-cov testfixtures jmespath leveldb -boto +botocore # optional for shell wrapper tests bpython ipython diff --git a/tox.ini b/tox.ini index b8d45d5b9..fb31762d8 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,7 @@ envlist = py27 deps = -rrequirements.txt # Extras - boto + botocore Pillow != 3.0.0 leveldb -rtests/requirements.txt From 7748ee6bba8eacb889ce68cd6ced273255b8b9a9 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 14:52:03 +0300 Subject: [PATCH 05/22] mock date in s3 tests when using botocore --- scrapy/core/downloader/handlers/s3.py | 8 +-- tests/test_downloader_handlers.py | 93 ++++++++++++++++++--------- 2 files changed, 67 insertions(+), 34 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 0903b84ad..cb2bb46b1 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -68,10 +68,10 @@ class S3DownloadHandler(object): except Exception as ex: raise NotConfigured(str(ex)) else: - SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] - # TODO - anon - self._signer = SignerCls(botocore.credentials.Credentials( - aws_access_key_id, aws_secret_access_key)) + if not self.anon: + SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] + self._signer = SignerCls(botocore.credentials.Credentials( + aws_access_key_id, aws_secret_access_key)) self._download_http = httpdownloadhandler(settings).download_request diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 0f79a208d..6c4d2e0db 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -1,5 +1,10 @@ import os import six +import contextlib +try: + from unittest import mock +except ImportError: + import mock from twisted.trial import unittest from twisted.protocols.policies import WrappingFactory @@ -433,16 +438,16 @@ class HttpDownloadHandlerMock(object): class BaseS3TestCase(unittest.TestCase): - if six.PY3: - try: - import botocore - except ImportError: + try: + import botocore + except ImportError: + if six.PY2: + try: + import boto + except ImportError: + skip = 'missing botocore or boto library' + else: skip = 'missing botocore library' - else: - try: - import boto - except ImportError: - skip = 'missing boto library' class S3AnonTestCase(BaseS3TestCase): @@ -464,7 +469,7 @@ class S3AnonTestCase(BaseS3TestCase): httpreq.url, 'http://aws-publicdatasets.s3.amazonaws.com/') -class S3TestCase(unittest.TestCase): +class S3TestCase(BaseS3TestCase): download_handler_cls = S3DownloadHandler # test use same example keys than amazon developer guide @@ -481,63 +486,89 @@ class S3TestCase(unittest.TestCase): self.download_request = s3reqh.download_request self.spider = Spider('foo') + @contextlib.contextmanager + def _mocked_date(self, date): + try: + import botocore.auth + except ImportError: + yield + else: + # We need to mock botocore.auth.formatdate, because otherwise + # botocore overrides Date header with current date and time + # and Authorization header is different each time + with mock.patch('botocore.auth.formatdate') as mock_formatdate: + mock_formatdate.return_value = date + yield + def test_request_signing1(self): # gets an object from the johnsmith bucket. - req = Request('s3://johnsmith/photos/puppy.jpg', - headers={'Date': 'Tue, 27 Mar 2007 19:36:42 +0000'}) - httpreq = self.download_request(req, self.spider) + date ='Tue, 27 Mar 2007 19:36:42 +0000' + req = Request('s3://johnsmith/photos/puppy.jpg', headers={'Date': date}) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ b'AWS 0PN5J17HBGZHT7JJ3X82:xXjDGYUmKxnwqr5KXNPGldn5LbA=') def test_request_signing2(self): # puts an object into the johnsmith bucket. + date = 'Tue, 27 Mar 2007 21:15:45 +0000' req = Request('s3://johnsmith/photos/puppy.jpg', method='PUT', headers={ 'Content-Type': 'image/jpeg', - 'Date': 'Tue, 27 Mar 2007 21:15:45 +0000', + 'Date': date, 'Content-Length': '94328', }) - httpreq = self.download_request(req, self.spider) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ b'AWS 0PN5J17HBGZHT7JJ3X82:hcicpDDvL9SsO6AkvxqmIWkmOuQ=') def test_request_signing3(self): # lists the content of the johnsmith bucket. + date = 'Tue, 27 Mar 2007 19:42:41 +0000' req = Request('s3://johnsmith/?prefix=photos&max-keys=50&marker=puppy', \ method='GET', headers={ 'User-Agent': 'Mozilla/5.0', - 'Date': 'Tue, 27 Mar 2007 19:42:41 +0000', + 'Date': date, }) - httpreq = self.download_request(req, self.spider) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ b'AWS 0PN5J17HBGZHT7JJ3X82:jsRt/rhG+Vtp88HrYL706QhE4w4=') def test_request_signing4(self): # fetches the access control policy sub-resource for the 'johnsmith' bucket. - req = Request('s3://johnsmith/?acl', \ - method='GET', headers={'Date': 'Tue, 27 Mar 2007 19:44:46 +0000'}) - httpreq = self.download_request(req, self.spider) + date = 'Tue, 27 Mar 2007 19:44:46 +0000' + req = Request('s3://johnsmith/?acl', + method='GET', headers={'Date': date}) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ b'AWS 0PN5J17HBGZHT7JJ3X82:thdUi9VAkzhkniLj96JIrOPGi0g=') def test_request_signing5(self): # deletes an object from the 'johnsmith' bucket using the # path-style and Date alternative. + date = 'Tue, 27 Mar 2007 21:20:27 +0000' req = Request('s3://johnsmith/photos/puppy.jpg', \ method='DELETE', headers={ - 'Date': 'Tue, 27 Mar 2007 21:20:27 +0000', + 'Date': date, 'x-amz-date': 'Tue, 27 Mar 2007 21:20:26 +0000', }) - httpreq = self.download_request(req, self.spider) - self.assertEqual(httpreq.headers['Authorization'], \ - b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=') + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) + # botocore does not override Date with x-amz-date + self.assertIn(httpreq.headers['Authorization'], [ + b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=', + b'AWS 0PN5J17HBGZHT7JJ3X82:otYM2krxnuHhAofO4oqIV7wcfdU=']) def test_request_signing6(self): # uploads an object to a CNAME style virtual hosted bucket with metadata. + date = 'Tue, 27 Mar 2007 21:06:08 +0000' req = Request('s3://static.johnsmith.net:8080/db-backup.dat.gz', \ method='PUT', headers={ 'User-Agent': 'curl/7.15.5', 'Host': 'static.johnsmith.net:8080', - 'Date': 'Tue, 27 Mar 2007 21:06:08 +0000', + 'Date': date, 'x-amz-acl': 'public-read', 'content-type': 'application/x-download', 'Content-MD5': '4gJE4saaMU4BqNR0kLY+lw==', @@ -548,20 +579,22 @@ class S3TestCase(unittest.TestCase): 'Content-Encoding': 'gzip', 'Content-Length': '5913339', }) - httpreq = self.download_request(req, self.spider) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual(httpreq.headers['Authorization'], \ b'AWS 0PN5J17HBGZHT7JJ3X82:C0FlOtU8Ylb9KDTpZqYkZPX91iI=') def test_request_signing7(self): # ensure that spaces are quoted properly before signing + date = 'Tue, 27 Mar 2007 19:42:41 +0000' req = Request( ("s3://johnsmith/photos/my puppy.jpg" "?response-content-disposition=my puppy.jpg"), method='GET', - headers={ - 'Date': 'Tue, 27 Mar 2007 19:42:41 +0000', - }) - httpreq = self.download_request(req, self.spider) + headers={'Date': date}, + ) + with self._mocked_date(date): + httpreq = self.download_request(req, self.spider) self.assertEqual( httpreq.headers['Authorization'], b'AWS 0PN5J17HBGZHT7JJ3X82:+CfvG8EZ3YccOrRVMXNaK2eKZmM=') From d6bea3bf2eb4793555366a3341fe41456704b860 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 15:17:59 +0300 Subject: [PATCH 06/22] botocore not only does not allow passing our own Date header, but does not handle x-amz-date according to the spec --- tests/test_downloader_handlers.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 6c4d2e0db..57225ee3d 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -546,6 +546,11 @@ class S3TestCase(BaseS3TestCase): b'AWS 0PN5J17HBGZHT7JJ3X82:thdUi9VAkzhkniLj96JIrOPGi0g=') def test_request_signing5(self): + try: import botocore + except ImportError: pass + else: + raise unittest.SkipTest( + 'botocore does not support overriding date with x-amz-date') # deletes an object from the 'johnsmith' bucket using the # path-style and Date alternative. date = 'Tue, 27 Mar 2007 21:20:27 +0000' @@ -557,9 +562,8 @@ class S3TestCase(BaseS3TestCase): with self._mocked_date(date): httpreq = self.download_request(req, self.spider) # botocore does not override Date with x-amz-date - self.assertIn(httpreq.headers['Authorization'], [ - b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=', - b'AWS 0PN5J17HBGZHT7JJ3X82:otYM2krxnuHhAofO4oqIV7wcfdU=']) + self.assertEqual(httpreq.headers['Authorization'], + b'AWS 0PN5J17HBGZHT7JJ3X82:k3nL7gH3+PadhTEVn5Ip83xlYzk=') def test_request_signing6(self): # uploads an object to a CNAME style virtual hosted bucket with metadata. From bcb92b50dc1d1106ee418d6d7a701d87cde4010c Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 5 Feb 2016 17:38:47 +0300 Subject: [PATCH 07/22] check that no extra kwargs are silently discarded --- scrapy/core/downloader/handlers/s3.py | 3 +++ tests/test_downloader_handlers.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index cb2bb46b1..e218a8741 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -68,6 +68,9 @@ class S3DownloadHandler(object): except Exception as ex: raise NotConfigured(str(ex)) else: + kw.pop('anon', None) + if kw: + raise TypeError('Unexpected keyword arguments: %s' % kw) if not self.anon: SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] self._signer = SignerCls(botocore.credentials.Credentials( diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 57225ee3d..c0342b806 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -500,6 +500,10 @@ class S3TestCase(BaseS3TestCase): mock_formatdate.return_value = date yield + def test_extra_kw(self): + with self.assertRaises(TypeError): + S3DownloadHandler(Settings(), extra_kw=True) + def test_request_signing1(self): # gets an object from the johnsmith bucket. date ='Tue, 27 Mar 2007 19:36:42 +0000' From 408bc1580b73c958b2a83785817ef72a1d642198 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 16:20:13 +0300 Subject: [PATCH 08/22] Pass env variables required for running tests against real s3 via tox. --- tox.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tox.ini b/tox.ini index fb31762d8..4d8236b4e 100644 --- a/tox.ini +++ b/tox.ini @@ -14,6 +14,10 @@ deps = Pillow != 3.0.0 leveldb -rtests/requirements.txt +passenv = + FEEDTEST_S3_URI + AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY commands = py.test --cov=scrapy --cov-report= {posargs:scrapy tests} From 19b2910ad145cb4b86ed621e7045f0afdf810d7f Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 16:25:29 +0300 Subject: [PATCH 09/22] Fix assert_aws_environ: check for botocore with boto fallback on PY2 --- scrapy/utils/test.py | 19 ++++++++++++++----- tests/test_downloader_handlers.py | 21 +++++---------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 51edfd353..1ac2e575f 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -5,6 +5,7 @@ This module contains some assorted functions used in tests import os from importlib import import_module +import six from twisted.trial.unittest import SkipTest @@ -12,14 +13,22 @@ def assert_aws_environ(): """Asserts the current environment is suitable for running AWS testsi. Raises SkipTest with the reason if it's not. """ - try: - import boto - except ImportError as e: - raise SkipTest(str(e)) - + skip_if_no_boto() if 'AWS_ACCESS_KEY_ID' not in os.environ: raise SkipTest("AWS keys not found") +def skip_if_no_boto(): + try: + import botocore + except ImportError: + if six.PY2: + try: + import boto + except ImportError: + raise SkipTest('missing botocore or boto library') + else: + raise SkipTest('missing botocore library') + def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it will be used to populate the crawler settings with a project level diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index c0342b806..f34a286c2 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -28,7 +28,7 @@ from scrapy.core.downloader.handlers.s3 import S3DownloadHandler from scrapy.spiders import Spider from scrapy.http import Request from scrapy.settings import Settings -from scrapy.utils.test import get_crawler +from scrapy.utils.test import get_crawler, skip_if_no_boto from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured @@ -437,22 +437,10 @@ class HttpDownloadHandlerMock(object): return request -class BaseS3TestCase(unittest.TestCase): - try: - import botocore - except ImportError: - if six.PY2: - try: - import boto - except ImportError: - skip = 'missing botocore or boto library' - else: - skip = 'missing botocore library' - - -class S3AnonTestCase(BaseS3TestCase): +class S3AnonTestCase(unittest.TestCase): def setUp(self): + skip_if_no_boto() self.s3reqh = S3DownloadHandler(Settings(), httpdownloadhandler=HttpDownloadHandlerMock, #anon=True, # is implicit @@ -469,7 +457,7 @@ class S3AnonTestCase(BaseS3TestCase): httpreq.url, 'http://aws-publicdatasets.s3.amazonaws.com/') -class S3TestCase(BaseS3TestCase): +class S3TestCase(unittest.TestCase): download_handler_cls = S3DownloadHandler # test use same example keys than amazon developer guide @@ -480,6 +468,7 @@ class S3TestCase(BaseS3TestCase): AWS_SECRET_ACCESS_KEY = 'uV3F3YluFJax1cknvbcGwgjvx4QpvB+leU8dUj2o' def setUp(self): + skip_if_no_boto() s3reqh = S3DownloadHandler(Settings(), self.AWS_ACCESS_KEY_ID, self.AWS_SECRET_ACCESS_KEY, httpdownloadhandler=HttpDownloadHandlerMock) From 5d2f067458ce8a3e39d717afae2034dace5db54c Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 16:36:15 +0300 Subject: [PATCH 10/22] S3FeedStorageTest: delete key after test --- tests/test_feedexport.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 176fd93e3..8015b0320 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -102,8 +102,10 @@ class S3FeedStorageTest(unittest.TestCase): file.write("content") yield storage.store(file) u = urlparse(uri) - key = connect_s3().get_bucket(u.hostname, validate=False).get_key(u.path) + bucket = connect_s3().get_bucket(u.hostname, validate=False) + key = bucket.get_key(u.path) self.assertEqual(key.get_contents_as_string(), "content") + bucket.delete_key(u.path) class StdoutFeedStorageTest(unittest.TestCase): From 3ada45a9bb6adcfe5546a515a0574e7efe94c720 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 17:27:34 +0300 Subject: [PATCH 11/22] S3FeedStorageTest: add botocore support, and organize boto/botocore checks --- scrapy/core/downloader/handlers/s3.py | 31 +++++++++----------------- scrapy/extensions/feedexport.py | 32 ++++++++++++++++++--------- scrapy/utils/boto.py | 20 +++++++++++++++++ scrapy/utils/test.py | 16 +++++--------- tests/test_feedexport.py | 24 +++++++++++++++----- 5 files changed, 77 insertions(+), 46 deletions(-) create mode 100644 scrapy/utils/boto.py diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index e218a8741..dd7bce2be 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -1,17 +1,13 @@ -import six from six.moves.urllib.parse import unquote from scrapy.exceptions import NotConfigured from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.python import to_unicode +from scrapy.utils.boto import is_botocore from .http import HTTPDownloadHandler -def get_s3_connection(): - try: - from boto.s3.connection import S3Connection - except ImportError: - return None +def _get_boto_connection(): + from boto.s3.connection import S3Connection class _v19_S3Connection(S3Connection): """A dummy S3Connection wrapper that doesn't do any synchronous download""" @@ -53,21 +49,9 @@ class S3DownloadHandler(object): self.anon = kw.get('anon') self._signer = None - try: + if is_botocore(): import botocore.auth import botocore.credentials - except ImportError: - if six.PY3: - raise NotConfigured("missing botocore library") - _S3Connection = get_s3_connection() - if _S3Connection is None: - raise NotConfigured("missing botocore or boto library") - try: - self.conn = _S3Connection( - aws_access_key_id, aws_secret_access_key, **kw) - except Exception as ex: - raise NotConfigured(str(ex)) - else: kw.pop('anon', None) if kw: raise TypeError('Unexpected keyword arguments: %s' % kw) @@ -75,6 +59,13 @@ class S3DownloadHandler(object): SignerCls = botocore.auth.AUTH_TYPE_MAPS['s3'] self._signer = SignerCls(botocore.credentials.Credentials( aws_access_key_id, aws_secret_access_key)) + else: + _S3Connection = _get_boto_connection() + try: + self.conn = _S3Connection( + aws_access_key_id, aws_secret_access_key, **kw) + except Exception as ex: + raise NotConfigured(str(ex)) self._download_http = httpdownloadhandler(settings).download_request diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fce5e251b..3dab2d77e 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -24,6 +24,7 @@ from scrapy.exceptions import NotConfigured from scrapy.utils.misc import load_object from scrapy.utils.log import failure_to_exc_info from scrapy.utils.python import without_none_values +from scrapy.utils.boto import is_botocore logger = logging.getLogger(__name__) @@ -90,24 +91,33 @@ class S3FeedStorage(BlockingFeedStorage): def __init__(self, uri): from scrapy.conf import settings - try: - import boto - except ImportError: - raise NotConfigured - self.connect_s3 = boto.connect_s3 u = urlparse(uri) self.bucketname = u.hostname self.access_key = u.username or settings['AWS_ACCESS_KEY_ID'] self.secret_key = u.password or settings['AWS_SECRET_ACCESS_KEY'] - self.keyname = u.path + self.is_botocore = is_botocore() + self.keyname = u.path[1:] # remove first "/" + if self.is_botocore: + import botocore.session + session = botocore.session.get_session() + self.s3_client = session.create_client( + 's3', aws_access_key_id=self.access_key, + aws_secret_access_key=self.secret_key) + else: + import boto + self.connect_s3 = boto.connect_s3 def _store_in_thread(self, file): file.seek(0) - conn = self.connect_s3(self.access_key, self.secret_key) - bucket = conn.get_bucket(self.bucketname, validate=False) - key = bucket.new_key(self.keyname) - key.set_contents_from_file(file) - key.close() + if self.is_botocore: + self.s3_client.put_object( + Bucket=self.bucketname, Key=self.keyname, Body=file) + else: + conn = self.connect_s3(self.access_key, self.secret_key) + bucket = conn.get_bucket(self.bucketname, validate=False) + key = bucket.new_key(self.keyname) + key.set_contents_from_file(file) + key.close() class FTPFeedStorage(BlockingFeedStorage): diff --git a/scrapy/utils/boto.py b/scrapy/utils/boto.py new file mode 100644 index 000000000..fd6b22b88 --- /dev/null +++ b/scrapy/utils/boto.py @@ -0,0 +1,20 @@ +"""Boto/botocore helpers""" + +import six + +from scrapy.exceptions import NotConfigured + + +def is_botocore(): + try: + import botocore + return True + except ImportError: + if six.PY2: + try: + import boto + return False + except ImportError: + raise NotConfigured('missing botocore or boto library') + else: + raise NotConfigured('missing botocore library') diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 1ac2e575f..d2f7c0ae4 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -5,9 +5,11 @@ This module contains some assorted functions used in tests import os from importlib import import_module -import six from twisted.trial.unittest import SkipTest +from scrapy.exceptions import NotConfigured +from scrapy.utils.boto import is_botocore + def assert_aws_environ(): """Asserts the current environment is suitable for running AWS testsi. @@ -19,15 +21,9 @@ def assert_aws_environ(): def skip_if_no_boto(): try: - import botocore - except ImportError: - if six.PY2: - try: - import boto - except ImportError: - raise SkipTest('missing botocore or boto library') - else: - raise SkipTest('missing botocore library') + is_botocore() + except NotConfigured as e: + raise SkipTest(e.message) def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 8015b0320..beb800fb5 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -22,6 +22,7 @@ from scrapy.extensions.feedexport import ( ) from scrapy.utils.test import assert_aws_environ from scrapy.utils.python import to_native_str +from scrapy.utils.boto import is_botocore class FileFeedStorageTest(unittest.TestCase): @@ -95,17 +96,30 @@ class S3FeedStorageTest(unittest.TestCase): uri = os.environ.get('FEEDTEST_S3_URI') if not uri: raise unittest.SkipTest("No S3 URI available for testing") - from boto import connect_s3 storage = S3FeedStorage(uri) verifyObject(IFeedStorage, storage) file = storage.open(scrapy.Spider("default")) file.write("content") yield storage.store(file) u = urlparse(uri) - bucket = connect_s3().get_bucket(u.hostname, validate=False) - key = bucket.get_key(u.path) - self.assertEqual(key.get_contents_as_string(), "content") - bucket.delete_key(u.path) + content = self._get_content_and_delete(u.hostname, u.path[1:]) + self.assertEqual(content, "content") + + def _get_content_and_delete(self, bucket, path): + if is_botocore(): + import botocore.session + session = botocore.session.get_session() + client = session.create_client('s3') + key = client.get_object(Bucket=bucket, Key=path) + content = key['Body'].read() + client.delete_object(Bucket=bucket, Key=path) + else: + from boto import connect_s3 + bucket = connect_s3().get_bucket(bucket, validate=False) + key = bucket.get_key(path) + content = key.get_contents_as_string() + bucket.delete_key(path) + return content class StdoutFeedStorageTest(unittest.TestCase): From d1470e85a2987ad53c3b3a89d801a846a053e133 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 17:28:50 +0300 Subject: [PATCH 12/22] S3FeedStorageTest: pass on py3, add some non-ascii content to be sure --- tests/test_feedexport.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index beb800fb5..f3cf1c2cb 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -99,11 +99,12 @@ class S3FeedStorageTest(unittest.TestCase): storage = S3FeedStorage(uri) verifyObject(IFeedStorage, storage) file = storage.open(scrapy.Spider("default")) - file.write("content") + expected_content = b"content: \xe2\x98\x83" + file.write(expected_content) yield storage.store(file) u = urlparse(uri) content = self._get_content_and_delete(u.hostname, u.path[1:]) - self.assertEqual(content, "content") + self.assertEqual(content, expected_content) def _get_content_and_delete(self, bucket, path): if is_botocore(): From 32cd8c91654a8442495cc00aebf5322d1fbc644b Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 17:50:47 +0300 Subject: [PATCH 13/22] add direct test for S3FilesStore --- scrapy/utils/test.py | 20 ++++++++++++++++++++ tests/test_feedexport.py | 22 +++------------------- tests/test_pipeline_files.py | 27 ++++++++++++++++++++++++++- tox.ini | 2 +- 4 files changed, 50 insertions(+), 21 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index d2f7c0ae4..43abd64a0 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -2,6 +2,7 @@ This module contains some assorted functions used in tests """ +from __future__ import absolute_import import os from importlib import import_module @@ -25,6 +26,25 @@ def skip_if_no_boto(): except NotConfigured as e: raise SkipTest(e.message) +def get_s3_content_and_delete(bucket, path): + """ Get content from s3 key, and delete key afterwards. + """ + if is_botocore(): + import botocore.session + session = botocore.session.get_session() + client = session.create_client('s3') + key = client.get_object(Bucket=bucket, Key=path) + content = key['Body'].read() + client.delete_object(Bucket=bucket, Key=path) + else: + import boto + # assuming boto=2.2.2 + bucket = boto.connect_s3().get_bucket(bucket, validate=False) + key = bucket.get_key(path) + content = key.get_contents_as_string() + bucket.delete_key(path) + return content + def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it will be used to populate the crawler settings with a project level diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index f3cf1c2cb..fd2f5a2ba 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -20,7 +20,7 @@ from scrapy.extensions.feedexport import ( IFeedStorage, FileFeedStorage, FTPFeedStorage, S3FeedStorage, StdoutFeedStorage ) -from scrapy.utils.test import assert_aws_environ +from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete from scrapy.utils.python import to_native_str from scrapy.utils.boto import is_botocore @@ -93,7 +93,7 @@ class S3FeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store(self): assert_aws_environ() - uri = os.environ.get('FEEDTEST_S3_URI') + uri = os.environ.get('S3_TEST_FILE_URI') if not uri: raise unittest.SkipTest("No S3 URI available for testing") storage = S3FeedStorage(uri) @@ -103,25 +103,9 @@ class S3FeedStorageTest(unittest.TestCase): file.write(expected_content) yield storage.store(file) u = urlparse(uri) - content = self._get_content_and_delete(u.hostname, u.path[1:]) + content = get_s3_content_and_delete(u.hostname, u.path[1:]) self.assertEqual(content, expected_content) - def _get_content_and_delete(self, bucket, path): - if is_botocore(): - import botocore.session - session = botocore.session.get_session() - client = session.create_client('s3') - key = client.get_object(Bucket=bucket, Key=path) - content = key['Body'].read() - client.delete_object(Bucket=bucket, Key=path) - else: - from boto import connect_s3 - bucket = connect_s3().get_bucket(bucket, validate=False) - key = bucket.get_key(path) - content = key.get_contents_as_string() - bucket.delete_key(path) - return content - class StdoutFeedStorageTest(unittest.TestCase): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index c9977f5ca..6ea47086f 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -4,15 +4,18 @@ import hashlib import warnings from tempfile import mkdtemp from shutil import rmtree +from six.moves.urllib.parse import urlparse +from six import BytesIO from twisted.trial import unittest from twisted.internet import defer -from scrapy.pipelines.files import FilesPipeline, FSFilesStore +from scrapy.pipelines.files import FilesPipeline, FSFilesStore, S3FilesStore from scrapy.item import Item, Field from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.utils.python import to_bytes +from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete from tests import mock @@ -179,6 +182,28 @@ class FilesPipelineTestCaseFields(unittest.TestCase): self.assertEqual(item['stored_file'], [results[0][1]]) +class TestS3FilesStore(unittest.TestCase): + @defer.inlineCallbacks + def test_persist(self): + assert_aws_environ() + uri = os.environ.get('S3_TEST_FILE_URI') + if not uri: + raise unittest.SkipTest("No S3 URI available for testing") + data = b"TestS3FilesStore: \xe2\x98\x83" + buf = BytesIO(data) + meta = {'foo': 'bar'} + path = '' + store = S3FilesStore(uri) + yield store.persist_file(path, buf, info=None, meta=meta) + s = yield store.stat_file(path, info=None) + self.assertIn('last_modified', s) + self.assertIn('checksum', s) + self.assertEqual(s['checksum'], b'3187896a9657a28163abb31667df64c8') + u = urlparse(uri) + content = get_s3_content_and_delete(u.hostname, u.path[1:]) + self.assertEqual(content, data) + + class ItemWithFiles(Item): file_urls = Field() files = Field() diff --git a/tox.ini b/tox.ini index 4d8236b4e..2a8067618 100644 --- a/tox.ini +++ b/tox.ini @@ -15,7 +15,7 @@ deps = leveldb -rtests/requirements.txt passenv = - FEEDTEST_S3_URI + S3_TEST_FILE_URI AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY commands = From cfc567f48e934dc5b148c681c11a73f974e0d37c Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:12:55 +0300 Subject: [PATCH 14/22] botocore support for S3FilesStore --- scrapy/pipelines/files.py | 65 +++++++++++++++++++++++++++------------ tests/test_feedexport.py | 1 - 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index e4011d31d..c757b0a3f 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -28,6 +28,7 @@ from scrapy.utils.misc import md5sum from scrapy.utils.log import failure_to_exc_info from scrapy.utils.python import to_bytes from scrapy.utils.request import referer_str +from scrapy.utils.boto import is_botocore logger = logging.getLogger(__name__) @@ -86,20 +87,30 @@ class S3FilesStore(object): } def __init__(self, uri): - try: + self.is_botocore = is_botocore() + if self.is_botocore: + import botocore.session + session = botocore.session.get_session() + self.s3_client = session.create_client( + 's3', aws_access_key_id=self.AWS_ACCESS_KEY_ID, + aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY) + else: from boto.s3.connection import S3Connection self.S3Connection = S3Connection - except ImportError: - raise NotConfigured("missing boto library") assert uri.startswith('s3://') self.bucket, self.prefix = uri[5:].split('/', 1) def stat_file(self, path, info): def _onsuccess(boto_key): - checksum = boto_key.etag.strip('"') - last_modified = boto_key.last_modified - modified_tuple = parsedate_tz(last_modified) - modified_stamp = int(mktime_tz(modified_tuple)) + if self.is_botocore: + checksum = boto_key['ETag'].strip('"') + last_modified = boto_key['LastModified'] + modified_stamp = time.mktime(last_modified.timetuple()) + else: + checksum = boto_key.etag.strip('"') + last_modified = boto_key.last_modified + modified_tuple = parsedate_tz(last_modified) + modified_stamp = int(mktime_tz(modified_tuple)) return {'checksum': checksum, 'last_modified': modified_stamp} return self._get_boto_key(path).addCallback(_onsuccess) @@ -111,24 +122,40 @@ class S3FilesStore(object): return c.get_bucket(self.bucket, validate=False) def _get_boto_key(self, path): - b = self._get_boto_bucket() key_name = '%s%s' % (self.prefix, path) - return threads.deferToThread(b.get_key, key_name) + if self.is_botocore: + return threads.deferToThread( + self.s3_client.head_object, + Bucket=self.bucket, + Key=key_name) + else: + b = self._get_boto_bucket() + return threads.deferToThread(b.get_key, key_name) def persist_file(self, path, buf, info, meta=None, headers=None): """Upload file to S3 storage""" - b = self._get_boto_bucket() key_name = '%s%s' % (self.prefix, path) - k = b.new_key(key_name) - if meta: - for metakey, metavalue in six.iteritems(meta): - k.set_metadata(metakey, str(metavalue)) - h = self.HEADERS.copy() - if headers: - h.update(headers) buf.seek(0) - return threads.deferToThread(k.set_contents_from_string, buf.getvalue(), - headers=h, policy=self.POLICY) + if self.is_botocore: + return threads.deferToThread( + self.s3_client.put_object, + Bucket=self.bucket, + Key=key_name, + Body=buf, + Metadata={k: str(v) for k, v in six.iteritems(meta)}, + ACL=self.POLICY) + else: + b = self._get_boto_bucket() + k = b.new_key(key_name) + if meta: + for metakey, metavalue in six.iteritems(meta): + k.set_metadata(metakey, str(metavalue)) + h = self.HEADERS.copy() + if headers: + h.update(headers) + return threads.deferToThread( + k.set_contents_from_string, buf.getvalue(), + headers=h, policy=self.POLICY) class FilesPipeline(MediaPipeline): diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index fd2f5a2ba..c76d26b57 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -22,7 +22,6 @@ from scrapy.extensions.feedexport import ( ) from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete from scrapy.utils.python import to_native_str -from scrapy.utils.boto import is_botocore class FileFeedStorageTest(unittest.TestCase): From 3cb7a567ea0f0623fb77bd19b652d89625189f67 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:14:41 +0300 Subject: [PATCH 15/22] py3 fix for TestS3FilesStore: checksum is a native string --- tests/test_pipeline_files.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 6ea47086f..e445d9989 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -198,7 +198,7 @@ class TestS3FilesStore(unittest.TestCase): s = yield store.stat_file(path, info=None) self.assertIn('last_modified', s) self.assertIn('checksum', s) - self.assertEqual(s['checksum'], b'3187896a9657a28163abb31667df64c8') + self.assertEqual(s['checksum'], '3187896a9657a28163abb31667df64c8') u = urlparse(uri) content = get_s3_content_and_delete(u.hostname, u.path[1:]) self.assertEqual(content, data) From 08bc41cc685ae2a8282caf20147bd0963932de24 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:15:55 +0300 Subject: [PATCH 16/22] py3: reviewed s3 downloader handlers --- tests/py3-ignores.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt index 6385ad3b1..ec2947003 100644 --- a/tests/py3-ignores.txt +++ b/tests/py3-ignores.txt @@ -7,7 +7,6 @@ scrapy/xlib/tx/endpoints.py scrapy/xlib/tx/client.py scrapy/xlib/tx/_newclient.py scrapy/xlib/tx/__init__.py -scrapy/core/downloader/handlers/s3.py scrapy/core/downloader/handlers/ftp.py scrapy/linkextractors/sgml.py scrapy/linkextractors/regex.py From 77ebb136840acaa700b6c0ada5e028217a503cfe Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:26:15 +0300 Subject: [PATCH 17/22] fix assertRaises for precise env --- tests/test_downloader_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index f34a286c2..e08d2e4a4 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -490,8 +490,8 @@ class S3TestCase(unittest.TestCase): yield def test_extra_kw(self): - with self.assertRaises(TypeError): - S3DownloadHandler(Settings(), extra_kw=True) + self.assertRaises( + TypeError, S3DownloadHandler, Settings(), extra_kw=True) def test_request_signing1(self): # gets an object from the johnsmith bucket. From e7c4806c5ea7092ecf41724bcd1a4179f9685324 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:33:18 +0300 Subject: [PATCH 18/22] Update feedstorage docs: add botocore, mention that boto is supported only on Python 2 --- docs/topics/feed-exports.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 03c6fb3fb..e5037129c 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -99,12 +99,12 @@ The storages backends supported out of the box are: * :ref:`topics-feed-storage-fs` * :ref:`topics-feed-storage-ftp` - * :ref:`topics-feed-storage-s3` (requires boto_) + * :ref:`topics-feed-storage-s3` (requires botocore_ or boto_) * :ref:`topics-feed-storage-stdout` Some storage backends may be unavailable if the required external libraries are -not available. For example, the S3 backend is only available if the boto_ -library is installed. +not available. For example, the S3 backend is only available if the botocore_ +or boto_ library is installed (Scrapy supports boto_ only on Python 2). .. _topics-feed-uri-params: @@ -177,7 +177,7 @@ The feeds are stored on `Amazon S3`_. * ``s3://mybucket/path/to/export.csv`` * ``s3://aws_key:aws_secret@mybucket/path/to/export.csv`` - * Required external libraries: `boto`_ + * Required external libraries: `botocore`_ or `boto`_ The AWS credentials can be passed as user/password in the URI, or they can be passed through the following settings: @@ -332,4 +332,5 @@ format in :setting:`FEED_EXPORTERS`. E.g., to disable the built-in CSV exporter .. _URI: http://en.wikipedia.org/wiki/Uniform_Resource_Identifier .. _Amazon S3: http://aws.amazon.com/s3/ -.. _boto: http://code.google.com/p/boto/ +.. _boto: https://github.com/boto/boto +.. _botocore: https://github.com/boto/botocore From d1ecb8cd38bd8922780731682b696adc97974240 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 19:48:28 +0300 Subject: [PATCH 19/22] Fix S3TestCase for precise env: we reraise TypeError as NotConfigured in this case --- tests/test_downloader_handlers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index e08d2e4a4..1885a53a0 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -490,8 +490,12 @@ class S3TestCase(unittest.TestCase): yield def test_extra_kw(self): - self.assertRaises( - TypeError, S3DownloadHandler, Settings(), extra_kw=True) + try: + S3DownloadHandler(Settings(), extra_kw=True) + except Exception as e: + self.assertIsInstance(e, (TypeError, NotConfigured)) + else: + assert False def test_request_signing1(self): # gets an object from the johnsmith bucket. From 49313a6988bb9161f9036098c454a180d48ca4b4 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 15 Feb 2016 20:16:40 +0300 Subject: [PATCH 20/22] use absolute_import to import external boto package --- scrapy/utils/boto.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scrapy/utils/boto.py b/scrapy/utils/boto.py index fd6b22b88..421ab2f7e 100644 --- a/scrapy/utils/boto.py +++ b/scrapy/utils/boto.py @@ -1,5 +1,6 @@ """Boto/botocore helpers""" +from __future__ import absolute_import import six from scrapy.exceptions import NotConfigured From 617631f2646d349c4bdc288b3d7b41ad483f5ec6 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 18 Feb 2016 10:10:16 +0300 Subject: [PATCH 21/22] Fix method name: this always returns unicode keys and values --- scrapy/core/downloader/handlers/s3.py | 2 +- scrapy/http/headers.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index dd7bce2be..d8bbdd326 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -82,7 +82,7 @@ class S3DownloadHandler(object): awsrequest = botocore.awsrequest.AWSRequest( method=request.method, url='%s://s3.amazonaws.com/%s%s' % (scheme, bucket, path), - headers=request.headers.to_native_string_dict(), + headers=request.headers.to_unicode_dict(), data=request.body) self._signer.add_auth(awsrequest) request = request.replace( diff --git a/scrapy/http/headers.py b/scrapy/http/headers.py index d0c4cd0fb..62507eb19 100644 --- a/scrapy/http/headers.py +++ b/scrapy/http/headers.py @@ -79,7 +79,10 @@ class Headers(CaselessDict): def to_string(self): return headers_dict_to_raw(self) - def to_native_string_dict(self): + def to_unicode_dict(self): + """ Return headers as a CaselessDict with unicode keys + and unicode values. Multiple values are joined with ','. + """ return CaselessDict( (to_unicode(key, encoding=self.encoding), to_unicode(b','.join(value), encoding=self.encoding)) From d61fbcc8b5fee1c2407d42fb72bd72bb18d40e25 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 18 Feb 2016 10:57:02 +0300 Subject: [PATCH 22/22] Support headers in S3FilesStore.persist_file for botocore --- scrapy/pipelines/files.py | 36 +++++++++++++++++++++++++++++++++++- scrapy/utils/test.py | 4 ++-- tests/test_pipeline_files.py | 18 ++++++++++++++++-- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index c757b0a3f..45ceddcbb 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -29,6 +29,7 @@ from scrapy.utils.log import failure_to_exc_info from scrapy.utils.python import to_bytes from scrapy.utils.request import referer_str from scrapy.utils.boto import is_botocore +from scrapy.utils.datatypes import CaselessDict logger = logging.getLogger(__name__) @@ -137,13 +138,17 @@ class S3FilesStore(object): key_name = '%s%s' % (self.prefix, path) buf.seek(0) if self.is_botocore: + extra = self._headers_to_botocore_kwargs(self.HEADERS) + if headers: + extra.update(self._headers_to_botocore_kwargs(headers)) return threads.deferToThread( self.s3_client.put_object, Bucket=self.bucket, Key=key_name, Body=buf, Metadata={k: str(v) for k, v in six.iteritems(meta)}, - ACL=self.POLICY) + ACL=self.POLICY, + **extra) else: b = self._get_boto_bucket() k = b.new_key(key_name) @@ -157,6 +162,35 @@ class S3FilesStore(object): k.set_contents_from_string, buf.getvalue(), headers=h, policy=self.POLICY) + def _headers_to_botocore_kwargs(self, headers): + """ Convert headers to botocore keyword agruments. + """ + # This is required while we need to support both boto and botocore. + mapping = CaselessDict({ + 'Content-Type': 'ContentType', + 'Cache-Control': 'CacheControl', + 'Content-Disposition': 'ContentDisposition', + 'Content-Encoding': 'ContentEncoding', + 'Content-Language': 'ContentLanguage', + 'Content-Length': 'ContentLength', + 'Content-MD5': 'ContentMD5', + 'Expires': 'Expires', + 'X-Amz-Grant-Full-Control': 'GrantFullControl', + 'X-Amz-Grant-Read': 'GrantRead', + 'X-Amz-Grant-Read-ACP': 'GrantReadACP', + 'X-Amz-Grant-Write-ACP': 'GrantWriteACP', + }) + extra = {} + for key, value in six.iteritems(headers): + try: + kwarg = mapping[key] + except KeyError: + raise TypeError( + 'Header "%s" is not supported by botocore' % key) + else: + extra[kwarg] = value + return extra + class FilesPipeline(MediaPipeline): """Abstract pipeline that implement the file downloading diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 43abd64a0..bf66a8cbe 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -26,7 +26,7 @@ def skip_if_no_boto(): except NotConfigured as e: raise SkipTest(e.message) -def get_s3_content_and_delete(bucket, path): +def get_s3_content_and_delete(bucket, path, with_key=False): """ Get content from s3 key, and delete key afterwards. """ if is_botocore(): @@ -43,7 +43,7 @@ def get_s3_content_and_delete(bucket, path): key = bucket.get_key(path) content = key.get_contents_as_string() bucket.delete_key(path) - return content + return (content, key) if with_key else content def get_crawler(spidercls=None, settings_dict=None): """Return an unconfigured Crawler object. If settings_dict is given, it diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index e445d9989..77e75d5ac 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -16,6 +16,7 @@ from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.utils.python import to_bytes from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete +from scrapy.utils.boto import is_botocore from tests import mock @@ -194,14 +195,27 @@ class TestS3FilesStore(unittest.TestCase): meta = {'foo': 'bar'} path = '' store = S3FilesStore(uri) - yield store.persist_file(path, buf, info=None, meta=meta) + yield store.persist_file( + path, buf, info=None, meta=meta, + headers={'Content-Type': 'image/png'}) s = yield store.stat_file(path, info=None) self.assertIn('last_modified', s) self.assertIn('checksum', s) self.assertEqual(s['checksum'], '3187896a9657a28163abb31667df64c8') u = urlparse(uri) - content = get_s3_content_and_delete(u.hostname, u.path[1:]) + content, key = get_s3_content_and_delete( + u.hostname, u.path[1:], with_key=True) self.assertEqual(content, data) + if is_botocore(): + self.assertEqual(key['Metadata'], {'foo': 'bar'}) + self.assertEqual( + key['CacheControl'], S3FilesStore.HEADERS['Cache-Control']) + self.assertEqual(key['ContentType'], 'image/png') + else: + self.assertEqual(key.metadata, {'foo': 'bar'}) + self.assertEqual( + key.cache_control, S3FilesStore.HEADERS['Cache-Control']) + self.assertEqual(key.content_type, 'image/png') class ItemWithFiles(Item):