Repository: scrapy/scrapy
Pull request #1285 (2015-06-05 22:28:13)

make it easier to use default settings
I think that requiring users to write `CrawlerRunner({})` instead of `CrawlerRunner()` is unnecessary.


Patch:
diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst
index 7bdd28573..4c8c83924 100644
--- a/docs/topics/practices.rst
+++ b/docs/topics/practices.rst
@@ -94,9 +94,7 @@ reactor after `MySpider` has finished running.
         ...
 
     configure_logging({'LOG_FORMAT': '%(levelname)s: %(message)s'})
-    runner = CrawlerRunner({
-        'USER_AGENT': 'Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1)'
-    })
+    runner = CrawlerRunner()
 
     d = runner.crawl(MySpider)
     d.addBoth(lambda _: reactor.stop())
@@ -128,7 +126,7 @@ Here is an example that runs multiple spiders simultaneously:
         # Your second spider definition
         ...
 
-    process = CrawlerProcess({})
+    process = CrawlerProcess()
     process.crawl(MySpider1)
     process.crawl(MySpider2)
     process.start() # the script will block here until all crawling jobs are finished
@@ -151,7 +149,7 @@ Same example using :class:`~scrapy.crawler.CrawlerRunner`:
         ...
 
     configure_logging({})
-    runner = CrawlerRunner({})
+    runner = CrawlerRunner()
     runner.crawl(MySpider1)
     runner.crawl(MySpider2)
     d = runner.join()
@@ -176,7 +174,7 @@ Same example but running the spiders sequentially by chaining the deferreds:
         ...
 
     configure_logging({})
-    runner = CrawlerRunner({})
+    runner = CrawlerRunner()
 
     @defer.inlineCallbacks
     def crawl():
diff --git a/scrapy/crawler.py b/scrapy/crawler.py
index 204395c77..3e18182c0 100644
--- a/scrapy/crawler.py
+++ b/scrapy/crawler.py
@@ -23,8 +23,8 @@ logger = logging.getLogger(__name__)
 
 class Crawler(object):
 
-    def __init__(self, spidercls, settings):
-        if isinstance(settings, dict):
+    def __init__(self, spidercls, settings=None):
+        if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
 
         self.spidercls = spidercls
@@ -108,8 +108,8 @@ class CrawlerRunner(object):
             ":meth:`crawl` and managed by this class."
     )
 
-    def __init__(self, settings):
-        if isinstance(settings, dict):
+    def __init__(self, settings=None):
+        if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
         self.settings = settings
         self.spider_loader = _get_spider_loader(settings)
@@ -205,7 +205,7 @@ class CrawlerProcess(CrawlerRunner):
     process. See :ref:`run-from-script` for an example.
     """
 
-    def __init__(self, settings):
+    def __init__(self, settings=None):
         super(CrawlerProcess, self).__init__(settings)
         install_shutdown_handlers(self._signal_shutdown)
         configure_logging(self.settings)


Test patch:
diff --git a/tests/test_crawler.py b/tests/test_crawler.py
index 1c0804675..53a1202e3 100644
--- a/tests/test_crawler.py
+++ b/tests/test_crawler.py
@@ -10,7 +10,14 @@ from scrapy.utils.misc import load_object
 from scrapy.extensions.throttle import AutoThrottle
 
 
-class CrawlerTestCase(unittest.TestCase):
+class BaseCrawlerTest(unittest.TestCase):
+
+    def assertOptionIsDefault(self, settings, key):
+        self.assertIsInstance(settings, Settings)
+        self.assertEqual(settings[key], getattr(default_settings, key))
+
+
+class CrawlerTestCase(BaseCrawlerTest):
 
     def setUp(self):
         self.crawler = Crawler(DefaultSpider, Settings())
@@ -47,11 +54,11 @@ class CrawlerTestCase(unittest.TestCase):
     def test_crawler_accepts_dict(self):
         crawler = Crawler(DefaultSpider, {'foo': 'bar'})
         self.assertEqual(crawler.settings['foo'], 'bar')
-        self.assertEqual(
-            crawler.settings['RETRY_ENABLED'],
-            default_settings.RETRY_ENABLED
-        )
-        self.assertIsInstance(crawler.settings, Settings)
+        self.assertOptionIsDefault(crawler.settings, 'RETRY_ENABLED')
+
+    def test_crawler_accepts_None(self):
+        crawler = Crawler(DefaultSpider)
+        self.assertOptionIsDefault(crawler.settings, 'RETRY_ENABLED')
 
 
 class SpiderSettingsTestCase(unittest.TestCase):
@@ -77,7 +84,7 @@ class CustomSpiderLoader(SpiderLoader):
     pass
 
 
-class CrawlerRunnerTestCase(unittest.TestCase):
+class CrawlerRunnerTestCase(BaseCrawlerTest):
 
     def test_spider_manager_verify_interface(self):
         settings = Settings({
@@ -93,11 +100,11 @@ class CrawlerRunnerTestCase(unittest.TestCase):
     def test_crawler_runner_accepts_dict(self):
         runner = CrawlerRunner({'foo': 'bar'})
         self.assertEqual(runner.settings['foo'], 'bar')
-        self.assertEqual(
-            runner.settings['RETRY_ENABLED'],
-            default_settings.RETRY_ENABLED
-        )
-        self.assertIsInstance(runner.settings, Settings)
+        self.assertOptionIsDefault(runner.settings, 'RETRY_ENABLED')
+
+    def test_crawler_runner_accepts_None(self):
+        runner = CrawlerRunner()
+        self.assertOptionIsDefault(runner.settings, 'RETRY_ENABLED')
 
     def test_deprecated_attribute_spiders(self):
         with warnings.catch_warnings(record=True) as w:
@@ -119,12 +126,12 @@ class CrawlerRunnerTestCase(unittest.TestCase):
             self.assertIn('Please use SPIDER_LOADER_CLASS', str(w[0].message))
 
 
-class CrawlerProcessTest(unittest.TestCase):
+class CrawlerProcessTest(BaseCrawlerTest):
     def test_crawler_process_accepts_dict(self):
         runner = CrawlerProcess({'foo': 'bar'})
         self.assertEqual(runner.settings['foo'], 'bar')
-        self.assertEqual(
-            runner.settings['RETRY_ENABLED'],
-            default_settings.RETRY_ENABLED
-        )
-        self.assertIsInstance(runner.settings, Settings)
+        self.assertOptionIsDefault(runner.settings, 'RETRY_ENABLED')
+
+    def test_crawler_process_accepts_None(self):
+        runner = CrawlerProcess()
+        self.assertOptionIsDefault(runner.settings, 'RETRY_ENABLED')