From c1aab2f58ef7c701f2d7468add9392238a08f2e8 Mon Sep 17 00:00:00 2001 From: Pablo Hoffman Date: Wed, 8 Sep 2010 00:15:09 -0300 Subject: [PATCH] Copy callback/errback attributes when copying Requests --- docs/topics/request-response.rst | 24 +----------------------- scrapy/core/engine.py | 7 +------ scrapy/http/request/__init__.py | 2 +- scrapy/tests/test_http_request.py | 4 ++-- 4 files changed, 5 insertions(+), 32 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index f6c96a49e..6959070b7 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -138,7 +138,7 @@ Request objects Return a new Request which is a copy of this Request. See also: :ref:`topics-request-response-ref-request-callback-arguments`. - .. method:: Request.replace([url, callback, method, headers, body, cookies, meta, encoding, dont_filter]) + .. method:: Request.replace([url, callback, method, headers, body, cookies, meta, encoding, dont_filter, callback, errback]) Return a Request object with the same members, except for those members given new values by whichever keyword arguments are specified. The @@ -146,28 +146,6 @@ Request objects is given in the ``meta`` argument). See also :ref:`topics-request-response-ref-request-callback-arguments`. -.. _topics-request-response-ref-callback-copy: - -Caveats with copying Requests and callbacks -------------------------------------------- - -When you copy a request using the :meth:`Request.copy` or -:meth:`Request.replace` methods the callback of the request is not copied by -default. This is because of legacy reasons along with limitations in the -underlying network library, which doesn't allow sharing `Twisted deferreds`_. - -.. _Twisted deferreds: http://twistedmatrix.com/projects/core/documentation/howto/defer.html - -For example:: - - request = Request("http://www.example.com", callback=myfunc) - request2 = request.copy() # doesn't copy the callback - request3 = request.replace(callback=request.callback) - -In the above example, ``request2`` is a copy of ``request`` but it has no -callback, while ``request3`` is a copy of ``request`` and also contains the -callback. - .. _topics-request-response-ref-request-callback-arguments: Passing arguments to callback functions diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 17a683f7f..25d5dcc36 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -159,12 +159,7 @@ class ExecutionEngine(object): level=log.DEBUG, spider=spider) return response elif isinstance(response, Request): - newrequest = response - dfd = mustbe_deferred(self.schedule, newrequest, spider) - if newrequest.callback: - # XXX: this is a bit hacky and should be removed - dfd.addCallbacks(newrequest.callback, newrequest.errback) - return dfd + return mustbe_deferred(self.schedule, response, spider) def _on_error(_failure): """handle an error processing a page""" diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 1274127eb..9c669a1d8 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -99,7 +99,7 @@ class Request(object_ref): given new values. """ for x in ['url', 'method', 'headers', 'body', 'cookies', 'meta', \ - 'encoding', 'priority', 'dont_filter']: + 'encoding', 'priority', 'dont_filter', 'callback', 'errback']: kwargs.setdefault(x, getattr(self, x)) cls = kwargs.pop('cls', self.__class__) return cls(*args, **kwargs) diff --git a/scrapy/tests/test_http_request.py b/scrapy/tests/test_http_request.py index 6d2766a94..9248de464 100644 --- a/scrapy/tests/test_http_request.py +++ b/scrapy/tests/test_http_request.py @@ -131,8 +131,8 @@ class RequestTest(unittest.TestCase): # make sure copy does not propagate callbacks assert r1.callback is somecallback assert r1.errback is somecallback - assert r2.callback is None - assert r2.errback is None + assert r2.callback is r1.callback + assert r2.errback is r2.errback # make sure meta dict is shallow copied assert r1.meta is not r2.meta, "meta must be a shallow copy, not identical"