Merge pull request scrapy#1421 from dangra/nyov/lazyload-downloadhandlers

lazy-load downloadhandlers (continuation of scrapy#1357)

Author: kmike

scrapy/core/downloader/handlers/__init__.py

@@ -1,5 +1,6 @@
 """Download handlers for different schemes"""
 
+import logging
 from twisted.internet import defer
 import six
 from scrapy.exceptions import NotSupported, NotConfigured
@@ -8,37 +9,63 @@
 from scrapy import signals
 
 
+logger = logging.getLogger(__name__)
+
+
 class DownloadHandlers(object):
 
     def __init__(self, crawler):
-        self._handlers = {}
-        self._notconfigured = {}
+        self._crawler = crawler
+        self._schemes = {}  # stores acceptable schemes on instancing
+        self._handlers = {}  # stores instanced handlers for schemes
+        self._notconfigured = {}  # remembers failed handlers
         handlers = crawler.settings.get('DOWNLOAD_HANDLERS_BASE')
         handlers.update(crawler.settings.get('DOWNLOAD_HANDLERS', {}))
         for scheme, clspath in six.iteritems(handlers):
             # Allow to disable a handler just like any other
             # component (extension, middleware, etc).
             if clspath is None:
                 continue
-            cls = load_object(clspath)
-            try:
-                dh = cls(crawler.settings)
-            except NotConfigured as ex:
-                self._notconfigured[scheme] = str(ex)
-            else:
-                self._handlers[scheme] = dh
+            self._schemes[scheme] = clspath
 
         crawler.signals.connect(self._close, signals.engine_stopped)
 
+    def _get_handler(self, scheme):
+        """Lazy-load the downloadhandler for a scheme
+        only on the first request for that scheme.
+        """
+        if scheme in self._handlers:
+            return self._handlers[scheme]
+        if scheme in self._notconfigured:
+            return None
+        if scheme not in self._schemes:
+            self._notconfigured[scheme] = 'no handler available for that scheme'
+            return None
+
+        path = self._schemes[scheme]
+        try:
+            dhcls = load_object(path)
+            dh = dhcls(self._crawler.settings)
+        except NotConfigured as ex:
+            self._notconfigured[scheme] = str(ex)
+            return None
+        except Exception as ex:
+            logger.error('Loading "%(clspath)s" for scheme "%(scheme)s"',
+                         {"clspath": path, "scheme": scheme},
+                         exc_info=True,  extra={'crawler': self._crawler})
+            self._notconfigured[scheme] = str(ex)
+            return None
+        else:
+            self._handlers[scheme] = dh
+        return self._handlers[scheme]
+
     def download_request(self, request, spider):
         scheme = urlparse_cached(request).scheme
-        try:
-            handler = self._handlers[scheme].download_request
-        except KeyError:
-            msg = self._notconfigured.get(scheme, \
-                    'no handler available for that scheme')
-            raise NotSupported("Unsupported URL scheme '%s': %s" % (scheme, msg))
-        return handler(request, spider)
+        handler = self._get_handler(scheme)
+        if not handler:
+            raise NotSupported("Unsupported URL scheme '%s': %s" %
+                               (scheme, self._notconfigured[scheme]))
+        return handler.download_request(request, spider)
 
     @defer.inlineCallbacks
     def _close(self, *_a, **_kw):

tests/test_downloader_handlers.py

@@ -52,22 +52,31 @@ def test_enabled_handler(self):
         handlers = {'scheme': 'tests.test_downloader_handlers.DummyDH'}
         crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers})
         dh = DownloadHandlers(crawler)
+        self.assertIn('scheme', dh._schemes)
+        for scheme in handlers: # force load handlers
+            dh._get_handler(scheme)
         self.assertIn('scheme', dh._handlers)
         self.assertNotIn('scheme', dh._notconfigured)
 
     def test_not_configured_handler(self):
         handlers = {'scheme': 'tests.test_downloader_handlers.OffDH'}
         crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers})
         dh = DownloadHandlers(crawler)
+        self.assertIn('scheme', dh._schemes)
+        for scheme in handlers: # force load handlers
+            dh._get_handler(scheme)
         self.assertNotIn('scheme', dh._handlers)
         self.assertIn('scheme', dh._notconfigured)
 
     def test_disabled_handler(self):
         handlers = {'scheme': None}
         crawler = get_crawler(settings_dict={'DOWNLOAD_HANDLERS': handlers})
         dh = DownloadHandlers(crawler)
+        self.assertNotIn('scheme', dh._schemes)
+        for scheme in handlers: # force load handlers
+            dh._get_handler(scheme)
         self.assertNotIn('scheme', dh._handlers)
-        self.assertNotIn('scheme', dh._notconfigured)
+        self.assertIn('scheme', dh._notconfigured)
 
 
 class FileTestCase(unittest.TestCase):