RomanNum3ral
/
scrapy
mirror of https://github.com/scrapy/scrapy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3348 from elacuesta/enhancement/alternate-feedexport-constructors 

[MRG+1] Add from_crawler constructor for feed exporters and storages
This commit is contained in:
Daniel Graña 2018-07-25 11:55:16 -03:00 committed by GitHub
parent 258121db04 917c1fde68
commit 732d7e1cce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 188 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
scrapy/extensions/feedexport.py
View File

@ -21,7 +21,7 @@ from w3lib.url import file_uri_to_path
from scrapy import signals
from scrapy.utils.ftp import ftp_makedirs_cwd
from scrapy.exceptions import NotConfigured
from scrapy.utils.misc import load_object
from scrapy.utils.misc import create_instance, 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
@ -93,12 +93,29 @@ class FileFeedStorage(object):
class S3FeedStorage(BlockingFeedStorage):
def __init__(self, uri):
from scrapy.conf import settings
def __init__(self, uri, access_key=None, secret_key=None):
# BEGIN Backwards compatibility for initialising without keys (and
# without using from_crawler)
no_defaults = access_key is None and secret_key is None
if no_defaults:
from scrapy.conf import settings
if 'AWS_ACCESS_KEY_ID' in settings or 'AWS_SECRET_ACCESS_KEY' in settings:
import warnings
from scrapy.exceptions import ScrapyDeprecationWarning
warnings.warn(
"Initialising `scrapy.extensions.feedexport.S3FeedStorage` "
"without AWS keys is deprecated. Please supply credentials or "
"use the `from_crawler()` constructor.",
category=ScrapyDeprecationWarning,
stacklevel=2
)
access_key = settings['AWS_ACCESS_KEY_ID']
secret_key = settings['AWS_SECRET_ACCESS_KEY']
# END Backwards compatibility
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.access_key = u.username or access_key
self.secret_key = u.password or secret_key
self.is_botocore = is_botocore()
self.keyname = u.path[1:] # remove first "/"
if self.is_botocore:
@ -111,6 +128,11 @@ class S3FeedStorage(BlockingFeedStorage):
import boto
self.connect_s3 = boto.connect_s3
@classmethod
def from_crawler(cls, crawler, uri):
return cls(uri, crawler.settings['AWS_ACCESS_KEY_ID'],
crawler.settings['AWS_SECRET_ACCESS_KEY'])
def _store_in_thread(self, file):
file.seek(0)
if self.is_botocore:
@ -181,6 +203,7 @@ class FeedExporter(object):
@classmethod
def from_crawler(cls, crawler):
o = cls(crawler.settings)
o.crawler = crawler
crawler.signals.connect(o.open_spider, signals.spider_opened)
crawler.signals.connect(o.close_spider, signals.spider_closed)
crawler.signals.connect(o.item_scraped, signals.item_scraped)
@ -253,11 +276,16 @@ class FeedExporter(object):
logger.error("Unknown feed storage scheme: %(scheme)s",
{'scheme': scheme})
def _get_instance(self, objcls, *args, **kwargs):
return create_instance(
objcls, self.settings, getattr(self, 'crawler', None),
*args, **kwargs)
def _get_exporter(self, *args, **kwargs):
return self.exporters[self.format](*args, **kwargs)
return self._get_instance(self.exporters[self.format], *args, **kwargs)
def _get_storage(self, uri):
return self.storages[urlparse(uri).scheme](uri)
return self._get_instance(self.storages[urlparse(uri).scheme], uri)
def _get_uri_params(self, spider):
params = {}

9
scrapy/middleware.py
View File

@ -3,7 +3,7 @@ import logging
import pprint
from scrapy.exceptions import NotConfigured
from scrapy.utils.misc import load_object
from scrapy.utils.misc import create_instance, load_object
from scrapy.utils.defer import process_parallel, process_chain, process_chain_both
logger = logging.getLogger(__name__)
@ -32,12 +32,7 @@ class MiddlewareManager(object):
for clspath in mwlist:
try:
mwcls = load_object(clspath)
if crawler and hasattr(mwcls, 'from_crawler'):
mw = mwcls.from_crawler(crawler)
elif hasattr(mwcls, 'from_settings'):
mw = mwcls.from_settings(settings)
else:
mw = mwcls()
mw = create_instance(mwcls, settings, crawler)
middlewares.append(mw)
enabled.append(clspath)
except NotConfigured as e:

25
scrapy/utils/misc.py
View File

@ -117,3 +117,28 @@ def md5sum(file):
def rel_has_nofollow(rel):
"""Return True if link rel attribute has nofollow type"""
return True if rel is not None and 'nofollow' in rel.split() else False
def create_instance(objcls, settings, crawler, *args, **kwargs):
"""Construct a class instance using its ``from_crawler`` or
``from_settings`` constructors, if available.
At least one of ``settings`` and ``crawler`` needs to be different from
``None``. If ``settings `` is ``None``, ``crawler.settings`` will be used.
If ``crawler`` is ``None``, only the ``from_settings`` constructor will be
tried.
``*args`` and ``**kwargs`` are forwarded to the constructors.
Raises ``ValueError`` if both ``settings`` and ``crawler`` are ``None``.
"""
if settings is None:
if crawler is None:
raise ValueError("Specifiy at least one of settings and crawler.")
settings = crawler.settings
if crawler and hasattr(objcls, 'from_crawler'):
return objcls.from_crawler(crawler, *args, **kwargs)
elif hasattr(objcls, 'from_settings'):
return objcls.from_settings(settings, *args, **kwargs)
else:
return objcls(*args, **kwargs)

70
tests/test_feedexport.py
View File

@ -6,16 +6,19 @@ from io import BytesIO
import tempfile
import shutil
from six.moves.urllib.parse import urlparse
import warnings
from zope.interface.verify import verifyObject
from twisted.trial import unittest
from twisted.internet import defer
from scrapy.crawler import CrawlerRunner
from scrapy.settings import Settings
from tests import mock
from tests.mockserver import MockServer
from w3lib.url import path_to_file_uri
import scrapy
from scrapy.exporters import CsvItemExporter
from scrapy.extensions.feedexport import (
IFeedStorage, FileFeedStorage, FTPFeedStorage,
S3FeedStorage, StdoutFeedStorage,
@ -130,13 +133,49 @@ class BlockingFeedStorageTest(unittest.TestCase):
class S3FeedStorageTest(unittest.TestCase):
@mock.patch('scrapy.conf.settings', new={'AWS_ACCESS_KEY_ID': 'conf_key',
'AWS_SECRET_ACCESS_KEY': 'conf_secret'}, create=True)
def test_parse_credentials(self):
try:
import boto
except ImportError:
raise unittest.SkipTest("S3FeedStorage requires boto")
aws_credentials = {'AWS_ACCESS_KEY_ID': 'settings_key',
'AWS_SECRET_ACCESS_KEY': 'settings_secret'}
crawler = get_crawler(settings_dict=aws_credentials)
# Instantiate with crawler
storage = S3FeedStorage.from_crawler(crawler,
's3://mybucket/export.csv')
self.assertEqual(storage.access_key, 'settings_key')
self.assertEqual(storage.secret_key, 'settings_secret')
# Instantiate directly
storage = S3FeedStorage('s3://mybucket/export.csv',
aws_credentials['AWS_ACCESS_KEY_ID'],
aws_credentials['AWS_SECRET_ACCESS_KEY'])
self.assertEqual(storage.access_key, 'settings_key')
self.assertEqual(storage.secret_key, 'settings_secret')
# URI priority > settings priority
storage = S3FeedStorage('s3://uri_key:uri_secret@mybucket/export.csv',
aws_credentials['AWS_ACCESS_KEY_ID'],
aws_credentials['AWS_SECRET_ACCESS_KEY'])
self.assertEqual(storage.access_key, 'uri_key')
self.assertEqual(storage.secret_key, 'uri_secret')
# Backwards compatibility for initialising without settings
with warnings.catch_warnings(record=True) as w:
storage = S3FeedStorage('s3://mybucket/export.csv')
self.assertEqual(storage.access_key, 'conf_key')
self.assertEqual(storage.secret_key, 'conf_secret')
self.assertTrue('without AWS keys' in str(w[-1].message))
@defer.inlineCallbacks
def test_store(self):
assert_aws_environ()
uri = os.environ.get('S3_TEST_FILE_URI')
if not uri:
raise unittest.SkipTest("No S3 URI available for testing")
storage = S3FeedStorage(uri)
access_key = os.environ.get('AWS_ACCESS_KEY_ID')
secret_key = os.environ.get('AWS_SECRET_ACCESS_KEY')
storage = S3FeedStorage(uri, access_key, secret_key)
verifyObject(IFeedStorage, storage)
file = storage.open(scrapy.Spider("default"))
expected_content = b"content: \xe2\x98\x83"
@ -159,6 +198,23 @@ class StdoutFeedStorageTest(unittest.TestCase):
self.assertEqual(out.getvalue(), b"content")
class FromCrawlerMixin(object):
init_with_crawler = False
@classmethod
def from_crawler(cls, crawler, *args, **kwargs):
cls.init_with_crawler = True
return cls(*args, **kwargs)
class FromCrawlerCsvItemExporter(CsvItemExporter, FromCrawlerMixin):
pass
class FromCrawlerFileFeedStorage(FileFeedStorage, FromCrawlerMixin):
pass
class FeedExportTest(unittest.TestCase):
class MyItem(scrapy.Item):
@ -598,3 +654,15 @@ class FeedExportTest(unittest.TestCase):
data = yield self.exported_data(items, settings)
print(row['format'], row['indent'])
self.assertEqual(row['expected'], data)
@defer.inlineCallbacks
def test_init_exporters_storages_with_crawler(self):
settings = {
'FEED_EXPORTERS': {'csv': 'tests.test_feedexport.'
'FromCrawlerCsvItemExporter'},
'FEED_STORAGES': {'file': 'tests.test_feedexport.'
'FromCrawlerFileFeedStorage'},
}
yield self.exported_data({}, settings)
self.assertTrue(FromCrawlerCsvItemExporter.init_with_crawler)
self.assertTrue(FromCrawlerFileFeedStorage.init_with_crawler)

58
tests/test_utils_misc/__init__.py
View File

@ -3,7 +3,9 @@ import os
import unittest
from scrapy.item import Item, Field
from scrapy.utils.misc import load_object, arg_to_iter, walk_modules
from scrapy.utils.misc import arg_to_iter, create_instance, load_object, walk_modules
from tests import mock
__doctests__ = ['scrapy.utils.misc']
@ -74,5 +76,59 @@ class UtilsMiscTestCase(unittest.TestCase):
self.assertEqual(list(arg_to_iter({'a':1})), [{'a': 1}])
self.assertEqual(list(arg_to_iter(TestItem(name="john"))), [TestItem(name="john")])
def test_create_instance(self):
settings = mock.MagicMock()
crawler = mock.MagicMock(spec_set=['settings'])
args = (True, 100.)
kwargs = {'key': 'val'}
def _test_with_settings(mock, settings):
create_instance(mock, settings, None, *args, **kwargs)
if hasattr(mock, 'from_crawler'):
self.assertEqual(mock.from_crawler.call_count, 0)
if hasattr(mock, 'from_settings'):
mock.from_settings.assert_called_once_with(settings, *args,
**kwargs)
self.assertEqual(mock.call_count, 0)
else:
mock.assert_called_once_with(*args, **kwargs)
def _test_with_crawler(mock, settings, crawler):
create_instance(mock, settings, crawler, *args, **kwargs)
if hasattr(mock, 'from_crawler'):
mock.from_crawler.assert_called_once_with(crawler, *args,
**kwargs)
if hasattr(mock, 'from_settings'):
self.assertEqual(mock.from_settings.call_count, 0)
self.assertEqual(mock.call_count, 0)
elif hasattr(mock, 'from_settings'):
mock.from_settings.assert_called_once_with(settings, *args,
**kwargs)
self.assertEqual(mock.call_count, 0)
else:
mock.assert_called_once_with(*args, **kwargs)
# Check usage of correct constructor using four mocks:
# 1. with no alternative constructors
# 2. with from_settings() constructor
# 3. with from_crawler() constructor
# 4. with from_settings() and from_crawler() constructor
spec_sets = ([], ['from_settings'], ['from_crawler'],
['from_settings', 'from_crawler'])
for specs in spec_sets:
m = mock.MagicMock(spec_set=specs)
_test_with_settings(m, settings)
m.reset_mock()
_test_with_crawler(m, settings, crawler)
# Check adoption of crawler settings
m = mock.MagicMock(spec_set=['from_settings'])
create_instance(m, None, crawler, *args, **kwargs)
m.from_settings.assert_called_once_with(crawler.settings, *args,
**kwargs)
with self.assertRaises(ValueError):
create_instance(m, None, None)
if __name__ == "__main__":
unittest.main()