From 8c7997083fc1455daeda8d13d8037325f4b74909 Mon Sep 17 00:00:00 2001 From: nyov Date: Sun, 12 Jul 2015 16:40:51 +0000 Subject: [PATCH 1/5] lazy-loading for DownloadHandlers --- scrapy/core/downloader/handlers/__init__.py | 49 ++++++++++++++------- tests/test_downloader_handlers.py | 11 ++++- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index ea0842e62..abf01c905 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -11,8 +11,10 @@ from scrapy import signals class DownloadHandlers(object): def __init__(self, crawler): - self._handlers = {} - self._notconfigured = {} + self._crawler_settings = crawler.settings + 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): @@ -20,25 +22,40 @@ class DownloadHandlers(object): # 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 + + dhcls = load_object(self._schemes[scheme]) + try: + dh = dhcls(self._crawler_settings) + except NotConfigured as ex: + 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): diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 131f6edb7..e4d957d8e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -52,6 +52,9 @@ class LoadTestCase(unittest.TestCase): 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) @@ -59,6 +62,9 @@ class LoadTestCase(unittest.TestCase): 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) @@ -66,8 +72,11 @@ class LoadTestCase(unittest.TestCase): 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): From d3804b3439d72e718cf68ff9fc1798bb9232625c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 10 Aug 2015 16:52:36 -0300 Subject: [PATCH 2/5] log errors importing or instanciating handlers --- scrapy/core/downloader/handlers/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index abf01c905..062f674b2 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/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,6 +9,9 @@ from scrapy.utils.misc import load_object from scrapy import signals +logger = logging.getLogger(__name__) + + class DownloadHandlers(object): def __init__(self, crawler): @@ -39,12 +43,16 @@ class DownloadHandlers(object): 'no handler available for that scheme' return None - dhcls = load_object(self._schemes[scheme]) try: + dhcls = load_object(self._schemes[scheme]) dh = dhcls(self._crawler_settings) except NotConfigured as ex: self._notconfigured[scheme] = str(ex) return None + except Exception as ex: + logger.exception() + self._notconfigured[scheme] = str(ex) + return None else: self._handlers[scheme] = dh return self._handlers[scheme] From 15ccf79cad46385041fb8c3a7bf697d9d7ee7c55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 10 Aug 2015 18:18:13 -0300 Subject: [PATCH 3/5] Log errors importing or initializing download handlers --- scrapy/core/downloader/handlers/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 062f674b2..65e5bc21f 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -43,14 +43,16 @@ class DownloadHandlers(object): 'no handler available for that scheme' return None + path = self._schemes[scheme] try: - dhcls = load_object(self._schemes[scheme]) + 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.exception() + logger.exception('Loading "{}" for scheme "{}" handler'\ + .format(path, scheme)) self._notconfigured[scheme] = str(ex) return None else: From eb44152a585a54c5c2ee27c9eaed32aa2f9b15a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 10 Aug 2015 18:25:11 -0300 Subject: [PATCH 4/5] lints --- scrapy/core/downloader/handlers/__init__.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 65e5bc21f..0e732cfe7 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -16,9 +16,9 @@ class DownloadHandlers(object): def __init__(self, crawler): self._crawler_settings = crawler.settings - self._schemes = {} # stores acceptable schemes on instancing - self._handlers = {} # stores instanced handlers for schemes - self._notconfigured = {} # remembers failed handlers + 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): @@ -39,8 +39,7 @@ class DownloadHandlers(object): if scheme in self._notconfigured: return None if scheme not in self._schemes: - self._notconfigured[scheme] = \ - 'no handler available for that scheme' + self._notconfigured[scheme] = 'no handler available for that scheme' return None path = self._schemes[scheme] @@ -51,7 +50,7 @@ class DownloadHandlers(object): self._notconfigured[scheme] = str(ex) return None except Exception as ex: - logger.exception('Loading "{}" for scheme "{}" handler'\ + logger.exception('Loading "{}" for scheme "{}" handler' .format(path, scheme)) self._notconfigured[scheme] = str(ex) return None @@ -64,7 +63,7 @@ class DownloadHandlers(object): handler = self._get_handler(scheme) if not handler: raise NotSupported("Unsupported URL scheme '%s': %s" % - (scheme, self._notconfigured[scheme])) + (scheme, self._notconfigured[scheme])) return handler.download_request(request, spider) @defer.inlineCallbacks From 7717501ab28fa2760da6b1b9be998eb589988821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Tue, 11 Aug 2015 10:38:31 -0300 Subject: [PATCH 5/5] Use log formatting and pass crawler reference --- scrapy/core/downloader/handlers/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 0e732cfe7..6c9514af6 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -15,7 +15,7 @@ logger = logging.getLogger(__name__) class DownloadHandlers(object): def __init__(self, crawler): - self._crawler_settings = crawler.settings + self._crawler = crawler self._schemes = {} # stores acceptable schemes on instancing self._handlers = {} # stores instanced handlers for schemes self._notconfigured = {} # remembers failed handlers @@ -45,13 +45,14 @@ class DownloadHandlers(object): path = self._schemes[scheme] try: dhcls = load_object(path) - dh = dhcls(self._crawler_settings) + dh = dhcls(self._crawler.settings) except NotConfigured as ex: self._notconfigured[scheme] = str(ex) return None except Exception as ex: - logger.exception('Loading "{}" for scheme "{}" handler' - .format(path, scheme)) + 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: