From cd1ad337b173632f0f42534930b893787936c8d7 Mon Sep 17 00:00:00 2001 From: Daniel Grana Date: Sun, 21 Jun 2009 02:54:31 -0300 Subject: [PATCH] Downloader cleanup * remove debug messages * move deactivating of downloads to last callback * simplify calling of download_any function * raises RuntimeError while openinng/closing twice --- scrapy/core/downloader/manager.py | 72 ++++++++-------------------- scrapy/core/downloader/middleware.py | 3 +- 2 files changed, 22 insertions(+), 53 deletions(-) diff --git a/scrapy/core/downloader/manager.py b/scrapy/core/downloader/manager.py index da1422497..a61a258eb 100644 --- a/scrapy/core/downloader/manager.py +++ b/scrapy/core/downloader/manager.py @@ -8,12 +8,9 @@ from twisted.internet import reactor, defer from scrapy.core.exceptions import IgnoreRequest from scrapy.spider import spiders -from scrapy.core.downloader.handlers import download_any from scrapy.core.downloader.middleware import DownloaderMiddlewareManager -from scrapy import log from scrapy.conf import settings from scrapy.utils.defer import chain_deferred, mustbe_deferred -from scrapy.utils.request import request_info class SiteDetails(object): @@ -58,10 +55,7 @@ class Downloader(object): self.engine = engine self.sites = {} self.middleware = DownloaderMiddlewareManager() - self.middleware.download_function = self.enqueue - self.download_function = download_any self.concurrent_domains = settings.getint('CONCURRENT_DOMAINS') - self.debug_mode = settings.getbool('DOWNLOADER_DEBUG') def fetch(self, request, spider): """ Main method to use to request a download @@ -72,29 +66,20 @@ class Downloader(object): """ domain = spider.domain_name site = self.sites[domain] - if not site or site.closed: - if self.debug_mode: - raise IgnoreRequest('Unable to fetch (domain already closed): %s' % request) - else: - raise IgnoreRequest + if site.closed: + raise IgnoreRequest('Can\'t fetch on a closed domain: %s' + request) site.active.add(request) def _deactivate(_): site.active.remove(request) return _ - dwld = self.middleware.download(request, spider) - dwld.addBoth(_deactivate) - return dwld + + return self.middleware.download(request, spider).addBoth(_deactivate) def enqueue(self, request, spider): - """ Enqueue a Request for a effective download from site - """ - domain = spider.domain_name - site = self.sites.get(domain) - if not site or site.closed: - raise IgnoreRequest('Trying to enqueue %s from closed site %s' % (request, domain)) - + """Enqueue a Request for a effective download from site""" deferred = defer.Deferred() + site = self.sites[spider.domain_name] site.queue.append((request, deferred)) self.process_queue(spider) return deferred @@ -119,41 +104,29 @@ class Downloader(object): while site.queue and site.capacity()>0: request, deferred = site.queue.pop(0) - self._download(request, spider, deferred) + self._download(site, request, spider, deferred) if site.closed and site.is_idle(): del self.sites[domain] self.engine.closed_domain(domain) - def _download(self, request, spider, deferred): - if self.debug_mode: - log.msg('Activating %s' % request_info(request), log.DEBUG) - domain = spider.domain_name - site = self.sites.get(domain) + def _download(self, site, request, spider, deferred): site.downloading.add(request) - - def _remove(result): - if self.debug_mode: - log.msg('Deactivating %s' % request_info(request), log.DEBUG) + def _finish(_): site.downloading.remove(request) - return result - - def _finish(result): self.process_queue(spider) - dwld = mustbe_deferred(self.download_function, request, spider) - dwld.addBoth(_remove) + dwld = mustbe_deferred(request, spider) chain_deferred(dwld, deferred) - dwld.addBoth(_finish) + return dwld.addBoth(_finish) def open_domain(self, domain): """Allocate resources to begin processing a domain""" - spider = spiders.fromdomain(domain) - if domain in self.sites: # reopen - self.sites[domain].closed = False - return + if domain in self.sites: + raise RuntimeError('Downloader domain already opened: %s' % domain) # Instanciate site specific handling based on info provided by spider + spider = spiders.fromdomain(domain) delay = getattr(spider, 'download_delay', None) or settings.getint("DOWNLOAD_DELAY", 0) maxcr = getattr(spider, 'max_concurrent_requests', settings.getint('REQUESTS_PER_DOMAIN')) site = SiteDetails(download_delay=delay, max_concurrent_requests=maxcr) @@ -161,16 +134,13 @@ class Downloader(object): def close_domain(self, domain): """Free any resources associated with the given domain""" - if self.debug_mode: - log.msg("Downloader closing domain %s" % domain, log.DEBUG, domain=domain) - site = self.sites.get(domain) - if site: - site.closed = True - spider = spiders.fromdomain(domain) - self.process_queue(spider) - else: - if self.debug_mode: - log.msg('Domain %s already closed' % domain, log.DEBUG, domain=domain) + if domain not in self.sites: + raise RuntimeError('Downloader domain already closed: %s' % domain) + + site = self.sites[domain] + site.closed = True + spider = spiders.fromdomain(domain) + self.process_queue(spider) def needs_backout(self, domain): site = self.sites.get(domain) diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 68f7ef745..c3df8cb83 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -24,7 +24,6 @@ class DownloaderMiddlewareManager(object): self.response_middleware = [] self.exception_middleware = [] self.load() - self.download_function = download_any def _add_middleware(self, mw): if hasattr(mw, 'process_request'): @@ -61,7 +60,7 @@ class DownloaderMiddlewareManager(object): (method.im_self.__class__.__name__, response.__class__.__name__) if response: return response - return self.download_function(request=request, spider=spider) + return download_any(request=request, spider=spider) def process_response(response): assert response is not None, 'Received None in process_response'