From 735c0ceb7890dbea607dd6e4c0a14a4ce2b0afd2 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 9 Sep 2019 16:20:58 -0300 Subject: [PATCH 01/16] Custom name resolver implementing twisted.internet.interfaces.IHostnameResolver --- pytest.ini | 1 + scrapy/crawler.py | 38 ++++++++++++++++++---------------- scrapy/resolver.py | 51 ++++++++++++++++++++++++++-------------------- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/pytest.ini b/pytest.ini index c3f3292bb..a0e89f0a9 100644 --- a/pytest.ini +++ b/pytest.ini @@ -158,6 +158,7 @@ flake8-ignore = scrapy/mail.py E402 E128 E501 E502 scrapy/middleware.py E128 E501 scrapy/pqueues.py E501 + scrapy/resolver.py E501 scrapy/responsetypes.py E128 E501 E305 scrapy/robotstxt.py E501 scrapy/shell.py E501 diff --git a/scrapy/crawler.py b/scrapy/crawler.py index f87e67d93..1350ea84f 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -4,34 +4,36 @@ import signal import warnings from twisted.internet import defer -from zope.interface.verify import verifyClass, DoesNotImplement +from zope.interface.verify import DoesNotImplement, verifyClass -from scrapy import Spider +from scrapy import signals, Spider from scrapy.core.engine import ExecutionEngine -from scrapy.resolver import CachingThreadedResolver -from scrapy.interfaces import ISpiderLoader +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extension import ExtensionManager +from scrapy.interfaces import ISpiderLoader +from scrapy.resolver import CachingHostnameResolver from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_reactor_installed -from scrapy.utils.ossignal import install_shutdown_handlers, signal_names -from scrapy.utils.misc import load_object from scrapy.utils.log import ( - LogCounterHandler, configure_logging, log_scrapy_info, - get_scrapy_root_handler, install_scrapy_root_handler) -from scrapy import signals + configure_logging, + get_scrapy_root_handler, + install_scrapy_root_handler, + log_scrapy_info, + LogCounterHandler, +) +from scrapy.utils.misc import load_object +from scrapy.utils.ossignal import install_shutdown_handlers, signal_names logger = logging.getLogger(__name__) -class Crawler(object): +class Crawler: def __init__(self, spidercls, settings=None): if isinstance(spidercls, Spider): - raise ValueError( - 'The spidercls argument must be a class, not an object') + raise ValueError('The spidercls argument must be a class, not an object') if isinstance(settings, dict) or settings is None: settings = Settings(settings) @@ -110,7 +112,7 @@ class Crawler(object): yield defer.maybeDeferred(self.engine.stop) -class CrawlerRunner(object): +class CrawlerRunner: """ This is a convenient helper class that keeps track of, manages and runs crawlers inside an already setup :mod:`~twisted.internet.reactor`. @@ -303,7 +305,7 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - reactor.installResolver(self._get_dns_resolver()) + reactor.installNameResolver(self._get_dns_resolver()) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) @@ -315,10 +317,10 @@ class CrawlerProcess(CrawlerRunner): cache_size = self.settings.getint('DNSCACHE_SIZE') else: cache_size = 0 - return CachingThreadedResolver( - reactor=reactor, + return CachingHostnameResolver( + resolver=reactor.nameResolver, cache_size=cache_size, - timeout=self.settings.getfloat('DNS_TIMEOUT') + timeout=self.settings.getfloat('DNS_TIMEOUT'), ) def _graceful_stop_reactor(self): diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 4df949015..03964f269 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,32 +1,39 @@ -from twisted.internet import defer -from twisted.internet.base import ThreadedResolver +from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver +from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache -# TODO: cache misses +# TODO: cache misses dnscache = LocalCache(10000) -class CachingThreadedResolver(ThreadedResolver): - def __init__(self, reactor, cache_size, timeout): - super(CachingThreadedResolver, self).__init__(reactor) - dnscache.limit = cache_size +@implementer(IHostnameResolver) +class CachingHostnameResolver(object): + + def __init__(self, resolver, cache_size, timeout): + self.resolver = resolver self.timeout = timeout + dnscache.limit = cache_size - def getHostByName(self, name, timeout=None): - if name in dnscache: - return defer.succeed(dnscache[name]) - # in Twisted<=16.6, getHostByName() is always called with - # a default timeout of 60s (actually passed as (1, 3, 11, 45) tuple), - # so the input argument above is simply overridden - # to enforce Scrapy's DNS_TIMEOUT setting's value - timeout = (self.timeout,) - d = super(CachingThreadedResolver, self).getHostByName(name, timeout) - if dnscache.limit: - d.addCallback(self._cache_result, name) - return d + def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, + addressTypes=None, transportSemantics='TCP'): - def _cache_result(self, result, name): - dnscache[name] = result - return result + @provider(IResolutionReceiver) + class CachingResolutionReceiver(resolutionReceiver): + def resolutionBegan(self, resolution): + super(CachingResolutionReceiver, self).resolutionBegan(resolution) + self.resolution = resolution + + def resolutionComplete(self): + super(CachingResolutionReceiver, self).resolutionComplete() + dnscache[hostName] = self.resolution + + try: + result = dnscache[hostName] + except KeyError: + result = self.resolver.resolveHostName( + CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics + ) + finally: + return result From f1c184631e8cefc191dc0077083138c241bfd6a8 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Dec 2019 17:44:05 -0300 Subject: [PATCH 02/16] Name resolver: timeout --- scrapy/resolver.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 03964f269..8792ed6ab 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,3 +1,4 @@ +from twisted.internet import reactor from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver from zope.interface.declarations import implementer, provider @@ -21,9 +22,14 @@ class CachingHostnameResolver(object): @provider(IResolutionReceiver) class CachingResolutionReceiver(resolutionReceiver): + + def __init__(self, timeout): + self.timeout = timeout + def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution + # reactor.callLater(self.timeout, resolution.cancel) def resolutionComplete(self): super(CachingResolutionReceiver, self).resolutionComplete() @@ -33,7 +39,11 @@ class CachingHostnameResolver(object): result = dnscache[hostName] except KeyError: result = self.resolver.resolveHostName( - CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics + CachingResolutionReceiver(self.timeout), + hostName, + portNumber, + addressTypes, + transportSemantics ) finally: return result From 55babf9acd6cd357f4211a86af01a1f2abe2f0cb Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 15 Jan 2020 12:25:20 -0300 Subject: [PATCH 03/16] Cache resolution only if the DNS request was successful --- scrapy/resolver.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 8792ed6ab..0ba22ed0d 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -10,7 +10,7 @@ dnscache = LocalCache(10000) @implementer(IHostnameResolver) -class CachingHostnameResolver(object): +class CachingHostnameResolver: def __init__(self, resolver, cache_size, timeout): self.resolver = resolver @@ -25,15 +25,21 @@ class CachingHostnameResolver(object): def __init__(self, timeout): self.timeout = timeout + self.resolved = False def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution # reactor.callLater(self.timeout, resolution.cancel) + def addressResolved(self, address): + super(CachingResolutionReceiver, self).addressResolved(address) + self.resolved = True + def resolutionComplete(self): super(CachingResolutionReceiver, self).resolutionComplete() - dnscache[hostName] = self.resolution + if self.resolved: + dnscache[hostName] = self.resolution try: result = dnscache[hostName] From 8c3de288fa2564473f49198b40efdeaa2428f1ca Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 15 Jan 2020 12:31:36 -0300 Subject: [PATCH 04/16] Remove non-working DNS timeout code --- scrapy/crawler.py | 1 - scrapy/resolver.py | 12 +++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 1350ea84f..61851acc3 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -320,7 +320,6 @@ class CrawlerProcess(CrawlerRunner): return CachingHostnameResolver( resolver=reactor.nameResolver, cache_size=cache_size, - timeout=self.settings.getfloat('DNS_TIMEOUT'), ) def _graceful_stop_reactor(self): diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 0ba22ed0d..ddbae61a9 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,4 +1,3 @@ -from twisted.internet import reactor from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver from zope.interface.declarations import implementer, provider @@ -12,9 +11,8 @@ dnscache = LocalCache(10000) @implementer(IHostnameResolver) class CachingHostnameResolver: - def __init__(self, resolver, cache_size, timeout): + def __init__(self, resolver, cache_size): self.resolver = resolver - self.timeout = timeout dnscache.limit = cache_size def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, @@ -23,14 +21,10 @@ class CachingHostnameResolver: @provider(IResolutionReceiver) class CachingResolutionReceiver(resolutionReceiver): - def __init__(self, timeout): - self.timeout = timeout - self.resolved = False - def resolutionBegan(self, resolution): super(CachingResolutionReceiver, self).resolutionBegan(resolution) self.resolution = resolution - # reactor.callLater(self.timeout, resolution.cancel) + self.resolved = False def addressResolved(self, address): super(CachingResolutionReceiver, self).addressResolved(address) @@ -45,7 +39,7 @@ class CachingHostnameResolver: result = dnscache[hostName] except KeyError: result = self.resolver.resolveHostName( - CachingResolutionReceiver(self.timeout), + CachingResolutionReceiver(), hostName, portNumber, addressTypes, From e69cf415c8326349c871fb23faba2ed822aa08ee Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 03:58:07 -0300 Subject: [PATCH 05/16] Ability to choose name resolver --- scrapy/crawler.py | 14 ++------ scrapy/resolver.py | 54 ++++++++++++++++++++++++++++- scrapy/settings/default_settings.py | 1 + 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 61851acc3..c5351c08f 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -305,23 +305,13 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - reactor.installNameResolver(self._get_dns_resolver()) + resolver_class = load_object(self.settings["DNS_RESOLVER"]) + resolver_class.install(reactor, self.settings) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) reactor.run(installSignalHandlers=False) # blocking call - def _get_dns_resolver(self): - from twisted.internet import reactor - if self.settings.getbool('DNSCACHE_ENABLED'): - cache_size = self.settings.getint('DNSCACHE_SIZE') - else: - cache_size = 0 - return CachingHostnameResolver( - resolver=reactor.nameResolver, - cache_size=cache_size, - ) - def _graceful_stop_reactor(self): d = self.stop() d.addBoth(self._stop_reactor) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index ddbae61a9..2bef9f1b8 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -1,4 +1,6 @@ -from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver +from twisted.internet import defer +from twisted.internet.base import ThreadedResolver +from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, IResolverSimple from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache @@ -8,8 +10,58 @@ from scrapy.utils.datatypes import LocalCache dnscache = LocalCache(10000) +@implementer(IResolverSimple) +class CachingThreadedResolver(ThreadedResolver): + """ + Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests + """ + + @classmethod + def install(cls, reactor, settings): + if settings.getbool('DNSCACHE_ENABLED'): + cache_size = settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + resolver = cls(reactor, cache_size, settings.getfloat('DNS_TIMEOUT')) + reactor.installResolver(resolver) + + def __init__(self, reactor, cache_size, timeout): + super(CachingThreadedResolver, self).__init__(reactor) + dnscache.limit = cache_size + self.timeout = timeout + + def getHostByName(self, name, timeout=None): + if name in dnscache: + return defer.succeed(dnscache[name]) + # in Twisted<=16.6, getHostByName() is always called with + # a default timeout of 60s (actually passed as (1, 3, 11, 45) tuple), + # so the input argument above is simply overridden + # to enforce Scrapy's DNS_TIMEOUT setting's value + timeout = (self.timeout,) + d = super(CachingThreadedResolver, self).getHostByName(name, timeout) + if dnscache.limit: + d.addCallback(self._cache_result, name) + return d + + def _cache_result(self, result, name): + dnscache[name] = result + return result + + @implementer(IHostnameResolver) class CachingHostnameResolver: + """ + Experimental caching resolver, supporting IPv4 and IPv6 + """ + + @classmethod + def install(cls, reactor, settings): + if settings.getbool('DNSCACHE_ENABLED'): + cache_size = settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + resolver = cls(reactor.nameResolver, cache_size) + reactor.installNameResolver(resolver) def __init__(self, resolver, cache_size): self.resolver = resolver diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d03fd37b0..46ed3be96 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -60,6 +60,7 @@ DEPTH_PRIORITY = 0 DNSCACHE_ENABLED = True DNSCACHE_SIZE = 10000 +DNS_RESOLVER = 'scrapy.resolver.CachingThreadedResolver' DNS_TIMEOUT = 60 DOWNLOAD_DELAY = 0 From 0f155b059a43b2cc48149a26c6584910038a23f1 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 04:27:13 -0300 Subject: [PATCH 06/16] Make Flake8 happy (remove unused import) --- scrapy/crawler.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index c5351c08f..5658e264b 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -11,7 +11,6 @@ from scrapy.core.engine import ExecutionEngine from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extension import ExtensionManager from scrapy.interfaces import ISpiderLoader -from scrapy.resolver import CachingHostnameResolver from scrapy.settings import overridden_settings, Settings from scrapy.signalmanager import SignalManager from scrapy.utils.asyncio import install_asyncio_reactor, is_asyncio_reactor_installed From f45b4c7f8d191c6855c12f3497ece6b6121289df Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 10:09:34 -0300 Subject: [PATCH 07/16] from_crawler support for name resolvers --- scrapy/crawler.py | 2 +- scrapy/resolver.py | 48 ++++++++++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 5658e264b..4531c3aac 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -305,7 +305,7 @@ class CrawlerProcess(CrawlerRunner): d.addBoth(self._stop_reactor) resolver_class = load_object(self.settings["DNS_RESOLVER"]) - resolver_class.install(reactor, self.settings) + resolver_class.install_on_reactor(reactor, crawler=self) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 2bef9f1b8..792563e79 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -4,6 +4,7 @@ from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache +from scrapy.utils.misc import create_instance # TODO: cache misses @@ -13,23 +14,27 @@ dnscache = LocalCache(10000) @implementer(IResolverSimple) class CachingThreadedResolver(ThreadedResolver): """ - Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests + Default caching resolver. IPv4 only, supports setting a timeout value for DNS requests. """ - @classmethod - def install(cls, reactor, settings): - if settings.getbool('DNSCACHE_ENABLED'): - cache_size = settings.getint('DNSCACHE_SIZE') - else: - cache_size = 0 - resolver = cls(reactor, cache_size, settings.getfloat('DNS_TIMEOUT')) - reactor.installResolver(resolver) - def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size self.timeout = timeout + @classmethod + def from_crawler(cls, crawler, reactor): + if crawler.settings.getbool('DNSCACHE_ENABLED'): + cache_size = crawler.settings.getint('DNSCACHE_SIZE') + else: + cache_size = 0 + return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) + + @classmethod + def install_on_reactor(cls, reactor, crawler): + resolver = create_instance(cls, None, crawler, reactor=reactor) + reactor.installResolver(resolver) + def getHostByName(self, name, timeout=None): if name in dnscache: return defer.succeed(dnscache[name]) @@ -51,21 +56,26 @@ class CachingThreadedResolver(ThreadedResolver): @implementer(IHostnameResolver) class CachingHostnameResolver: """ - Experimental caching resolver, supporting IPv4 and IPv6 + Experimental caching resolver. Resolves IPv4 and IPv6 addresses, + does not support setting a timeout value for DNS requests. """ + def __init__(self, reactor, cache_size): + self.resolver = reactor.nameResolver + dnscache.limit = cache_size + @classmethod - def install(cls, reactor, settings): - if settings.getbool('DNSCACHE_ENABLED'): - cache_size = settings.getint('DNSCACHE_SIZE') + def from_crawler(cls, crawler, reactor): + if crawler.settings.getbool('DNSCACHE_ENABLED'): + cache_size = crawler.settings.getint('DNSCACHE_SIZE') else: cache_size = 0 - resolver = cls(reactor.nameResolver, cache_size) - reactor.installNameResolver(resolver) + return cls(reactor, cache_size) - def __init__(self, resolver, cache_size): - self.resolver = resolver - dnscache.limit = cache_size + @classmethod + def install_on_reactor(cls, reactor, crawler): + resolver = create_instance(cls, None, crawler, reactor=reactor) + reactor.installNameResolver(resolver) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): From 3cfa73b8b12dfe2dd1364af856d2ec46a486b48d Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 18:01:18 -0300 Subject: [PATCH 08/16] Name resolvers: install_on_reactor as instance method --- scrapy/crawler.py | 5 +++-- scrapy/resolver.py | 13 ++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 4531c3aac..0cef75a6a 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -21,7 +21,7 @@ from scrapy.utils.log import ( log_scrapy_info, LogCounterHandler, ) -from scrapy.utils.misc import load_object +from scrapy.utils.misc import create_instance, load_object from scrapy.utils.ossignal import install_shutdown_handlers, signal_names @@ -305,7 +305,8 @@ class CrawlerProcess(CrawlerRunner): d.addBoth(self._stop_reactor) resolver_class = load_object(self.settings["DNS_RESOLVER"]) - resolver_class.install_on_reactor(reactor, crawler=self) + resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) + resolver.install_on_reactor(reactor) tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 792563e79..2b97603da 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -4,7 +4,6 @@ from twisted.internet.interfaces import IHostnameResolver, IResolutionReceiver, from zope.interface.declarations import implementer, provider from scrapy.utils.datatypes import LocalCache -from scrapy.utils.misc import create_instance # TODO: cache misses @@ -30,10 +29,8 @@ class CachingThreadedResolver(ThreadedResolver): cache_size = 0 return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) - @classmethod - def install_on_reactor(cls, reactor, crawler): - resolver = create_instance(cls, None, crawler, reactor=reactor) - reactor.installResolver(resolver) + def install_on_reactor(self, reactor): + reactor.installResolver(self) def getHostByName(self, name, timeout=None): if name in dnscache: @@ -72,10 +69,8 @@ class CachingHostnameResolver: cache_size = 0 return cls(reactor, cache_size) - @classmethod - def install_on_reactor(cls, reactor, crawler): - resolver = create_instance(cls, None, crawler, reactor=reactor) - reactor.installNameResolver(resolver) + def install_on_reactor(self, reactor): + reactor.installNameResolver(self) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): From 1040f581ec1ba3bcf8f08edb824d59c0e4a93700 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 20:14:52 -0300 Subject: [PATCH 09/16] Name resolvers: do not pass the reactor to the install method --- scrapy/crawler.py | 2 +- scrapy/resolver.py | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 0cef75a6a..35c6b7716 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -306,7 +306,7 @@ class CrawlerProcess(CrawlerRunner): resolver_class = load_object(self.settings["DNS_RESOLVER"]) resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) - resolver.install_on_reactor(reactor) + resolver.install_on_reactor() tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint('REACTOR_THREADPOOL_MAXSIZE')) reactor.addSystemEventTrigger('before', 'shutdown', self.stop) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 2b97603da..7c776f75e 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -19,6 +19,7 @@ class CachingThreadedResolver(ThreadedResolver): def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size + self.reactor = reactor self.timeout = timeout @classmethod @@ -29,8 +30,8 @@ class CachingThreadedResolver(ThreadedResolver): cache_size = 0 return cls(reactor, cache_size, crawler.settings.getfloat('DNS_TIMEOUT')) - def install_on_reactor(self, reactor): - reactor.installResolver(self) + def install_on_reactor(self,): + self.reactor.installResolver(self) def getHostByName(self, name, timeout=None): if name in dnscache: @@ -58,7 +59,8 @@ class CachingHostnameResolver: """ def __init__(self, reactor, cache_size): - self.resolver = reactor.nameResolver + self.reactor = reactor + self.original_resolver = reactor.nameResolver dnscache.limit = cache_size @classmethod @@ -69,8 +71,8 @@ class CachingHostnameResolver: cache_size = 0 return cls(reactor, cache_size) - def install_on_reactor(self, reactor): - reactor.installNameResolver(self) + def install_on_reactor(self): + self.reactor.installNameResolver(self) def resolveHostName(self, resolutionReceiver, hostName, portNumber=0, addressTypes=None, transportSemantics='TCP'): @@ -95,7 +97,7 @@ class CachingHostnameResolver: try: result = dnscache[hostName] except KeyError: - result = self.resolver.resolveHostName( + result = self.original_resolver.resolveHostName( CachingResolutionReceiver(), hostName, portNumber, From 90e3bd8715701aeea9187837bc32925e3225288f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 20:32:40 -0300 Subject: [PATCH 10/16] [test] Name resolvers --- tests/CrawlerProcess/alternative_name_resolver.py | 15 +++++++++++++++ tests/CrawlerProcess/default_name_resolver.py | 12 ++++++++++++ tests/test_crawler.py | 12 ++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 tests/CrawlerProcess/alternative_name_resolver.py create mode 100644 tests/CrawlerProcess/default_name_resolver.py diff --git a/tests/CrawlerProcess/alternative_name_resolver.py b/tests/CrawlerProcess/alternative_name_resolver.py new file mode 100644 index 000000000..2c466da04 --- /dev/null +++ b/tests/CrawlerProcess/alternative_name_resolver.py @@ -0,0 +1,15 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class IPv6Spider(scrapy.Spider): + name = "ipv6_spider" + start_urls = ["http://[::1]"] + + +process = CrawlerProcess(settings={ + "RETRY_ENABLED": False, + "DNS_RESOLVER": "scrapy.resolver.CachingHostnameResolver", +}) +process.crawl(IPv6Spider) +process.start() diff --git a/tests/CrawlerProcess/default_name_resolver.py b/tests/CrawlerProcess/default_name_resolver.py new file mode 100644 index 000000000..60d91b68b --- /dev/null +++ b/tests/CrawlerProcess/default_name_resolver.py @@ -0,0 +1,12 @@ +import scrapy +from scrapy.crawler import CrawlerProcess + + +class IPv6Spider(scrapy.Spider): + name = "ipv6_spider" + start_urls = ["http://[::1]"] + + +process = CrawlerProcess(settings={"RETRY_ENABLED": False}) +process.crawl(IPv6Spider) +process.start() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index fce60ca37..d85f2bc41 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -305,3 +305,15 @@ class CrawlerProcessSubprocess(unittest.TestCase): log = self.run_script('asyncio_enabled_reactor.py') self.assertIn('Spider closed (finished)', log) self.assertIn("DEBUG: Asyncio reactor is installed", log) + + def test_default_name_resolver(self): + log = self.run_script('default_name_resolver.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("twisted.internet.error.DNSLookupError: DNS lookup failed: no results for hostname lookup: ::1.", log) + self.assertIn("'downloader/exception_type_count/twisted.internet.error.DNSLookupError': 1,", log) + + def test_alternative_name_resolver(self): + log = self.run_script('alternative_name_resolver.py') + self.assertIn('Spider closed (finished)', log) + self.assertIn("twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.", log) + self.assertIn("'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1,", log) From d487498cff893d2c507ef5ed0957f101dc3ab4a9 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 22:02:01 -0300 Subject: [PATCH 11/16] Update name resolvers tests --- tests/test_crawler.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index d85f2bc41..14f4f8418 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -306,14 +306,20 @@ class CrawlerProcessSubprocess(unittest.TestCase): self.assertIn('Spider closed (finished)', log) self.assertIn("DEBUG: Asyncio reactor is installed", log) - def test_default_name_resolver(self): + def test_ipv6_default_name_resolver(self): log = self.run_script('default_name_resolver.py') self.assertIn('Spider closed (finished)', log) self.assertIn("twisted.internet.error.DNSLookupError: DNS lookup failed: no results for hostname lookup: ::1.", log) self.assertIn("'downloader/exception_type_count/twisted.internet.error.DNSLookupError': 1,", log) - def test_alternative_name_resolver(self): + def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertIn("twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 111: Connection refused.", log) - self.assertIn("'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1,", log) + self.assertTrue(any( + "twisted.internet.error.ConnectionRefusedError" in log, + "twisted.internet.error.ConnectError" in log, + )) + self.assertTrue(any( + "'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1," in log, + "'downloader/exception_type_count/twisted.internet.error.ConnectError': 1," in log, + )) From dee420a69cb5c8319a7df99a5765c5ca90a7063e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 16 Jan 2020 23:48:16 -0300 Subject: [PATCH 12/16] Fix name resolvers tests --- tests/test_crawler.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 14f4f8418..0ce0674de 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -315,11 +315,11 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertTrue(any( + self.assertTrue(any([ "twisted.internet.error.ConnectionRefusedError" in log, "twisted.internet.error.ConnectError" in log, - )) - self.assertTrue(any( + ])) + self.assertTrue(any([ "'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1," in log, "'downloader/exception_type_count/twisted.internet.error.ConnectError': 1," in log, - )) + ])) From 41f7ebf3add25b2251082b206b062c00d1daba58 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 17 Jan 2020 12:40:49 -0300 Subject: [PATCH 13/16] CachingThreadedResolver: No need to store the reactor as an instance attribute It's already done in the parent class --- scrapy/resolver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 7c776f75e..7751f3796 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -19,7 +19,6 @@ class CachingThreadedResolver(ThreadedResolver): def __init__(self, reactor, cache_size, timeout): super(CachingThreadedResolver, self).__init__(reactor) dnscache.limit = cache_size - self.reactor = reactor self.timeout = timeout @classmethod From 302d3f552b5a930a43fddac8eb8c5f769d8748bc Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 18 Jan 2020 01:41:57 -0300 Subject: [PATCH 14/16] [doc] DNS_RESOLVER setting --- docs/topics/settings.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c02f877fc..292eaea74 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -397,6 +397,19 @@ Default: ``10000`` DNS in-memory cache size. +.. setting:: DNS_RESOLVER + +DNS_RESOLVER +------------ + +Default: ``'scrapy.resolver.CachingThreadedResolver'`` + +The class to be used to resolve DNS names. The default ``scrapy.resolver.CachingThreadedResolver`` +supports specifying a timeout for DNS requests via the :setting:`DNS_TIMEOUT` setting, +but works only with IPv4 addresses. Scrapy provides an alternative resolver, +``scrapy.resolver.CachingHostnameResolver``, which supports IPv4/IPv6 addresses but does not +take the :setting:`DNS_TIMEOUT` setting into account. + .. setting:: DNS_TIMEOUT DNS_TIMEOUT From b471765d40a2a8a94f90af46ac6903e3786aec5f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 18 Jan 2020 01:52:29 -0300 Subject: [PATCH 15/16] [doc] FAQ entry about the IPv6 and the DNS_RESOLVER setting --- docs/faq.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/faq.rst b/docs/faq.rst index aae2411e0..b789a8cdb 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -353,6 +353,13 @@ method for this purpose. For example:: for _ in range(item['multiply_by']): yield deepcopy(item) +Does Scrapy support IPv6 addresses? +----------------------------------- + +Yes, by setting :setting:`DNS_RESOLVER` to ``scrapy.resolver.CachingHostnameResolver``. +Note that by doing so, you lose the ability to set a specific timeout for DNS requests +(the value of the :setting:`DNS_TIMEOUT` setting is ignored). + .. _user agents: https://en.wikipedia.org/wiki/User_agent .. _LIFO: https://en.wikipedia.org/wiki/Stack_(abstract_data_type) From 9899414300b4a6491ea17418c3403aed08e0faf6 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 23 Jan 2020 18:06:59 -0300 Subject: [PATCH 16/16] Name resolver: return result directly --- scrapy/resolver.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scrapy/resolver.py b/scrapy/resolver.py index 7751f3796..554a3a14d 100644 --- a/scrapy/resolver.py +++ b/scrapy/resolver.py @@ -94,14 +94,12 @@ class CachingHostnameResolver: dnscache[hostName] = self.resolution try: - result = dnscache[hostName] + return dnscache[hostName] except KeyError: - result = self.original_resolver.resolveHostName( + return self.original_resolver.resolveHostName( CachingResolutionReceiver(), hostName, portNumber, addressTypes, transportSemantics ) - finally: - return result