From ed4aec187f7868a7fcc7c7279489f02be79a8cf9 Mon Sep 17 00:00:00 2001 From: Pablo Hoffman Date: Wed, 22 Sep 2010 16:09:13 -0300 Subject: [PATCH] Ported code to use new unified access to spider settings, keeping backwards compatibility for old spider attributes. Refs #245 --- docs/faq.rst | 2 +- docs/topics/commands.rst | 10 ++--- docs/topics/downloader-middleware.rst | 10 ++--- docs/topics/settings.rst | 4 +- .../downloadermiddleware/defaultheaders.py | 10 +---- .../downloadermiddleware/downloadtimeout.py | 6 ++- .../contrib/downloadermiddleware/robotstxt.py | 3 +- .../contrib/downloadermiddleware/useragent.py | 10 +++-- scrapy/core/downloader/__init__.py | 25 ++++++----- scrapy/core/downloader/webclient.py | 6 +-- ...est_downloadermiddleware_defaultheaders.py | 37 +++++++++------- ...st_downloadermiddleware_downloadtimeout.py | 39 ++++++++-------- .../test_downloadermiddleware_useragent.py | 44 ++++++++++--------- scrapy/tests/test_engine.py | 6 +-- scrapy/utils/deprecate.py | 9 ++++ scrapy/utils/test.py | 17 +++++++ 16 files changed, 129 insertions(+), 109 deletions(-) create mode 100644 scrapy/utils/deprecate.py diff --git a/docs/faq.rst b/docs/faq.rst index c0b76b21f..966de7456 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -171,7 +171,7 @@ higher) in your spider:: name = 'myspider' - download_delay = 2 + DOWNLOAD_DELAY = 2 # [ ... rest of the spider code ... ] diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index d58973eb6..6339a4607 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -73,10 +73,10 @@ information on which commands must be run from inside projects, and which not. Also keep in mind that some commands may have slightly different behaviours when running them from inside projects. For example, the fetch command will use -spider-overridden behaviours (such as custom ``user_agent`` attribute) if the -url being fetched is associated with some specific spider. This is intentional, -as the ``fetch`` command is meant to be used to check how spiders are -downloading pages. +spider-overridden behaviours (such as custom :settings:`USER_AGENT` per-spider +setting) if the url being fetched is associated with some specific spider. This +is intentional, as the ``fetch`` command is meant to be used to check how +spiders are downloading pages. .. _topics-commands-ref: @@ -243,7 +243,7 @@ Downloads the given URL using the Scrapy downloader and writes the contents to standard output. The interesting thing about this command is that it fetches the page how the -the spider would download it. For example, if the spider has an ``user_agent`` +the spider would download it. For example, if the spider has an ``USER_AGENT`` attribute which overrides the User Agent, it will use that one. So this command can be used to "see" how your spider would fetch certain page. diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index d4139ff54..eb7a33009 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -177,9 +177,7 @@ DefaultHeadersMiddleware .. class:: DefaultHeadersMiddleware This middleware sets all default requests headers specified in the - :setting:`DEFAULT_REQUEST_HEADERS` setting plus those found in spider - ``default_request_headers`` attribute. Spider headers has precedence over - global headers. + :setting:`DEFAULT_REQUEST_HEADERS` setting. DownloadTimeoutMiddleware ------------------------- @@ -189,10 +187,8 @@ DownloadTimeoutMiddleware .. class:: DownloadTimeoutMiddleware - This middleware sets download timeout for requests based on - `download_timeout` spider attribute. It doesn't override timeout if - `download_timeout` is already set in request meta. Otherwise, - :setting:`DOWNLOAD_TIMEOUT` setting is used as default download timeout. + This middleware sets the download timeout for requests specified in the + :setting:`DOWNLOAD_TIMEOUT` setting. HttpAuthMiddleware ------------------ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index a63eb1f62..e3b452419 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -398,9 +398,7 @@ setting (which is enabled by default). By default, Scrapy doesn't wait a fixed amount of time between requests, but uses a random interval between 0.5 and 1.5 * :setting:`DOWNLOAD_DELAY`. -Another way to change the download delay (per spider, instead of globally) is -by using the ``download_delay`` spider attribute, which takes more precedence -than this setting. +You can also change this setting per spider. .. setting:: DOWNLOAD_HANDLERS diff --git a/scrapy/contrib/downloadermiddleware/defaultheaders.py b/scrapy/contrib/downloadermiddleware/defaultheaders.py index 1bef04cdd..61a05fc50 100644 --- a/scrapy/contrib/downloadermiddleware/defaultheaders.py +++ b/scrapy/contrib/downloadermiddleware/defaultheaders.py @@ -10,18 +10,10 @@ from scrapy.utils.python import WeakKeyCache class DefaultHeadersMiddleware(object): def __init__(self, settings=conf.settings): - self.global_default_headers = settings.get('DEFAULT_REQUEST_HEADERS') self._headers = WeakKeyCache(self._default_headers) def _default_headers(self, spider): - headers = dict(self.global_default_headers) - spider_headers = getattr(spider, 'default_request_headers', None) or {} - for k, v in spider_headers.iteritems(): - if v: - headers[k] = v - else: - headers.pop(k, None) - return headers.items() + return spider.settings.get('DEFAULT_REQUEST_HEADERS').items() def process_request(self, request, spider): for k, v in self._headers[spider]: diff --git a/scrapy/contrib/downloadermiddleware/downloadtimeout.py b/scrapy/contrib/downloadermiddleware/downloadtimeout.py index 0c250d4c4..01ccf7bfb 100644 --- a/scrapy/contrib/downloadermiddleware/downloadtimeout.py +++ b/scrapy/contrib/downloadermiddleware/downloadtimeout.py @@ -4,6 +4,7 @@ Download timeout middleware See documentation in docs/topics/downloader-middleware.rst """ from scrapy.utils.python import WeakKeyCache +from scrapy.utils import deprecate class DownloadTimeoutMiddleware(object): @@ -12,7 +13,10 @@ class DownloadTimeoutMiddleware(object): self._cache = WeakKeyCache(self._download_timeout) def _download_timeout(self, spider): - return getattr(spider, "download_timeout", None) + if hasattr(spider, 'download_timeout'): + deprecate.attribute(spider, 'download_timeout', 'DOWNLOAD_TIMEOUT') + return spider.download_timeout + return spider.settings.getint('DOWNLOAD_TIMEOUT') def process_request(self, request, spider): timeout = self._cache[spider] diff --git a/scrapy/contrib/downloadermiddleware/robotstxt.py b/scrapy/contrib/downloadermiddleware/robotstxt.py index fe47317f7..314a9586d 100644 --- a/scrapy/contrib/downloadermiddleware/robotstxt.py +++ b/scrapy/contrib/downloadermiddleware/robotstxt.py @@ -54,8 +54,7 @@ class RobotsTxtMiddleware(object): def spider_opened(self, spider): self._spider_netlocs[spider] = set() - self._useragents[spider] = getattr(spider, 'user_agent', None) \ - or settings['USER_AGENT'] + self._useragents[spider] = spider.settings['USER_AGENT'] def spider_closed(self, spider): for netloc in self._spider_netlocs[spider]: diff --git a/scrapy/contrib/downloadermiddleware/useragent.py b/scrapy/contrib/downloadermiddleware/useragent.py index 56df2b999..752a8577b 100644 --- a/scrapy/contrib/downloadermiddleware/useragent.py +++ b/scrapy/contrib/downloadermiddleware/useragent.py @@ -1,18 +1,20 @@ """Set User-Agent header per spider or use a default value from settings""" -from scrapy.conf import settings from scrapy.utils.python import WeakKeyCache +from scrapy.utils import deprecate class UserAgentMiddleware(object): """This middleware allows spiders to override the user_agent""" - def __init__(self, settings=settings): + def __init__(self): self.cache = WeakKeyCache(self._user_agent) - self.default_useragent = settings.get('USER_AGENT') def _user_agent(self, spider): - return getattr(spider, 'user_agent', None) or self.default_useragent + if hasattr(spider, 'user_agent'): + deprecate.attribute(spider, 'user_agent', 'USER_AGENT') + return spider.user_agent + return spider.settings['USER_AGENT'] def process_request(self, request, spider): ua = self.cache[spider] diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 38dcabf55..8eb99d9d0 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -12,6 +12,7 @@ from scrapy.exceptions import IgnoreRequest from scrapy.conf import settings from scrapy.utils.defer import mustbe_deferred from scrapy.utils.signal import send_catch_log +from scrapy.utils import deprecate from scrapy import signals from scrapy import log from .middleware import DownloaderMiddlewareManager @@ -21,18 +22,21 @@ from .handlers import DownloadHandlers class SpiderInfo(object): """Simple class to keep information and state for each open spider""" - def __init__(self, download_delay=None, max_concurrent_requests=None): - if download_delay is None: - self._download_delay = settings.getfloat('DOWNLOAD_DELAY') + def __init__(self, spider): + if hasattr(spider, 'download_delay'): + deprecate.attribute(spider, 'download_delay', 'DOWNLOAD_DELAY') + self._download_delay = spider.download_delay else: - self._download_delay = float(download_delay) + self._download_delay = spider.settings.getfloat('DOWNLOAD_DELAY') if self._download_delay: self.max_concurrent_requests = 1 - elif max_concurrent_requests is None: - self.max_concurrent_requests = settings.getint('CONCURRENT_REQUESTS_PER_SPIDER') else: - self.max_concurrent_requests = max_concurrent_requests - if self._download_delay and settings.getbool('RANDOMIZE_DOWNLOAD_DELAY'): + if hasattr(spider, 'max_concurrent_requests'): + deprecate.attribute(spider, 'max_concurrent_requests', 'CONCURRENT_REQUESTS_PER_SPIDER') + self.max_concurrent_requests = spider.max_concurrent_requests + else: + self.max_concurrent_requests = spider.settings.getint('CONCURRENT_REQUESTS_PER_SPIDER') + if self._download_delay and spider.settings.getbool('RANDOMIZE_DOWNLOAD_DELAY'): # same policy as wget --random-wait self.random_delay_interval = (0.5*self._download_delay, \ 1.5*self._download_delay) @@ -178,10 +182,7 @@ class Downloader(object): def open_spider(self, spider): """Allocate resources to begin processing a spider""" assert spider not in self.sites, "Spider already opened: %s" % spider - self.sites[spider] = SpiderInfo( - download_delay=getattr(spider, 'download_delay', None), - max_concurrent_requests=getattr(spider, 'max_concurrent_requests', None) - ) + self.sites[spider] = SpiderInfo(spider) def close_spider(self, spider): """Free any resources associated with the given spider""" diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index 0aeb861e0..b08e8ad81 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -8,10 +8,6 @@ from twisted.internet import defer from scrapy.http import Headers from scrapy.utils.httpobj import urlparse_cached from scrapy.core.downloader.responsetypes import responsetypes -from scrapy.conf import settings - - -DOWNLOAD_TIMEOUT = settings.getint('DOWNLOAD_TIMEOUT') def _parsed_url_args(parsed): @@ -89,7 +85,7 @@ class ScrapyHTTPClientFactory(HTTPClientFactory): followRedirect = False afterFoundGet = False - def __init__(self, request, timeout=DOWNLOAD_TIMEOUT): + def __init__(self, request, timeout=180): self.url = urldefrag(request.url)[0] self.method = request.method self.body = request.body or None diff --git a/scrapy/tests/test_downloadermiddleware_defaultheaders.py b/scrapy/tests/test_downloadermiddleware_defaultheaders.py index 5dfe5546c..cc227e20c 100644 --- a/scrapy/tests/test_downloadermiddleware_defaultheaders.py +++ b/scrapy/tests/test_downloadermiddleware_defaultheaders.py @@ -4,39 +4,44 @@ from scrapy.conf import settings from scrapy.contrib.downloadermiddleware.defaultheaders import DefaultHeadersMiddleware from scrapy.http import Request from scrapy.spider import BaseSpider +from scrapy.utils.test import get_crawler class TestDefaultHeadersMiddleware(TestCase): - def setUp(self): - self.spider = BaseSpider('foo') - self.mw = DefaultHeadersMiddleware() - self.default_request_headers = dict([(k, [v]) for k, v in \ - settings.get('DEFAULT_REQUEST_HEADERS').iteritems()]) + def get_defaults_spider_mw(self): + crawler = get_crawler() + spider = BaseSpider('foo') + spider.set_crawler(crawler) + defaults = dict([(k, [v]) for k, v in \ + crawler.settings.get('DEFAULT_REQUEST_HEADERS').iteritems()]) + return defaults, spider, DefaultHeadersMiddleware() def test_process_request(self): + defaults, spider, mw = self.get_defaults_spider_mw() req = Request('http://www.scrapytest.org') - self.mw.process_request(req, self.spider) - self.assertEquals(req.headers, self.default_request_headers) + mw.process_request(req, spider) + self.assertEquals(req.headers, defaults) def test_spider_default_request_headers(self): + defaults, spider, mw = self.get_defaults_spider_mw() spider_headers = {'Unexistant-Header': ['value']} # override one of the global default headers by spider - if self.default_request_headers: - k = set(self.default_request_headers).pop() + if defaults: + k = set(defaults).pop() spider_headers[k] = ['__newvalue__'] - self.spider.default_request_headers = spider_headers + spider.DEFAULT_REQUEST_HEADERS = spider_headers req = Request('http://www.scrapytest.org') - self.mw.process_request(req, self.spider) - self.assertEquals(req.headers, dict(self.default_request_headers, **spider_headers)) + mw.process_request(req, spider) + self.assertEquals(req.headers, dict(spider_headers)) def test_update_headers(self): + defaults, spider, mw = self.get_defaults_spider_mw() headers = {'Accept-Language': ['es'], 'Test-Header': ['test']} req = Request('http://www.scrapytest.org', headers=headers) self.assertEquals(req.headers, headers) - self.mw.process_request(req, self.spider) - self.default_request_headers.update(headers) - self.assertEquals(req.headers, self.default_request_headers) - + mw.process_request(req, spider) + defaults.update(headers) + self.assertEquals(req.headers, defaults) diff --git a/scrapy/tests/test_downloadermiddleware_downloadtimeout.py b/scrapy/tests/test_downloadermiddleware_downloadtimeout.py index fd60bee9e..fbe371996 100644 --- a/scrapy/tests/test_downloadermiddleware_downloadtimeout.py +++ b/scrapy/tests/test_downloadermiddleware_downloadtimeout.py @@ -3,31 +3,32 @@ import unittest from scrapy.contrib.downloadermiddleware.downloadtimeout import DownloadTimeoutMiddleware from scrapy.spider import BaseSpider from scrapy.http import Request +from scrapy.utils.test import get_crawler class DownloadTimeoutMiddlewareTest(unittest.TestCase): - def setUp(self): - self.mw = DownloadTimeoutMiddleware() - self.spider = BaseSpider('foo') - self.req = Request('http://scrapytest.org/') + def get_request_spider_mw(self): + crawler = get_crawler() + spider = BaseSpider('foo') + spider.set_crawler(crawler) + request = Request('http://scrapytest.org/') + return request, spider, DownloadTimeoutMiddleware() - def tearDown(self): - del self.mw - del self.spider - del self.req - - def test_spider_has_no_download_timeout(self): - assert self.mw.process_request(self.req, self.spider) is None - assert 'download_timeout' not in self.req.meta + def test_default_download_timeout(self): + req, spider, mw = self.get_request_spider_mw() + assert mw.process_request(req, spider) is None + self.assertEquals(req.meta.get('download_timeout'), 180) def test_spider_has_download_timeout(self): - self.spider.download_timeout = 2 - assert self.mw.process_request(self.req, self.spider) is None - self.assertEquals(self.req.meta.get('download_timeout'), 2) + req, spider, mw = self.get_request_spider_mw() + spider.DOWNLOAD_TIMEOUT = 2 + assert mw.process_request(req, spider) is None + self.assertEquals(req.meta.get('download_timeout'), 2) def test_request_has_download_timeout(self): - self.spider.download_timeout = 2 - self.req.meta['download_timeout'] = 1 - assert self.mw.process_request(self.req, self.spider) is None - self.assertEquals(self.req.meta.get('download_timeout'), 1) + req, spider, mw = self.get_request_spider_mw() + spider.DOWNLOAD_TIMEOUT = 2 + req.meta['download_timeout'] = 1 + assert mw.process_request(req, spider) is None + self.assertEquals(req.meta.get('download_timeout'), 1) diff --git a/scrapy/tests/test_downloadermiddleware_useragent.py b/scrapy/tests/test_downloadermiddleware_useragent.py index 866777341..44ac74c8b 100644 --- a/scrapy/tests/test_downloadermiddleware_useragent.py +++ b/scrapy/tests/test_downloadermiddleware_useragent.py @@ -3,47 +3,49 @@ from unittest import TestCase from scrapy.spider import BaseSpider from scrapy.http import Request from scrapy.contrib.downloadermiddleware.useragent import UserAgentMiddleware +from scrapy.utils.test import get_crawler class UserAgentMiddlewareTest(TestCase): - def setUp(self): - self.spider = BaseSpider('foo') - self.mw = UserAgentMiddleware() - - def tearDown(self): - del self.mw + def get_spider_and_mw(self, default_useragent): + crawler = get_crawler({'USER_AGENT': default_useragent}) + spider = BaseSpider('foo') + spider.set_crawler(crawler) + return spider, UserAgentMiddleware() def test_default_agent(self): - self.mw.default_useragent = 'default_useragent' + spider, mw = self.get_spider_and_mw('default_useragent') req = Request('http://scrapytest.org/') - assert self.mw.process_request(req, self.spider) is None + assert mw.process_request(req, spider) is None self.assertEquals(req.headers['User-Agent'], 'default_useragent') - # None or not present user_agent attribute is the same - self.spider.user_agent = None + def test_remove_agent(self): + # settings UESR_AGENT to None should remove the user agent + spider, mw = self.get_spider_and_mw('default_useragent') + spider.USER_AGENT = None req = Request('http://scrapytest.org/') - assert self.mw.process_request(req, self.spider) is None - self.assertEquals(req.headers['User-Agent'], 'default_useragent') + assert mw.process_request(req, spider) is None + assert req.headers.get('User-Agent') is None def test_spider_agent(self): - self.mw.default_useragent = 'default_useragent' - self.spider.user_agent = 'spider_useragent' + spider, mw = self.get_spider_and_mw('default_useragent') + spider.USER_AGENT = 'spider_useragent' req = Request('http://scrapytest.org/') - assert self.mw.process_request(req, self.spider) is None + assert mw.process_request(req, spider) is None self.assertEquals(req.headers['User-Agent'], 'spider_useragent') def test_header_agent(self): - self.mw.default_useragent = 'default_useragent' - self.spider.user_agent = 'spider_useragent' + spider, mw = self.get_spider_and_mw('default_useragent') + spider.USER_AGENT = 'spider_useragent' req = Request('http://scrapytest.org/', headers={'User-Agent': 'header_useragent'}) - assert self.mw.process_request(req, self.spider) is None + assert mw.process_request(req, spider) is None self.assertEquals(req.headers['User-Agent'], 'header_useragent') def test_no_agent(self): - self.mw.default_useragent = None - self.spider.user_agent = None + spider, mw = self.get_spider_and_mw(None) + spider.USER_AGENT = None req = Request('http://scrapytest.org/') - assert self.mw.process_request(req, self.spider) is None + assert mw.process_request(req, spider) is None assert 'User-Agent' not in req.headers diff --git a/scrapy/tests/test_engine.py b/scrapy/tests/test_engine.py index 8f4ab8fb7..2664daf69 100644 --- a/scrapy/tests/test_engine.py +++ b/scrapy/tests/test_engine.py @@ -17,8 +17,7 @@ from twisted.web import server, static, util from twisted.trial import unittest from scrapy import signals -from scrapy.settings import Settings -from scrapy.crawler import Crawler +from scrapy.utils.test import get_crawler from scrapy.xlib.pydispatch import dispatcher from scrapy.tests import tests_datadir from scrapy.spider import BaseSpider @@ -95,8 +94,7 @@ class CrawlerRun(object): dispatcher.connect(self.request_received, signals.request_received) dispatcher.connect(self.response_downloaded, signals.response_downloaded) - settings = Settings() - self.crawler = Crawler(settings) + self.crawler = get_crawler() self.crawler.install() self.crawler.configure() self.crawler.queue.append_spider(self.spider) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py new file mode 100644 index 000000000..58f580259 --- /dev/null +++ b/scrapy/utils/deprecate.py @@ -0,0 +1,9 @@ +"""Some helpers for deprecation messages""" + +import warnings + +def attribute(obj, oldattr, newattr, version='0.12'): + cname = obj.__class__.__name__ + warnings.warn("%s.%s attribute is deprecated and will be no longer supported " + "in Scrapy %s, use %s.%s attribute instead" % \ + (cname, oldattr, version, cname, newattr), DeprecationWarning, stacklevel=3) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 428b43f43..597ee909c 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -7,6 +7,9 @@ import os import libxml2 from twisted.trial.unittest import SkipTest +from scrapy.crawler import Crawler +from scrapy.settings import CrawlerSettings + def libxml2debug(testfunction): """Decorator for debugging libxml2 memory leaks inside a function. @@ -39,3 +42,17 @@ def assert_aws_environ(): if 'AWS_ACCESS_KEY_ID' not in os.environ: raise SkipTest("AWS keys not found") + +def get_crawler(settings_dict=None): + """Return an unconfigured Crawler object. If settings_dict is given, it + will be used as the settings present in the settings module of the + CrawlerSettings. + """ + class SettingsModuleMock(object): + pass + settings_module = SettingsModuleMock() + if settings_dict: + for k, v in settings_dict.items(): + setattr(settings_module, k, v) + settings = CrawlerSettings(settings_module) + return Crawler(settings)