Merge pull request #46 from scrapinghub/use-crawler-settings-as-fallback

Frontera configuration using Scrapy settings infrastructure.

Author: sibiryakov

docs/source/topics/frontier-api.rst

@@ -30,7 +30,7 @@ class method, using either a string path::
     >>> from frontera import FrontierManager
     >>> frontier = FrontierManager.from_settings('my_project.frontier.settings')
 
-or a :class:`Settings <frontera.settings.Settings>` object instance::
+or a :class:`BaseSettings <frontera.settings.BaseSettings>` object instance::
 
     >>> from frontera import FrontierManager, Settings
     >>> settings = Settings()

docs/source/topics/scrapy-integration.rst

@@ -81,6 +81,18 @@ Default: ``None``
 
 A file path pointing to Frontera settings.
 
+
+Defining frontier settings via Scrapy settings
+==============================================
+
+:ref:`Frontier settings <frontier-built-in-frontier-settings>` can also be defined via Scrapy settings.
+In this case, the order of precedence will be the following:
+
+1. Settings defined in the file pointed by FRONTERA_SETTINGS (higher precedence)
+2. Settings defined in the Scrapy settings
+3. Default frontier settings
+
+
 .. _Scrapy middlewares: http://doc.scrapy.org/en/latest/topics/downloader-middleware.html
 .. _Scrapy settings: http://doc.scrapy.org/en/latest/topics/settings.html
 .. _DOWNLOADER_MIDDLEWARES: http://doc.scrapy.org/en/latest/topics/settings.html#std:setting-DOWNLOADER_MIDDLEWARES

frontera/contrib/scrapy/schedulers/frontier.py

@@ -6,6 +6,7 @@
 from time import time
 
 from frontera.contrib.scrapy.manager import ScrapyFrontierManager
+from frontera.contrib.scrapy.settings_adapter import ScrapySettingsAdapter
 
 STATS_PREFIX = 'frontera'
 
@@ -75,12 +76,8 @@ def __init__(self, crawler):
         self.stats_manager = StatsManager(crawler.stats)
         self._pending_requests = deque()
         self.redirect_enabled = crawler.settings.get('REDIRECT_ENABLED')
-
-        frontier_settings = crawler.settings.get('FRONTERA_SETTINGS', None)
-        if not frontier_settings:
-            log.msg('FRONTERA_SETTINGS not found! Using default Frontera settings...', log.WARNING)
-        self.frontier = ScrapyFrontierManager(frontier_settings)
-
+        settings = ScrapySettingsAdapter(crawler.settings)
+        self.frontier = ScrapyFrontierManager(settings)
         self._delay_on_empty = self.frontier.manager.settings.get('DELAY_ON_EMPTY')
         self._delay_next_call = 0.0
 

frontera/contrib/scrapy/settings_adapter.py

@@ -0,0 +1,23 @@
+from frontera.settings import BaseSettings, DefaultSettings
+
+
+class ScrapySettingsAdapter(BaseSettings):
+    """
+    Wrapps the frontera settings, falling back to scrapy and default settings
+    """
+    def __init__(self, crawler_settings):
+        frontera_settings = crawler_settings.get('FRONTERA_SETTINGS', None)
+        super(ScrapySettingsAdapter, self).__init__(module=frontera_settings)
+        self._crawler_settings = crawler_settings or {}
+        self._default_settings = DefaultSettings()
+
+    def get(self, key, default_value=None):
+        val = super(ScrapySettingsAdapter, self).get(key)
+        if val is not None:
+            return val
+
+        val = self._crawler_settings.get(key)
+        if val is not None:
+            return val
+
+        return self._default_settings.get(key, default_value)

frontera/core/manager.py

@@ -2,7 +2,7 @@
 
 from frontera.exceptions import NotConfigured
 from frontera.utils.misc import load_object
-from frontera.settings import Settings
+from frontera.settings import Settings, BaseSettings
 from frontera.core.components import Backend, Middleware
 from frontera.logger import FrontierLogger
 from frontera.core import models
@@ -119,11 +119,10 @@ def __init__(self, request_model, response_model, backend, logger, event_log_man
     def from_settings(cls, settings=None):
         """
         Returns a :class:`FrontierManager <frontera.core.manager.FrontierManager>`  instance initialized with \
-        the passed settings argument. Argument value can either be a string path pointing to settings file or a \
-        :class:`Settings <frontera.settings.Settings>` object instance. If no settings is given,
+        the passed settings argument. If no settings is given,
         :ref:`frontier default settings <frontier-default-settings>` are used.
         """
-        manager_settings = Settings(settings)
+        manager_settings = Settings.object_from(settings)
         return FrontierManager(request_model=manager_settings.REQUEST_MODEL,
                                response_model=manager_settings.RESPONSE_MODEL,
                                backend=manager_settings.BACKEND,

frontera/settings/__init__.py

@@ -4,9 +4,12 @@
 import default_settings
 
 
-class Settings(object):
+class BaseSettings(object):
     """
     An object that holds frontier settings values.
+
+    This also defines the base interface for all classes that are to be used
+    as settings in frontera.
     """
     def __init__(self, module=None, attributes=None):
         """
@@ -15,7 +18,6 @@ def __init__(self, module=None, attributes=None):
 
         """
         self.attributes = {}
-        self.add_module(default_settings)
         if module:
             self.add_module(module)
         if attributes:
@@ -25,9 +27,24 @@ def __init__(self, module=None, attributes=None):
     def from_params(cls, **kwargs):
         return cls(attributes=kwargs)
 
+    @classmethod
+    def object_from(cls, settings):
+        """
+        Generates a new settings object based on a previous obj or settings
+        file.
+
+        `settings` can either be a string path pointing to settings file or a \
+        :class:`BaseSettings <frontera.settings.BaseSettings>` object instance.
+        """
+        if isinstance(settings, BaseSettings):
+            return settings
+        else:
+            return cls(settings)
+
     def __getattr__(self, name):
-        if name.isupper() and name in self.attributes:
-            return self.attributes[name]
+        val = self.get(name)
+        if val is not None:
+            return val
         else:
             return self.__dict__[name]
 
@@ -38,15 +55,11 @@ def __setattr__(self, name, value):
             self.__dict__[name] = value
 
     def add_module(self, module):
-        if isinstance(module, Settings):
-            for name, value in module.attributes.items():
-                self.set(name, value)
-        else:
-            if isinstance(module, six.string_types):
-                module = import_module(module)
-            for key in dir(module):
-                if key.isupper():
-                    self.set(key, getattr(module, key))
+        if isinstance(module, six.string_types):
+            module = import_module(module)
+        for key in dir(module):
+            if key.isupper():
+                self.set(key, getattr(module, key))
 
     def get(self, key, default_value=None):
         if not key.isupper():
@@ -60,3 +73,16 @@ def set(self, key, value):
     def set_from_dict(self, attributes):
         for name, value in attributes.items():
             self.set(name, value)
+
+
+class DefaultSettings(BaseSettings):
+    def __init__(self):
+        super(DefaultSettings, self).__init__(default_settings)
+
+
+class Settings(BaseSettings):
+    def __init__(self, module=None, attributes=None):
+        super(Settings, self).__init__(default_settings, attributes)
+
+        if module:
+            self.add_module(module)

frontera/tests/test_scrapy_settings_adapter.py

@@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+
+from frontera.contrib.scrapy.settings_adapter import ScrapySettingsAdapter
+
+
+def test_fallsback_to_crawler_settings():
+    settings = ScrapySettingsAdapter({'DELAY_ON_EMPTY': 10})
+    assert settings.get('DELAY_ON_EMPTY') == 10
+
+
+def test_frontera_settings_have_precedence_over_crawler_settings():
+    crawler_settings = {'MAX_REQUESTS': 10,
+                        'FRONTERA_SETTINGS': 'frontera.tests.scrapy_spider.frontera.settings'}
+    settings = ScrapySettingsAdapter(crawler_settings)
+    assert settings.get('MAX_REQUESTS') == 5

frontera/tests/test_scrapy_spider.py

@@ -20,4 +20,4 @@ def test_scrapy_spider():
 
     stats = crawler.stats.spider_stats['example']
     assert stats['frontera/crawled_pages_count'] == 5
-    assert spider.callback_calls > 0
+    assert spider.callback_calls > 0

frontera/tests/test_settings.py

@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+
+from frontera.settings import Settings, BaseSettings
+
+
+def test_settings_on_a_python_module_are_loaded():
+    settings = Settings('frontera.tests.scrapy_spider.frontera.settings')
+    assert settings.get('MAX_REQUESTS') == 5
+
+
+def test_settings_passed_as_attributes_can_be_found():
+    settings = Settings(attributes={'SETTING': 'value'})
+    assert settings.get('SETTING') == 'value'
+
+
+def test_fallsback_to_frontera_default_settings():
+    settings = Settings()
+    assert settings.get('MAX_NEXT_REQUESTS') == 0
+
+
+def test_allows_settings_to_be_accessed_by_attribute():
+    settings = Settings()
+    assert settings.MAX_NEXT_REQUESTS == 0
+
+
+def test_settings_attributes_can_be_assigned():
+    settings = Settings()
+    settings.NEW_ATTRIBUTE = 10
+    assert settings.NEW_ATTRIBUTE == 10
+
+
+def test_object_from_loads_settings_from_a_module():
+    module = 'frontera.tests.scrapy_spider.frontera.settings'
+    settings = BaseSettings.object_from(module)
+    assert settings.get('MAX_REQUESTS') == 5
+
+
+def test_new_instance_copies_the_given_instance():
+    settings = Settings()
+    new_instance = BaseSettings.object_from(settings)
+    assert new_instance.MAX_NEXT_REQUESTS == 0
+    assert type(new_instance) == Settings