From 87e63216ef945e580c3f30675020d968f8ad7d9f Mon Sep 17 00:00:00 2001
From: Daniel Grana <dangra@gmail.com>
Date: Fri, 20 Mar 2009 19:37:55 +0000
Subject: [PATCH] retrymw: remove fingerprint cache and use request meta
 instead, add more tests

--HG--
extra : convert_revision : svn%3Ab85faa78-f9eb-468e-a121-7cced6da292c%401003
---
 .../contrib/downloadermiddleware/retry.py     | 20 +++++----
 .../tests/test_downloadermiddleware_retry.py  | 44 +++++++++++++++----
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/scrapy/trunk/scrapy/contrib/downloadermiddleware/retry.py b/scrapy/trunk/scrapy/contrib/downloadermiddleware/retry.py
index 8d2eb3b4f..23b9d0a48 100644
--- a/scrapy/trunk/scrapy/contrib/downloadermiddleware/retry.py
+++ b/scrapy/trunk/scrapy/contrib/downloadermiddleware/retry.py
@@ -41,20 +41,24 @@ class RetryMiddleware(object):
                            ConnectionLost)
 
     def __init__(self):
-        self.failed_count = {}
-        self.retry_times = settings.getint('RETRY_TIMES')
+        self.max_retry_times = settings.getint('RETRY_TIMES')
         self.retry_http_codes = map(int, settings.getlist('RETRY_HTTP_CODES'))
 
     def process_exception(self, request, exception, spider):
-        if isinstance(exception, self.EXCEPTIONS_TO_RETRY) or (isinstance(exception, HttpException) and (int(exception.status) in self.retry_http_codes)):
-            fp = request_fingerprint(request)
-            self.failed_count[fp] = self.failed_count.get(fp, 0) + 1
+        if isinstance(exception, self.EXCEPTIONS_TO_RETRY) \
+                or (isinstance(exception, HttpException) \
+                        and (int(exception.status) in self.retry_http_codes)):
 
-            if self.failed_count[fp] <= self.retry_times:
-                log.msg("Retrying %s (failed %d times): %s" % (request, self.failed_count[fp], exception), domain=spider.domain_name, level=log.DEBUG)
+            retries = request.meta.get('retry_times', 0) + 1
+
+            if retries <= self.max_retry_times:
+                log.msg("Retrying %s (failed %d times): %s" % (request, retries, exception),
+                        domain=spider.domain_name, level=log.DEBUG)
                 retryreq = request.copy()
+                retryreq.meta['retry_times'] = retries
                 retryreq.dont_filter = True
                 return retryreq
             else:
-                log.msg("Discarding %s (failed %d times): %s" % (request, self.failed_count[fp], exception), domain=spider.domain_name, level=log.DEBUG)
+                log.msg("Discarding %s (failed %d times): %s" % (request, retries, exception),
+                        domain=spider.domain_name, level=log.DEBUG)
 
diff --git a/scrapy/trunk/scrapy/tests/test_downloadermiddleware_retry.py b/scrapy/trunk/scrapy/tests/test_downloadermiddleware_retry.py
index 7f6024bff..35adec70a 100644
--- a/scrapy/trunk/scrapy/tests/test_downloadermiddleware_retry.py
+++ b/scrapy/trunk/scrapy/tests/test_downloadermiddleware_retry.py
@@ -1,5 +1,9 @@
 import unittest
 
+from twisted.internet.error import TimeoutError as ServerTimeoutError, DNSLookupError, \
+                                   ConnectionRefusedError, ConnectionDone, ConnectError, \
+                                   ConnectionLost
+
 from scrapy.contrib.downloadermiddleware.retry import RetryMiddleware
 from scrapy.core.exceptions import HttpException
 from scrapy.spider import spiders
@@ -10,18 +14,42 @@ class RetryTest(unittest.TestCase):
         spiders.spider_modules = ['scrapy.tests.test_spiders']
         spiders.reload()
         self.spider = spiders.fromdomain('scrapytest.org')
+        self.mw = RetryMiddleware()
+        self.mw.max_retry_times = 2
 
-    def test_process_exception(self):
-        exception_404 = (Request('http://www.scrapytest.org/404'), HttpException('404', None, Response('http://www.scrapytest.org/404', body='')), self.spider)
-        exception_503 = (Request('http://www.scrapytest.org/503'), HttpException('503', None, Response('http://www.scrapytest.org/503', body='')), self.spider)
+    def test_process_exception_404(self):
+        req404 = Request('http://www.scrapytest.org/404')
+        exc404 = HttpException('404', None, Response('http://www.scrapytest.org/404', body=''))
 
-        mw = RetryMiddleware()
-        mw.retry_times = 1
+        # dont retry 404s
+        req = self.mw.process_exception(req404, exc404, self.spider)
+        self.assertTrue(req is None)
 
-        self.assertTrue(mw.process_exception(*exception_404) is None)
+    def test_process_exception_503(self):
+        req503 = Request('http://www.scrapytest.org/503')
+        exc503 = HttpException('503', None, Response('http://www.scrapytest.org/503', body=''))
+        self._test_retry_exception(req503, exc503)
+
+    def test_process_exception_twistederrors(self):
+        for exc in (ServerTimeoutError, DNSLookupError, ConnectionRefusedError, ConnectionDone, ConnectError, ConnectionLost):
+            req = Request('http://www.scrapytest.org/%s' % exc.__name__)
+            self._test_retry_exception(req, exc())
+
+    def _test_retry_exception(self, req, exception):
+        # first retry
+        req = self.mw.process_exception(req, exception, self.spider)
+        assert isinstance(req, Request)
+        self.assertEqual(req.meta['retry_times'], 1)
+
+        # second retry
+        req = self.mw.process_exception(req, exception, self.spider)
+        assert isinstance(req, Request)
+        self.assertEqual(req.meta['retry_times'], 2)
+
+        # discard it
+        req = self.mw.process_exception(req, exception, self.spider)
+        self.assertEqual(req, None)
 
-        self.assertTrue(isinstance(mw.process_exception(*exception_503), Request))
-        self.assertTrue(mw.process_exception(*exception_503) is None)
 
 if __name__ == "__main__":
     unittest.main()