From cf50561b8696b645940ae33ba9b168ef97580477 Mon Sep 17 00:00:00 2001 From: nyov Date: Wed, 26 Aug 2020 11:08:14 +0000 Subject: [PATCH] Allow passing classes directly in Settings (#3873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrián Chaves --- docs/topics/settings.rst | 26 +++++++++++++++++++++ scrapy/utils/deprecate.py | 2 +- scrapy/utils/misc.py | 14 +++++++++-- tests/test_crawler.py | 2 +- tests/test_downloader_handlers.py | 6 ++--- tests/test_feedexport.py | 22 +++++++++--------- tests/test_middleware.py | 2 +- tests/test_settings/__init__.py | 32 ++++++++++++++++++++++++++ tests/test_spidermiddleware_referer.py | 2 +- tests/test_utils_misc/__init__.py | 15 ++++++++++-- 10 files changed, 101 insertions(+), 22 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 618b9989e..2924c0566 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -98,6 +98,32 @@ class. The global defaults are located in the ``scrapy.settings.default_settings`` module and documented in the :ref:`topics-settings-ref` section. + +Import paths and classes +======================== + +.. versionadded:: VERSION + +When a setting references a callable object to be imported by Scrapy, such as a +class or a function, there are two different ways you can specify that object: + +- As a string containing the import path of that object + +- As the object itself + +For example:: + + from mybot.pipelines.validate import ValidateMyItem + ITEM_PIPELINES = { + # passing the classname... + ValidateMyItem: 300, + # ...equals passing the class path + 'mybot.pipelines.validate.ValidateMyItem': 300, + } + +.. note:: Passing non-callable objects is not supported. + + How to access settings ====================== diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 3c8e3c8b5..8277a3c8f 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -135,7 +135,7 @@ DEPRECATION_RULES = [ def update_classpath(path): """Update a deprecated path from an object with its new location""" for prefix, replacement in DEPRECATION_RULES: - if path.startswith(prefix): + if isinstance(path, str) and path.startswith(prefix): new_path = path.replace(prefix, replacement, 1) warnings.warn("`{}` class is deprecated, use `{}` instead".format(path, new_path), ScrapyDeprecationWarning) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index bd400bd30..d5d1f301c 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -39,10 +39,20 @@ def arg_to_iter(arg): def load_object(path): """Load an object given its absolute object path, and return it. - object can be the import path of a class, function, variable or an - instance, e.g. 'scrapy.downloadermiddlewares.redirect.RedirectMiddleware' + The object can be the import path of a class, function, variable or an + instance, e.g. 'scrapy.downloadermiddlewares.redirect.RedirectMiddleware'. + + If ``path`` is not a string, but is a callable object, such as a class or + a function, then return it as is. """ + if not isinstance(path, str): + if callable(path): + return path + else: + raise TypeError("Unexpected argument type, expected string " + "or object, got: %s" % type(path)) + try: dot = path.rindex('.') except ValueError: diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 7c2e251a9..85035a220 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -142,7 +142,7 @@ class CrawlerRunnerTestCase(BaseCrawlerTest): def test_spider_manager_verify_interface(self): settings = Settings({ - 'SPIDER_LOADER_CLASS': 'tests.test_crawler.SpiderLoaderWithWrongInterface' + 'SPIDER_LOADER_CLASS': SpiderLoaderWithWrongInterface, }) with warnings.catch_warnings(record=True) as w: self.assertRaises(AttributeError, CrawlerRunner, settings) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 7059f0892..e50bdb391 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -61,7 +61,7 @@ class OffDH: class LoadTestCase(unittest.TestCase): def test_enabled_handler(self): - handlers = {'scheme': 'tests.test_downloader_handlers.DummyDH'} + handlers = {'scheme': DummyDH} crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers}) dh = DownloadHandlers(crawler) self.assertIn('scheme', dh._schemes) @@ -69,7 +69,7 @@ class LoadTestCase(unittest.TestCase): self.assertNotIn('scheme', dh._notconfigured) def test_not_configured_handler(self): - handlers = {'scheme': 'tests.test_downloader_handlers.OffDH'} + handlers = {'scheme': OffDH} crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers}) dh = DownloadHandlers(crawler) self.assertIn('scheme', dh._schemes) @@ -87,7 +87,7 @@ class LoadTestCase(unittest.TestCase): self.assertIn('scheme', dh._notconfigured) def test_lazy_handlers(self): - handlers = {'scheme': 'tests.test_downloader_handlers.DummyLazyDH'} + handlers = {'scheme': DummyLazyDH} crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers}) dh = DownloadHandlers(crawler) self.assertIn('scheme', dh._schemes) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 850485b5e..689d25fef 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -845,7 +845,7 @@ class FeedExportTest(FeedExportTestBase): self._random_temp_filename(): {'format': 'xml'}, self._random_temp_filename(): {'format': 'csv'}, }, - 'FEED_STORAGES': {'file': 'tests.test_feedexport.LogOnStoreFileStorage'}, + 'FEED_STORAGES': {'file': LogOnStoreFileStorage}, 'FEED_STORE_EMPTY': False } @@ -1189,8 +1189,8 @@ class FeedExportTest(FeedExportTestBase): @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'}, + 'FEED_EXPORTERS': {'csv': FromCrawlerCsvItemExporter}, + 'FEED_STORAGES': {'file': FromCrawlerFileFeedStorage}, 'FEEDS': { self._random_temp_filename(): {'format': 'csv'}, }, @@ -1219,7 +1219,7 @@ class FeedExportTest(FeedExportTestBase): self._random_temp_filename(): {'format': 'xml'}, self._random_temp_filename(): {'format': 'csv'}, }, - 'FEED_STORAGES': {'file': 'tests.test_feedexport.DummyBlockingFeedStorage'}, + 'FEED_STORAGES': {'file': DummyBlockingFeedStorage}, } items = [ {'foo': 'bar1', 'baz': ''}, @@ -1240,7 +1240,7 @@ class FeedExportTest(FeedExportTestBase): self._random_temp_filename(): {'format': 'xml'}, self._random_temp_filename(): {'format': 'csv'}, }, - 'FEED_STORAGES': {'file': 'tests.test_feedexport.FailingBlockingFeedStorage'}, + 'FEED_STORAGES': {'file': FailingBlockingFeedStorage}, } items = [ {'foo': 'bar1', 'baz': ''}, @@ -1667,7 +1667,7 @@ class StdoutFeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.StdoutFeedStorageWithoutFeedOptions' + 'file': StdoutFeedStorageWithoutFeedOptions }, } crawler = get_crawler(settings_dict=settings_dict) @@ -1708,7 +1708,7 @@ class FileFeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.FileFeedStorageWithoutFeedOptions' + 'file': FileFeedStorageWithoutFeedOptions }, } crawler = get_crawler(settings_dict=settings_dict) @@ -1756,7 +1756,7 @@ class S3FeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.S3FeedStorageWithoutFeedOptions' + 'file': S3FeedStorageWithoutFeedOptions }, } crawler = get_crawler(settings_dict=settings_dict) @@ -1784,7 +1784,7 @@ class S3FeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.S3FeedStorageWithoutFeedOptionsWithFromCrawler' + 'file': S3FeedStorageWithoutFeedOptionsWithFromCrawler }, } crawler = get_crawler(settings_dict=settings_dict) @@ -1833,7 +1833,7 @@ class FTPFeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.FTPFeedStorageWithoutFeedOptions' + 'file': FTPFeedStorageWithoutFeedOptions }, } crawler = get_crawler(settings_dict=settings_dict) @@ -1861,7 +1861,7 @@ class FTPFeedStoragePreFeedOptionsTest(unittest.TestCase): settings_dict = { 'FEED_URI': 'file:///tmp/foobar', 'FEED_STORAGES': { - 'file': 'tests.test_feedexport.FTPFeedStorageWithoutFeedOptionsWithFromCrawler' + 'file': FTPFeedStorageWithoutFeedOptionsWithFromCrawler }, } crawler = get_crawler(settings_dict=settings_dict) diff --git a/tests/test_middleware.py b/tests/test_middleware.py index b2b75ef20..8651431b5 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -50,7 +50,7 @@ class TestMiddlewareManager(MiddlewareManager): @classmethod def _get_mwlist_from_settings(cls, settings): - return ['tests.test_middleware.%s' % x for x in ['M1', 'MOff', 'M3']] + return [M1, MOff, M3] def _add_middleware(self, mw): super()._add_middleware(mw) diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index 6e56a28f5..916fe012a 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -385,6 +385,38 @@ class SettingsTest(unittest.TestCase): self.assertIn('key', mydict) self.assertEqual(mydict['key'], 'val') + def test_passing_objects_as_values(self): + from scrapy.core.downloader.handlers.file import FileDownloadHandler + from scrapy.utils.misc import create_instance + from scrapy.utils.test import get_crawler + + class TestPipeline(): + def process_item(self, i, s): + return i + + settings = Settings({ + 'ITEM_PIPELINES': { + TestPipeline: 800, + }, + 'DOWNLOAD_HANDLERS': { + 'ftp': FileDownloadHandler, + }, + }) + + self.assertIn('ITEM_PIPELINES', settings.attributes) + + mypipeline, priority = settings.getdict('ITEM_PIPELINES').popitem() + self.assertEqual(priority, 800) + self.assertEqual(mypipeline, TestPipeline) + self.assertIsInstance(mypipeline(), TestPipeline) + self.assertEqual(mypipeline().process_item('item', None), 'item') + + myhandler = settings.getdict('DOWNLOAD_HANDLERS').pop('ftp') + self.assertEqual(myhandler, FileDownloadHandler) + myhandler_instance = create_instance(myhandler, None, get_crawler()) + self.assertIsInstance(myhandler_instance, FileDownloadHandler) + self.assertTrue(hasattr(myhandler_instance, 'download_request')) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index 5141f47af..9456b01d4 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -385,7 +385,7 @@ class CustomPythonOrgPolicy(ReferrerPolicy): class TestSettingsCustomPolicy(TestRefererMiddleware): - settings = {'REFERRER_POLICY': 'tests.test_spidermiddleware_referer.CustomPythonOrgPolicy'} + settings = {'REFERRER_POLICY': CustomPythonOrgPolicy} scenarii = [ ('https://example.com/', 'https://scrapy.org/', b'https://python.org/'), ('http://example.com/', 'http://scrapy.org/', b'http://python.org/'), diff --git a/tests/test_utils_misc/__init__.py b/tests/test_utils_misc/__init__.py index 9bb996d27..e95a3a316 100644 --- a/tests/test_utils_misc/__init__.py +++ b/tests/test_utils_misc/__init__.py @@ -12,11 +12,22 @@ __doctests__ = ['scrapy.utils.misc'] class UtilsMiscTestCase(unittest.TestCase): - def test_load_object(self): + def test_load_object_class(self): + obj = load_object(Field) + self.assertIs(obj, Field) + obj = load_object('scrapy.item.Field') + self.assertIs(obj, Field) + + def test_load_object_function(self): + obj = load_object(load_object) + self.assertIs(obj, load_object) obj = load_object('scrapy.utils.misc.load_object') - assert obj is load_object + self.assertIs(obj, load_object) + + def test_load_object_exceptions(self): self.assertRaises(ImportError, load_object, 'nomodule999.mod.function') self.assertRaises(NameError, load_object, 'scrapy.utils.misc.load_object999') + self.assertRaises(TypeError, load_object, dict()) def test_walk_modules(self): mods = walk_modules('tests.test_utils_misc.test_walk_modules')