Repository: scrapy/scrapy
Pull request #1456 (2015-08-25 20:59:26)

Small Python 3 fixes
low hanging fruits


Patch:
diff --git a/.gitignore b/.gitignore
index 4db22f1d8..b116640b4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,8 @@ venv
 build
 dist
 .idea
+htmlcov/
+.coverage
 
 # Windows
 Thumbs.db
diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py
index 244499be2..67198179d 100644
--- a/scrapy/core/scraper.py
+++ b/scrapy/core/scraper.py
@@ -16,6 +16,7 @@ from scrapy import signals
 from scrapy.http import Request, Response
 from scrapy.item import BaseItem
 from scrapy.core.spidermw import SpiderMiddlewareManager
+from scrapy.utils.request import referer_str
 
 logger = logging.getLogger(__name__)
 
@@ -150,10 +151,9 @@ class Scraper(object):
         if isinstance(exc, CloseSpider):
             self.crawler.engine.close_spider(spider, exc.reason or 'cancelled')
             return
-        referer = request.headers.get('Referer')
         logger.error(
             "Spider error processing %(request)s (referer: %(referer)s)",
-            {'request': request, 'referer': referer},
+            {'request': request, 'referer': referer_str(request)},
             exc_info=failure_to_exc_info(_failure),
             extra={'spider': spider}
         )
diff --git a/scrapy/logformatter.py b/scrapy/logformatter.py
index a0508e0b7..2160d9ab0 100644
--- a/scrapy/logformatter.py
+++ b/scrapy/logformatter.py
@@ -3,6 +3,7 @@ import logging
 
 from twisted.python.failure import Failure
 
+from scrapy.utils.request import referer_str
 
 SCRAPEDMSG = u"Scraped from %(src)s" + os.linesep + "%(item)s"
 DROPPEDMSG = u"Dropped: %(exception)s" + os.linesep + "%(item)s"
@@ -38,13 +39,16 @@ class LogFormatter(object):
             'args': {
                 'status': response.status,
                 'request': request,
-                'referer': request.headers.get('Referer'),
+                'referer': referer_str(request),
                 'flags': flags,
             }
         }
 
     def scraped(self, item, response, spider):
-        src = response.getErrorMessage() if isinstance(response, Failure) else response
+        if isinstance(response, Failure):
+            src = response.getErrorMessage()
+        else:
+            src = response
         return {
             'level': logging.DEBUG,
             'msg': SCRAPEDMSG,
diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index db49aff65..e4011d31d 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -26,7 +26,8 @@ from scrapy.exceptions import NotConfigured, IgnoreRequest
 from scrapy.http import Request
 from scrapy.utils.misc import md5sum
 from scrapy.utils.log import failure_to_exc_info
-from scrapy.utils.python import to_bytes, to_native_str
+from scrapy.utils.python import to_bytes
+from scrapy.utils.request import referer_str
 
 logger = logging.getLogger(__name__)
 
@@ -199,7 +200,7 @@ class FilesPipeline(MediaPipeline):
             if age_days > self.EXPIRES:
                 return  # returning None force download
 
-            referer = _get_referer(request)
+            referer = referer_str(request)
             logger.debug(
                 'File (uptodate): Downloaded %(medianame)s from %(request)s '
                 'referred in <%(referer)s>',
@@ -225,7 +226,7 @@ class FilesPipeline(MediaPipeline):
 
     def media_failed(self, failure, request, info):
         if not isinstance(failure.value, IgnoreRequest):
-            referer = _get_referer(request)
+            referer = referer_str(request)
             logger.warning(
                 'File (unknown-error): Error downloading %(medianame)s from '
                 '%(request)s referred in <%(referer)s>: %(exception)s',
@@ -237,7 +238,7 @@ class FilesPipeline(MediaPipeline):
         raise FileException
 
     def media_downloaded(self, response, request, info):
-        referer = _get_referer(request)
+        referer = referer_str(request)
 
         if response.status != 200:
             logger.warning(
@@ -339,11 +340,3 @@ class FilesPipeline(MediaPipeline):
     def file_key(self, url):
         return self.file_path(url)
     file_key._base = True
-
-
-def _get_referer(request):
-    """ Return Referer HTTP header suitable for logging """
-    referrer = request.headers.get('Referer')
-    if referrer is None:
-        return referrer
-    return to_native_str(referrer, errors='replace')
diff --git a/scrapy/statscollectors.py b/scrapy/statscollectors.py
index 62b037f36..6da9ddcd2 100644
--- a/scrapy/statscollectors.py
+++ b/scrapy/statscollectors.py
@@ -50,6 +50,7 @@ class StatsCollector(object):
     def _persist_stats(self, stats, spider):
         pass
 
+
 class MemoryStatsCollector(StatsCollector):
 
     def __init__(self, crawler):
diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py
index 0487d1e1b..e361b7433 100644
--- a/scrapy/utils/request.py
+++ b/scrapy/utils/request.py
@@ -8,7 +8,6 @@ import hashlib
 import weakref
 from six.moves.urllib.parse import urlunparse
 
-from twisted.internet.defer import Deferred
 from w3lib.http import basic_auth_header
 from scrapy.utils.python import to_bytes, to_native_str
 
@@ -86,3 +85,10 @@ def request_httprepr(request):
     s += request.body
     return s
 
+
+def referer_str(request):
+    """ Return Referer HTTP header suitable for logging. """
+    referrer = request.headers.get('Referer')
+    if referrer is None:
+        return referrer
+    return to_native_str(referrer, errors='replace')


Test patch:
diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt
index 5a009db36..f380e6679 100644
--- a/tests/py3-ignores.txt
+++ b/tests/py3-ignores.txt
@@ -20,7 +20,6 @@ tests/test_downloadermiddleware_retry.py
 tests/test_downloadermiddleware_stats.py
 tests/test_downloadermiddleware_useragent.py
 tests/test_engine.py
-tests/test_logformatter.py
 tests/test_mail.py
 tests/test_pipeline_files.py
 tests/test_pipeline_images.py
@@ -30,9 +29,7 @@ tests/test_spidermiddleware_httperror.py
 tests/test_spidermiddleware_offsite.py
 tests/test_spidermiddleware_referer.py
 tests/test_spider.py
-tests/test_stats.py
 tests/test_utils_iterators.py
-tests/test_utils_log.py
 tests/test_utils_template.py
 tests/test_webclient.py
 
diff --git a/tests/test_logformatter.py b/tests/test_logformatter.py
index ec42ef8ab..50e9662c6 100644
--- a/tests/test_logformatter.py
+++ b/tests/test_logformatter.py
@@ -1,4 +1,5 @@
 import unittest
+import six
 
 from scrapy.spiders import Spider
 from scrapy.http import Request, Response
@@ -42,7 +43,7 @@ class LoggingContribTest(unittest.TestCase):
         logkws = self.formatter.dropped(item, exception, response, self.spider)
         logline = logkws['msg'] % logkws['args']
         lines = logline.splitlines()
-        assert all(isinstance(x, unicode) for x in lines)
+        assert all(isinstance(x, six.text_type) for x in lines)
         self.assertEqual(lines, [u"Dropped: \u2018", '{}'])
 
     def test_scraped(self):
@@ -52,7 +53,7 @@ class LoggingContribTest(unittest.TestCase):
         logkws = self.formatter.scraped(item, response, self.spider)
         logline = logkws['msg'] % logkws['args']
         lines = logline.splitlines()
-        assert all(isinstance(x, unicode) for x in lines)
+        assert all(isinstance(x, six.text_type) for x in lines)
         self.assertEqual(lines, [u"Scraped from <200 http://www.example.com>", u'name: \xa3'])
 
 if __name__ == "__main__":
diff --git a/tests/test_stats.py b/tests/test_stats.py
index 5c7c0e6bb..9f950ebc9 100644
--- a/tests/test_stats.py
+++ b/tests/test_stats.py
@@ -4,6 +4,7 @@ from scrapy.spiders import Spider
 from scrapy.statscollectors import StatsCollector, DummyStatsCollector
 from scrapy.utils.test import get_crawler
 
+
 class StatsCollectorTest(unittest.TestCase):
 
     def setUp(self):
@@ -50,6 +51,3 @@ class StatsCollectorTest(unittest.TestCase):
         stats.set_value('test', 'value', spider=self.spider)
         self.assertEqual(stats.get_stats(), {})
         self.assertEqual(stats.get_stats('a'), {})
-
-if __name__ == "__main__":
-    unittest.main()