Merge branch 'master' into spider.parse

artwork/README.rst

@@ -1,5 +1,4 @@
-:orphan:
-
+==============
 Scrapy artwork
 ==============
 

docs/conf.py

@@ -286,6 +286,7 @@ intersphinx_mapping = {
     'sphinx': ('https://www.sphinx-doc.org/en/master', None),
     'tox': ('https://tox.readthedocs.io/en/latest', None),
     'twisted': ('https://twistedmatrix.com/documents/current', None),
+    'twistedapi': ('https://twistedmatrix.com/documents/current/api', None),
 }
 
 

docs/news.rst

@@ -288,6 +288,13 @@ Backward-incompatible changes
     :class:`~scrapy.http.Request` objects instead of arbitrary Python data
     structures.
 
+*   An additional ``crawler`` parameter has been added to the ``__init__`` method
+    of the :class:`scrapy.core.scheduler.Scheduler` class. 
+    Custom scheduler subclasses which don't accept arbitrary parameters in 
+    their ``__init__`` method might break because of this change.
+
+    For more information, refer to the documentation for the :setting:`SCHEDULER` setting.
+
 See also :ref:`1.7-deprecation-removals` below.
 
 

docs/topics/media-pipeline.rst

@@ -410,7 +410,7 @@ See here the methods that you can override in your custom Files Pipeline:
 
 .. class:: FilesPipeline
 
-   .. method:: file_path(request, response, info)
+   .. method:: file_path(self, request, response=None, info=None)
 
       This method is called once per downloaded item. It returns the
       download path of the file originating from the specified
@@ -434,7 +434,7 @@ See here the methods that you can override in your custom Files Pipeline:
 
         class MyFilesPipeline(FilesPipeline):
 
-            def file_path(self, request, response, info):
+            def file_path(self, request, response=None, info=None):
                 return 'files/' + os.path.basename(urlparse(request.url).path)
 
       By default the :meth:`file_path` method returns
@@ -524,7 +524,7 @@ See here the methods that you can override in your custom Images Pipeline:
     The :class:`ImagesPipeline` is an extension of the :class:`FilesPipeline`,
     customizing the field names and adding custom behavior for images.
 
-   .. method:: file_path(request, response, info)
+   .. method:: file_path(self, request, response=None, info=None)
 
       This method is called once per downloaded item. It returns the
       download path of the file originating from the specified
@@ -548,7 +548,7 @@ See here the methods that you can override in your custom Images Pipeline:
 
         class MyImagesPipeline(ImagesPipeline):
 
-            def file_path(self, request, response, info):
+            def file_path(self, request, response=None, info=None):
                 return 'files/' + os.path.basename(urlparse(request.url).path)
 
       By default the :meth:`file_path` method returns

docs/topics/signals.rst

@@ -295,6 +295,23 @@ request_reached_downloader
     :param spider: the spider that yielded the request
     :type spider: :class:`~scrapy.spiders.Spider` object
 
+request_left_downloader
+-----------------------
+
+.. signal:: request_left_downloader
+.. function:: request_left_downloader(request, spider)
+
+    Sent when a :class:`~scrapy.http.Request` leaves the downloader, even in case of
+    failure.
+
+    This signal does not support returning deferreds from its handlers.
+
+    :param request: the request that reached the downloader
+    :type request: :class:`~scrapy.http.Request` object
+
+    :param spider: the spider that yielded the request
+    :type spider: :class:`~scrapy.spiders.Spider` object
+
 response_received
 -----------------
 

extras/scrapy_zsh_completion

@@ -1,11 +1,12 @@
 #compdef scrapy
 _scrapy() {
     local context state state_descr line
+    local ret=1
     typeset -A opt_args
     _arguments \
-	"(- 1 *)--help[Help]" \
+	"(- 1 *)"{-h,--help}"[Help]" \
 	"1: :->command" \
-	"*:: :->args"
+	"*:: :->args" && ret=0
 
     case $state in
 	command)
@@ -134,6 +135,8 @@ _scrapy() {
 	    esac
 	    ;;
     esac
+
+    return ret
 }
 
 _scrapy_cmds() {

scrapy/core/downloader/__init__.py

@@ -181,6 +181,9 @@ class Downloader(object):
         def finish_transferring(_):
             slot.transferring.remove(request)
             self._process_queue(spider, slot)
+            self.signals.send_catch_log(signal=signals.request_left_downloader,
+                                        request=request,
+                                        spider=spider)
             return _
 
         return dfd.addBoth(finish_transferring)

scrapy/core/spidermw.py

@@ -3,13 +3,14 @@ Spider Middleware manager
 
 See documentation in docs/topics/spider-middleware.rst
 """
-from itertools import chain, islice
+from itertools import islice
 
 from twisted.python.failure import Failure
+
 from scrapy.exceptions import _InvalidOutput
 from scrapy.middleware import MiddlewareManager
-from scrapy.utils.defer import mustbe_deferred
 from scrapy.utils.conf import build_component_list
+from scrapy.utils.defer import mustbe_deferred
 from scrapy.utils.python import MutableChain
 
 
@@ -17,6 +18,13 @@ def _isiterable(possible_iterator):
     return hasattr(possible_iterator, '__iter__')
 
 
+def _fname(f):
+    return "%s.%s".format(
+        f.__self__.__class__.__name__,
+        f.__func__.__name__
+    )
+
+
 class SpiderMiddlewareManager(MiddlewareManager):
 
     component_name = 'spider middleware'
@@ -31,27 +39,36 @@ class SpiderMiddlewareManager(MiddlewareManager):
             self.methods['process_spider_input'].append(mw.process_spider_input)
         if hasattr(mw, 'process_start_requests'):
             self.methods['process_start_requests'].appendleft(mw.process_start_requests)
-        self.methods['process_spider_output'].appendleft(getattr(mw, 'process_spider_output', None))
-        self.methods['process_spider_exception'].appendleft(getattr(mw, 'process_spider_exception', None))
+        process_spider_output = getattr(mw, 'process_spider_output', None)
+        self.methods['process_spider_output'].appendleft(process_spider_output)
+        process_spider_exception = getattr(mw, 'process_spider_exception', None)
+        self.methods['process_spider_exception'].appendleft(process_spider_exception)
 
     def scrape_response(self, scrape_func, response, request, spider):
-        fname = lambda f: '%s.%s' % (
-                f.__self__.__class__.__name__,
-                f.__func__.__name__)
 
         def process_spider_input(response):
             for method in self.methods['process_spider_input']:
                 try:
                     result = method(response=response, spider=spider)
                     if result is not None:
-                        raise _InvalidOutput('Middleware {} must return None or raise an exception, got {}'
-                                             .format(fname(method), type(result)))
+                        msg = "Middleware {} must return None or raise an exception, got {}"
+                        raise _InvalidOutput(msg.format(_fname(method), type(result)))
                 except _InvalidOutput:
                     raise
                 except Exception:
                     return scrape_func(Failure(), request, spider)
             return scrape_func(response, request, spider)
 
+        def _evaluate_iterable(iterable, exception_processor_index, recover_to):
+            try:
+                for r in iterable:
+                    yield r
+            except Exception as ex:
+                exception_result = process_spider_exception(Failure(ex), exception_processor_index)
+                if isinstance(exception_result, Failure):
+                    raise
+                recover_to.extend(exception_result)
+
         def process_spider_exception(_failure, start_index=0):
             exception = _failure.value
             # don't handle _InvalidOutput exception
@@ -69,8 +86,8 @@ class SpiderMiddlewareManager(MiddlewareManager):
                 elif result is None:
                     continue
                 else:
-                    raise _InvalidOutput('Middleware {} must return None or an iterable, got {}'
-                                         .format(fname(method), type(result)))
+                    msg = "Middleware {} must return None or an iterable, got {}"
+                    raise _InvalidOutput(msg.format(_fname(method), type(result)))
             return _failure
 
         def process_spider_output(result, start_index=0):
@@ -78,22 +95,12 @@ class SpiderMiddlewareManager(MiddlewareManager):
             # chain, they went through it already from the process_spider_exception method
             recovered = MutableChain()
 
-            def evaluate_iterable(iterable, index):
-                try:
-                    for r in iterable:
-                        yield r
-                except Exception as ex:
-                    exception_result = process_spider_exception(Failure(ex), index+1)
-                    if isinstance(exception_result, Failure):
-                        raise
-                    recovered.extend(exception_result)
-
             method_list = islice(self.methods['process_spider_output'], start_index, None)
             for method_index, method in enumerate(method_list, start=start_index):
                 if method is None:
                     continue
-                # the following might fail directly if the output value is not a generator
                 try:
+                    # might fail directly if the output value is not a generator
                     result = method(response=response, result=result, spider=spider)
                 except Exception as ex:
                     exception_result = process_spider_exception(Failure(ex), method_index+1)
@@ -101,15 +108,20 @@ class SpiderMiddlewareManager(MiddlewareManager):
                         raise
                     return exception_result
                 if _isiterable(result):
-                    result = evaluate_iterable(result, method_index)
+                    result = _evaluate_iterable(result, method_index+1, recovered)
                 else:
-                    raise _InvalidOutput('Middleware {} must return an iterable, got {}'
-                                         .format(fname(method), type(result)))
+                    msg = "Middleware {} must return an iterable, got {}"
+                    raise _InvalidOutput(msg.format(_fname(method), type(result)))
 
-            return chain(result, recovered)
+            return MutableChain(result, recovered)
+
+        def process_callback_output(result):
+            recovered = MutableChain()
+            result = _evaluate_iterable(result, 0, recovered)
+            return MutableChain(process_spider_output(result), recovered)
 
         dfd = mustbe_deferred(process_spider_input, response)
-        dfd.addCallbacks(callback=process_spider_output, errback=process_spider_exception)
+        dfd.addCallbacks(callback=process_callback_output, errback=process_spider_exception)
         return dfd
 
     def process_start_requests(self, start_requests, spider):

scrapy/dupefilters.py

@@ -49,7 +49,7 @@ class RFPDupeFilter(BaseDupeFilter):
             return True
         self.fingerprints.add(fp)
         if self.file:
-            self.file.write(fp + os.linesep)
+            self.file.write(fp + '\n')
 
     def request_fingerprint(self, request):
         return request_fingerprint(request)

scrapy/signals.py

@@ -14,6 +14,7 @@ spider_error = object()
 request_scheduled = object()
 request_dropped = object()
 request_reached_downloader = object()
+request_left_downloader = object()
 response_received = object()
 response_downloaded = object()
 item_scraped = object()

scrapy/utils/python.py

@@ -1,15 +1,15 @@
 """
 This module contains essential stuff that should've come with Python itself ;)
 """
+import errno
 import gc
+import inspect
 import os
 import re
-import inspect
+import sys
 import weakref
-import errno
 from functools import partial, wraps
 from itertools import chain
-import sys
 
 from scrapy.utils.decorators import deprecated
 
@@ -371,10 +371,11 @@ else:
         gc.collect()
 
 
-class MutableChain(object):
+class MutableChain:
     """
     Thin wrapper around itertools.chain, allowing to add iterables "in-place"
     """
+
     def __init__(self, *args):
         self.data = chain(*args)
 

tests/spiders.py

@@ -117,6 +117,24 @@ class AsyncDefAsyncioReturnSpider(SimpleSpider):
         return [{'id': 1}, {'id': 2}]
 
 
+class AsyncDefAsyncioReqsReturnSpider(SimpleSpider):
+
+    name = 'asyncdef_asyncio_reqs_return'
+
+    async def parse(self, response):
+        await asyncio.sleep(0.2)
+        req_id = response.meta.get('req_id', 0)
+        status = await get_from_asyncio_queue(response.status)
+        self.logger.info("Got response %d, req_id %d" % (status, req_id))
+        if req_id > 0:
+            return
+        reqs = []
+        for i in range(1, 3):
+            req = Request(self.start_urls[0], dont_filter=True, meta={'req_id': i})
+            reqs.append(req)
+        return reqs
+
+
 class ItemSpider(FollowAllSpider):
 
     name = 'item'

tests/test_crawl.py

@@ -12,6 +12,7 @@ from scrapy.http import Request
 from scrapy.utils.python import to_unicode
 from tests.mockserver import MockServer
 from tests.spiders import (
+    AsyncDefAsyncioReqsReturnSpider,
     AsyncDefAsyncioReturnSpider,
     AsyncDefAsyncioSpider,
     AsyncDefSpider,
@@ -364,7 +365,7 @@ class CrawlSpiderTestCase(TestCase):
 
     @mark.only_asyncio()
     @defer.inlineCallbacks
-    def test_async_def_asyncio_parse_list(self):
+    def test_async_def_asyncio_parse_items_list(self):
         items = []
 
         def _on_item_scraped(item):
@@ -377,3 +378,12 @@ class CrawlSpiderTestCase(TestCase):
         self.assertIn("Got response 200", str(log))
         self.assertIn({'id': 1}, items)
         self.assertIn({'id': 2}, items)
+
+    @mark.only_asyncio()
+    @defer.inlineCallbacks
+    def test_async_def_asyncio_parse_reqs_list(self):
+        crawler = self.runner.create_crawler(AsyncDefAsyncioReqsReturnSpider)
+        with LogCapture() as log:
+            yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver)
+        for req_id in range(3):
+            self.assertIn("Got response 200, req_id %d" % req_id, str(log))

tests/test_dupefilters.py

@@ -2,6 +2,8 @@ import hashlib
 import tempfile
 import unittest
 import shutil
+import os
+import sys
 from testfixtures import LogCapture
 
 from scrapy.dupefilters import RFPDupeFilter
@@ -84,17 +86,21 @@ class RFPDupeFilterTest(unittest.TestCase):
         path = tempfile.mkdtemp()
         try:
             df = RFPDupeFilter(path)
-            df.open()
-            assert not df.request_seen(r1)
-            assert df.request_seen(r1)
-            df.close('finished')
+            try:
+                df.open()
+                assert not df.request_seen(r1)
+                assert df.request_seen(r1)
+            finally:
+                df.close('finished')
 
             df2 = RFPDupeFilter(path)
-            df2.open()
-            assert df2.request_seen(r1)
-            assert not df2.request_seen(r2)
-            assert df2.request_seen(r2)
-            df2.close('finished')
+            try:
+                df2.open()
+                assert df2.request_seen(r1)
+                assert not df2.request_seen(r2)
+                assert df2.request_seen(r2)
+            finally:
+                df2.close('finished')
         finally:
             shutil.rmtree(path)
 
@@ -129,6 +135,30 @@ class RFPDupeFilterTest(unittest.TestCase):
 
         case_insensitive_dupefilter.close('finished')
 
+    def test_seenreq_newlines(self):
+        """ Checks against adding duplicate \r to
+        line endings on Windows platforms. """
+
+        r1 = Request('http://scrapytest.org/1')
+
+        path = tempfile.mkdtemp()
+        try:
+            df = RFPDupeFilter(path)
+            df.open()
+            df.request_seen(r1)
+            df.close('finished')
+
+            with open(os.path.join(path, 'requests.seen'), 'rb') as seen_file:
+                line = next(seen_file).decode()
+                assert not line.endswith('\r\r\n')
+                if sys.platform == 'win32':
+                    assert line.endswith('\r\n')
+                else:
+                    assert line.endswith('\n')
+
+        finally:
+            shutil.rmtree(path)
+
     def test_log(self):
         with LogCapture() as l:
             settings = {'DUPEFILTER_DEBUG': False,

tests/test_request_left.py

@@ -0,0 +1,60 @@
+from twisted.internet import defer
+from twisted.trial.unittest import TestCase
+from scrapy.signals import request_left_downloader
+from scrapy.spiders import Spider
+from scrapy.utils.test import get_crawler
+from tests.mockserver import MockServer
+
+
+class SignalCatcherSpider(Spider):
+    name = 'signal_catcher'
+
+    def __init__(self, crawler, url, *args, **kwargs):
+        super(SignalCatcherSpider, self).__init__(*args, **kwargs)
+        crawler.signals.connect(self.on_request_left,
+                                signal=request_left_downloader)
+        self.caught_times = 0
+        self.start_urls = [url]
+
+    @classmethod
+    def from_crawler(cls, crawler, *args, **kwargs):
+        spider = cls(crawler, *args, **kwargs)
+        return spider
+
+    def on_request_left(self, request, spider):
+        self.caught_times = self.caught_times + 1
+
+
+class TestCatching(TestCase):
+
+    def setUp(self):
+        self.mockserver = MockServer()
+        self.mockserver.__enter__()
+
+    def tearDown(self):
+        self.mockserver.__exit__(None, None, None)
+
+    @defer.inlineCallbacks
+    def test_success(self):
+        crawler = get_crawler(SignalCatcherSpider)
+        yield crawler.crawl(self.mockserver.url("/status?n=200"))
+        self.assertEqual(crawler.spider.caught_times, 1)
+
+    @defer.inlineCallbacks
+    def test_timeout(self):
+        crawler = get_crawler(SignalCatcherSpider,
+                              {'DOWNLOAD_TIMEOUT': 0.1})
+        yield crawler.crawl(self.mockserver.url("/delay?n=0.2"))
+        self.assertEqual(crawler.spider.caught_times, 1)
+
+    @defer.inlineCallbacks
+    def test_disconnect(self):
+        crawler = get_crawler(SignalCatcherSpider)
+        yield crawler.crawl(self.mockserver.url("/drop"))
+        self.assertEqual(crawler.spider.caught_times, 1)
+
+    @defer.inlineCallbacks
+    def test_noconnect(self):
+        crawler = get_crawler(SignalCatcherSpider)
+        yield crawler.crawl('http://thereisdefinetelynosuchdomain.com')
+        self.assertEqual(crawler.spider.caught_times, 1)

tests/test_spidermiddleware_output_chain.py

@@ -1,10 +1,10 @@
-
 from testfixtures import LogCapture
-from twisted.trial.unittest import TestCase
 from twisted.internet import defer
+from twisted.trial.unittest import TestCase
 
-from scrapy import Spider, Request
+from scrapy import Request, Spider
 from scrapy.utils.test import get_crawler
+
 from tests.mockserver import MockServer
 
 
@@ -74,7 +74,7 @@ class ProcessSpiderInputSpiderWithErrback(ProcessSpiderInputSpiderWithoutErrback
     name = 'ProcessSpiderInputSpiderWithErrback'
 
     def start_requests(self):
-        yield Request(url=self.mockserver.url('/status?n=200'), callback=self.parse, errback=self.errback)
+        yield Request(self.mockserver.url('/status?n=200'), self.parse, errback=self.errback)
 
     def errback(self, failure):
         self.logger.info('Got a Failure on the Request errback')
@@ -100,6 +100,17 @@ class GeneratorCallbackSpider(Spider):
         raise ImportError()
 
 
+# ================================================================================
+# (2.1) exceptions from a spider callback (generator, middleware right after callback)
+class GeneratorCallbackSpiderMiddlewareRightAfterSpider(GeneratorCallbackSpider):
+    name = 'GeneratorCallbackSpiderMiddlewareRightAfterSpider'
+    custom_settings = {
+        'SPIDER_MIDDLEWARES': {
+            __name__ + '.LogExceptionMiddleware': 100000,
+        },
+    }
+
+
 # ================================================================================
 # (3) exceptions from a spider callback (not a generator)
 class NotGeneratorCallbackSpider(Spider):
@@ -117,6 +128,17 @@ class NotGeneratorCallbackSpider(Spider):
         return [{'test': 1}, {'test': 1/0}]
 
 
+# ================================================================================
+# (3.1) exceptions from a spider callback (not a generator, middleware right after callback)
+class NotGeneratorCallbackSpiderMiddlewareRightAfterSpider(NotGeneratorCallbackSpider):
+    name = 'NotGeneratorCallbackSpiderMiddlewareRightAfterSpider'
+    custom_settings = {
+        'SPIDER_MIDDLEWARES': {
+            __name__ + '.LogExceptionMiddleware': 100000,
+        },
+    }
+
+
 # ================================================================================
 # (4) exceptions from a middleware process_spider_output method (generator)
 class GeneratorOutputChainSpider(Spider):
@@ -320,6 +342,16 @@ class TestSpiderMiddleware(TestCase):
         self.assertIn("Middleware: ImportError exception caught", str(log2))
         self.assertIn("'item_scraped_count': 2", str(log2))
 
+    @defer.inlineCallbacks
+    def test_generator_callback_right_after_callback(self):
+        """
+        (2.1) Special case of (2): Exceptions should be caught
+        even if the middleware is placed right after the spider
+        """
+        log21 = yield self.crawl_log(GeneratorCallbackSpiderMiddlewareRightAfterSpider)
+        self.assertIn("Middleware: ImportError exception caught", str(log21))
+        self.assertIn("'item_scraped_count': 2", str(log21))
+
     @defer.inlineCallbacks
     def test_not_a_generator_callback(self):
         """
@@ -330,6 +362,16 @@ class TestSpiderMiddleware(TestCase):
         self.assertIn("Middleware: ZeroDivisionError exception caught", str(log3))
         self.assertNotIn("item_scraped_count", str(log3))
 
+    @defer.inlineCallbacks
+    def test_not_a_generator_callback_right_after_callback(self):
+        """
+        (3.1) Special case of (3): Exceptions should be caught
+        even if the middleware is placed right after the spider
+        """
+        log31 = yield self.crawl_log(NotGeneratorCallbackSpiderMiddlewareRightAfterSpider)
+        self.assertIn("Middleware: ZeroDivisionError exception caught", str(log31))
+        self.assertNotIn("item_scraped_count", str(log31))
+
     @defer.inlineCallbacks
     def test_generator_output_chain(self):
         """