Promote a new CrawlSpider that allows overriding `parse`

[redapple]

I see a lot of StackOverflow questions and problems with CrawlSpider and overriden parse methods. (e.g. https://stackoverflow.com/questions/23511230)

I'd like to see a new implementation, called CrawlSpider2 or CrawlingSpider or something, that uses another internal method with another name than parse,
so that user could define their own parse method.

Then, the question is if users will expect this parse method to be used by default for each downloaded page (is addition to being parsed for links with Rules),
or if the reference to parse should be explicit.

Thoughts?

[kmike]

IMHO the cause for such questions is that CrawlSpider is used in overview, and then Spider is used in tutorial. I propose promoting Spider instead of CrawlSpider (see #609).

[redapple]

@kmike probably. I still think not being able to use parse in CrawlSpider (unless you know what you're doing), is a bit confusing.

When you know Spider and assume parse is the default method for responses, for CrawlSpider I would expect the parse method to be called at least for start_urls but for that you need work with parse_start_url.
http://doc.scrapy.org/en/latest/topics/spiders.html#scrapy.spider.Spider.parse
And why not assume parse to be default callback for Rules ? (unless you say otherwise in the Rule).

[kmike]

parse is called for start_urls:

from scrapy.contrib.spiders.crawl import CrawlSpider

class MySpider(CrawlSpider):
    name = 'testspider'
    start_urls = ['http://scrapinghub.com/']

    def parse(self, response):
        print(response.url)

You need to call super() to make rules work, but that is expected - if you override something it is reasonable to call super.

Can't comment about parse being default callback for Rules, I haven't used rules much. Also, I'm not sure I like that parse is a default callback for Request - it makes sense for requests issued in start_requests, but for other requests it kind-of suggests to put all code in a single callback, while usually you have multiple callbacks.

[redapple]

Sure, parse is called. I'm just saying that so many people get it wrong there must be something unclear.

At least we should provide an example in the docs on how to properly override parse.

The warning is wrong: if you override the parse method, the crawl spider will no longer work.

When writing crawl spider rules, avoid using parse as callback, since the CrawlSpider uses the parse method itself to implement its logic. So if you override the parse method, the crawl spider will no longer work.

... unless you return super(MySpider, self).parse(response)

And again, for Spider the docs suggest you should override it to do something useful. For CrawlSpider you should not.

[nyov]

When you haven't read the source and understand what's going on, it's easy to forget you should, or even need to call super().
Yes, the warning in the docs is also misleading, but someone reading it will know how to fix it (not using parse).

This issue is so common, it's becoming a "useability bug" - not a bug in itself, but creating unnecessary headaches, not intuitive.

I propose having another internal _parse method called by the Scraper instead of parse, and using that in spiders who want internal "pre-processing", then exposing parse as the public, documented, no-baggage method to implement/override from a spider.

[eliasdorneles]

So, here is my suggestion:

--- a/scrapy/contrib/spiders/crawl.py
+++ b/scrapy/contrib/spiders/crawl.py
@@ -36,7 +36,15 @@ class CrawlSpider(Spider):
         self._compile_rules()

     def parse(self, response):
-        return self._parse_response(response, self.parse_start_url, cb_kwargs={}, follow=True)
+        return self._parse_rules(response)
+
+    def _parse_rules(self, response):
+        return self._parse_response(response,
+                                    self.parse_start_url, cb_kwargs={}, follow=True)
+
+    def start_requests(self):
+        for url in self.start_urls:
+            yield Request(url, self._parse_rules, dont_filter=True)

     def parse_start_url(self, response):
         return []

Pros:

1. Provides backward compatibility for implementations of CrawlSpider that yield Requests with parse() as callback to trigger applying the rules again.
2. Supports overriding the parse() method without big changes under the hood.

Cons:

1. When overriding the parse() method and wanting to use it as a callback for a rule, user will need to set it explicitly to 'parse' (because the default callback for the rules is now '_parse_rules').

Did I miss anything? (I probably am...)

[redapple]

Closing in favor of #712