Merge pull request #1728 from scrapy/deprecate-make-requests-from-url

deprecate Spider.make_requests_from_url.

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

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):

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__

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)

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):

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)