Merge pull request #1456 from scrapy/py3-fixes

Small Python 3 fixes

.gitignore

@@ -10,6 +10,8 @@ venv
 build
 dist
 .idea
+htmlcov/
+.coverage
 
 # Windows
 Thumbs.db

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

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,

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

scrapy/statscollectors.py

@@ -50,6 +50,7 @@ class StatsCollector(object):
     def _persist_stats(self, stats, spider):
         pass
 
+
 class MemoryStatsCollector(StatsCollector):
 
     def __init__(self, crawler):

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

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
 

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__":

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