Merge branch 'master' into add-artwork-license

docs/topics/downloader-middleware.rst

@@ -829,7 +829,7 @@ REDIRECT_MAX_TIMES
 Default: ``20``
 
 The maximum number of redirections that will be followed for a single request.
-After this maximum the request's response is returned as is.
+After this maximum, the request's response is returned as is.
 
 MetaRefreshMiddleware
 ---------------------

scrapy/core/downloader/middleware.py

@@ -40,14 +40,14 @@ class DownloaderMiddlewareManager(MiddlewareManager):
                         % (method.__self__.__class__.__name__, response.__class__.__name__)
                     )
                 if response:
-                    defer.returnValue(response)
-            defer.returnValue((yield download_func(request=request, spider=spider)))
+                    return response
+            return (yield download_func(request=request, spider=spider))
 
         @defer.inlineCallbacks
         def process_response(response):
             assert response is not None, 'Received None in process_response'
             if isinstance(response, Request):
-                defer.returnValue(response)
+                return response
 
             for method in self.methods['process_response']:
                 response = yield deferred_from_coro(method(request=request, response=response, spider=spider))
@@ -57,12 +57,12 @@ class DownloaderMiddlewareManager(MiddlewareManager):
                         % (method.__self__.__class__.__name__, type(response))
                     )
                 if isinstance(response, Request):
-                    defer.returnValue(response)
-            defer.returnValue(response)
+                    return response
+            return response
 
         @defer.inlineCallbacks
-        def process_exception(_failure):
-            exception = _failure.value
+        def process_exception(failure):
+            exception = failure.value
             for method in self.methods['process_exception']:
                 response = yield deferred_from_coro(method(request=request, exception=exception, spider=spider))
                 if response is not None and not isinstance(response, (Response, Request)):
@@ -71,8 +71,8 @@ class DownloaderMiddlewareManager(MiddlewareManager):
                         % (method.__self__.__class__.__name__, type(response))
                     )
                 if response:
-                    defer.returnValue(response)
-            defer.returnValue(_failure)
+                    return response
+            return failure
 
         deferred = mustbe_deferred(process_request, request)
         deferred.addErrback(process_exception)

scrapy/pipelines/media.py

@@ -1,7 +1,7 @@
 import functools
 import logging
 from collections import defaultdict
-from twisted.internet.defer import Deferred, DeferredList, _DefGen_Return
+from twisted.internet.defer import Deferred, DeferredList
 from twisted.python.failure import Failure
 
 from scrapy.settings import Settings
@@ -141,24 +141,26 @@ class MediaPipeline:
             # This code fixes a memory leak by avoiding to keep references to
             # the Request and Response objects on the Media Pipeline cache.
             #
-            # Twisted inline callbacks pass return values using the function
-            # twisted.internet.defer.returnValue, which encapsulates the return
-            # value inside a _DefGen_Return base exception.
-            #
-            # What happens when the media_downloaded callback raises another
+            # What happens when the media_downloaded callback raises an
             # exception, for example a FileException('download-error') when
-            # the Response status code is not 200 OK, is that it stores the
-            # _DefGen_Return exception on the FileException context.
+            # the Response status code is not 200 OK, is that the original
+            # StopIteration exception (which in turn contains the failed
+            # Response and by extension, the original Request) gets encapsulated
+            # within the FileException context.
+            #
+            # Originally, Scrapy was using twisted.internet.defer.returnValue
+            # inside functions decorated with twisted.internet.defer.inlineCallbacks,
+            # encapsulating the returned Response in a _DefGen_Return exception
+            # instead of a StopIteration.
             #
             # To avoid keeping references to the Response and therefore Request
             # objects on the Media Pipeline cache, we should wipe the context of
-            # the exception encapsulated by the Twisted Failure when its a
-            # _DefGen_Return instance.
+            # the encapsulated exception when it is a StopIteration instance
             #
             # This problem does not occur in Python 2.7 since we don't have
             # Exception Chaining (https://www.python.org/dev/peps/pep-3134/).
             context = getattr(result.value, '__context__', None)
-            if isinstance(context, _DefGen_Return):
+            if isinstance(context, StopIteration):
                 setattr(result.value, '__context__', None)
 
         info.downloading.remove(fp)

tests/test_feedexport.py

@@ -433,7 +433,7 @@ class FeedExportTest(unittest.TestCase):
             for file_path in FEEDS.keys():
                 os.remove(str(file_path))
 
-        defer.returnValue(content)
+        return content
 
     @defer.inlineCallbacks
     def exported_data(self, items, settings):
@@ -448,7 +448,7 @@ class FeedExportTest(unittest.TestCase):
                     yield item
 
         data = yield self.run_and_export(TestSpider, settings)
-        defer.returnValue(data)
+        return data
 
     @defer.inlineCallbacks
     def exported_no_data(self, settings):
@@ -462,7 +462,7 @@ class FeedExportTest(unittest.TestCase):
                 pass
 
         data = yield self.run_and_export(TestSpider, settings)
-        defer.returnValue(data)
+        return data
 
     @defer.inlineCallbacks
     def assertExportedCsv(self, items, header, rows, settings=None, ordered=True):

tests/test_pipeline_media.py

@@ -2,7 +2,7 @@ from testfixtures import LogCapture
 from twisted.trial import unittest
 from twisted.python.failure import Failure
 from twisted.internet import reactor
-from twisted.internet.defer import Deferred, inlineCallbacks, returnValue
+from twisted.internet.defer import Deferred, inlineCallbacks
 
 from scrapy.http import Request, Response
 from scrapy.settings import Settings
@@ -124,9 +124,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase):
         # Simulate the Media Pipeline behavior to produce a Twisted Failure
         try:
             # Simulate a Twisted inline callback returning a Response
-            # The returnValue method raises an exception encapsulating the value
-            returnValue(response)
-        except BaseException as exc:
+            raise StopIteration(response)
+        except StopIteration as exc:
             def_gen_return_exc = exc
             try:
                 # Simulate the media_downloaded callback raising a FileException
@@ -140,7 +139,7 @@ class BaseMediaPipelineTestCase(unittest.TestCase):
 
         # The Failure should encapsulate a FileException ...
         self.assertEqual(failure.value, file_exc)
-        # ... and it should have the returnValue exception set as its context
+        # ... and it should have the StopIteration exception set as its context
         self.assertEqual(failure.value.__context__, def_gen_return_exc)
 
         # Let's calculate the request fingerprint and fake some runtime data...
@@ -155,7 +154,7 @@ class BaseMediaPipelineTestCase(unittest.TestCase):
         self.assertEqual(info.downloaded[fp], failure)
         # ... encapsulating the original FileException ...
         self.assertEqual(info.downloaded[fp].value, file_exc)
-        # ... but it should not store the returnValue exception on its context
+        # ... but it should not store the StopIteration exception on its context
         context = getattr(info.downloaded[fp].value, '__context__', None)
         self.assertIsNone(context)
 

tests/test_spidermiddleware_output_chain.py

@@ -292,7 +292,7 @@ class TestSpiderMiddleware(TestCase):
         crawler = get_crawler(spider)
         with LogCapture() as log:
             yield crawler.crawl(mockserver=self.mockserver)
-        raise defer.returnValue(log)
+        return log
 
     @defer.inlineCallbacks
     def test_recovery(self):