diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index c123c2635..5e69055d1 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -144,16 +144,12 @@ scrapy.Spider .. method:: start_requests() This method must return an iterable with the first Requests to crawl for - this spider. + this spider. It is called by Scrapy when the spider is opened for + scraping. Scrapy calls it only once, so it is safe to implement + :meth:`start_requests` as a generator. - This is the method called by Scrapy when the spider is opened for - scraping when no particular URLs are specified. If particular URLs are - specified, the :meth:`make_requests_from_url` is used instead to create - the Requests. This method is also called only once from Scrapy, so it's - safe to implement it as a generator. - - The default implementation uses :meth:`make_requests_from_url` to - generate Requests for each url in :attr:`start_urls`. + The default implementation generates ``Request(url, dont_filter=True)`` + for each url in :attr:`start_urls`. If you want to change the Requests used to start scraping a domain, this is the method to override. For example, if you need to start by logging in using @@ -172,18 +168,6 @@ scrapy.Spider # each of them, with another callback pass - .. method:: make_requests_from_url(url) - - A method that receives a URL and returns a :class:`~scrapy.http.Request` - object (or a list of :class:`~scrapy.http.Request` objects) to scrape. This - method is used to construct the initial requests in the - :meth:`start_requests` method, and is typically used to convert urls to - requests. - - Unless overridden, this method returns Requests with the :meth:`parse` - method as their callback function, and with dont_filter parameter enabled - (see :class:`~scrapy.http.Request` class for more info). - .. method:: parse(response) This is the default callback used by Scrapy to process downloaded diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index c08bb964a..30cb7590a 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -12,6 +12,7 @@ from scrapy.utils.trackref import object_ref from scrapy.utils.url import url_is_from_spider from scrapy.utils.deprecate import create_deprecated_class from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.deprecate import method_is_overridden class Spider(object_ref): @@ -66,10 +67,23 @@ class Spider(object_ref): crawler.signals.connect(self.close, signals.spider_closed) def start_requests(self): - for url in self.start_urls: - yield self.make_requests_from_url(url) + cls = self.__class__ + if method_is_overridden(cls, Spider, 'make_requests_from_url'): + warnings.warn( + "Spider.make_requests_from_url method is deprecated; it " + "won't be called in future Scrapy releases. Please " + "override Spider.start_requests method instead (see %s.%s)." % ( + cls.__module__, cls.__name__ + ), + ) + for url in self.start_urls: + yield self.make_requests_from_url(url) + else: + for url in self.start_urls: + yield Request(url, dont_filter=True) def make_requests_from_url(self, url): + """ This method is deprecated. """ return Request(url, dont_filter=True) def parse(self, response): diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 0fc33e0c4..7ab39c97e 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -156,3 +156,35 @@ def update_classpath(path): ScrapyDeprecationWarning) return new_path return path + + +def method_is_overridden(subclass, base_class, method_name): + """ + Return True if a method named ``method_name`` of a ``base_class`` + is overridden in a ``subclass``. + + >>> class Base(object): + ... def foo(self): + ... pass + >>> class Sub1(Base): + ... pass + >>> class Sub2(Base): + ... def foo(self): + ... pass + >>> class Sub3(Sub1): + ... def foo(self): + ... pass + >>> class Sub4(Sub2): + ... pass + >>> method_is_overridden(Sub1, Base, 'foo') + False + >>> method_is_overridden(Sub2, Base, 'foo') + True + >>> method_is_overridden(Sub3, Base, 'foo') + True + >>> method_is_overridden(Sub4, Base, 'foo') + True + """ + base_method = getattr(base_class, method_name) + sub_method = getattr(subclass, method_name) + return base_method.__code__ is not sub_method.__code__ diff --git a/tests/spiders.py b/tests/spiders.py index 711d80cac..1038b69de 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -170,10 +170,7 @@ class DuplicateStartRequestsSpider(Spider): for i in range(0, self.distinct_urls): for j in range(0, self.dupe_factor): url = "http://localhost:8998/echo?headers=1&body=test%d" % i - yield self.make_requests_from_url(url) - - def make_requests_from_url(self, url): - return Request(url, dont_filter=self.dont_filter) + yield Request(url, dont_filter=self.dont_filter) def __init__(self, url="http://localhost:8998", *args, **kwargs): super(DuplicateStartRequestsSpider, self).__init__(*args, **kwargs) diff --git a/tests/test_engine.py b/tests/test_engine.py index baf6ef1bf..04113ddcf 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -66,8 +66,8 @@ class TestSpider(Spider): class TestDupeFilterSpider(TestSpider): - def make_requests_from_url(self, url): - return Request(url) # dont_filter=False + def start_requests(self): + return (Request(url) for url in self.start_urls) # no dont_filter=True class DictItemsSpider(TestSpider): diff --git a/tests/test_spider.py b/tests/test_spider.py index 079734a69..371b8c1ac 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -345,7 +345,7 @@ Sitemap: /sitemap-relative-url.xml 'http://www.example.com/sitemap-relative-url.xml']) -class BaseSpiderDeprecationTest(unittest.TestCase): +class DeprecationTest(unittest.TestCase): def test_basespider_is_deprecated(self): with warnings.catch_warnings(record=True) as w: @@ -399,6 +399,29 @@ class BaseSpiderDeprecationTest(unittest.TestCase): assert isinstance(CrawlSpider(name='foo'), Spider) assert isinstance(CrawlSpider(name='foo'), BaseSpider) + def test_make_requests_from_url_deprecated(self): + class MySpider4(Spider): + name = 'spider1' + start_urls = ['http://example.com'] -if __name__ == '__main__': - unittest.main() + class MySpider5(Spider): + name = 'spider2' + start_urls = ['http://example.com'] + + def make_requests_from_url(self, url): + return Request(url + "/foo", dont_filter=True) + + with warnings.catch_warnings(record=True) as w: + # spider without overridden make_requests_from_url method + # doesn't issue a warning + spider1 = MySpider4() + self.assertEqual(len(list(spider1.start_requests())), 1) + self.assertEqual(len(w), 0) + + # spider with overridden make_requests_from_url issues a warning, + # but the method still works + spider2 = MySpider5() + requests = list(spider2.start_requests()) + self.assertEqual(len(requests), 1) + self.assertEqual(requests[0].url, 'http://example.com/foo') + self.assertEqual(len(w), 1)