From 72b8613ee9827af031862bd84f1bea9acefcbebe Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 27 Nov 2019 14:46:20 -0300 Subject: [PATCH 01/20] bytes_received signal (no tests) --- docs/topics/signals.rst | 45 ++++++++++++++++------- scrapy/core/downloader/handlers/http11.py | 25 +++++++++++-- scrapy/signals.py | 1 + 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 3f29aa323..6efb73abb 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -73,7 +73,7 @@ engine_started Sent when the Scrapy engine has started crawling. - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. .. note:: This signal may be fired *after* the :signal:`spider_opened` signal, depending on how the spider was started. So **don't** rely on this signal @@ -88,7 +88,7 @@ engine_stopped Sent when the Scrapy engine is stopped (for example, when a crawling process has finished). - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. item_scraped ------------ @@ -99,7 +99,7 @@ item_scraped Sent when an item has been scraped, after it has passed all the :ref:`topics-item-pipeline` stages (without being dropped). - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. :param item: the item scraped :type item: dict or :class:`~scrapy.item.Item` object @@ -119,7 +119,7 @@ item_dropped Sent after an item has been dropped from the :ref:`topics-item-pipeline` when some stage raised a :exc:`~scrapy.exceptions.DropItem` exception. - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. :param item: the item dropped from the :ref:`topics-item-pipeline` :type item: dict or :class:`~scrapy.item.Item` object @@ -144,7 +144,7 @@ item_error Sent when a :ref:`topics-item-pipeline` generates an error (ie. raises an exception), except :exc:`~scrapy.exceptions.DropItem` exception. - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. :param item: the item dropped from the :ref:`topics-item-pipeline` :type item: dict or :class:`~scrapy.item.Item` object @@ -158,6 +158,23 @@ item_error :param failure: the exception raised :type failure: twisted.python.failure.Failure +bytes_received +-------------- + +.. signal:: bytes_received +.. function:: bytes_received(data, request) + + Sent by the HTTP 1.1 download handler when a group of bytes is + received for a specific request. + + This signal does not support returning deferreds from its handlers. + + :param data: the data received by the download handler + :type spider: :class:`bytes` object + + :param request: the request that generated the response + :type request: :class:`~scrapy.http.Request` object + spider_closed ------------- @@ -167,7 +184,7 @@ spider_closed Sent after a spider has been closed. This can be used to release per-spider resources reserved on :signal:`spider_opened`. - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. :param spider: the spider which has been closed :type spider: :class:`~scrapy.spiders.Spider` object @@ -191,7 +208,7 @@ spider_opened reserve per-spider resources, but can be used for any task that needs to be performed when a spider is opened. - This signal supports returning deferreds from their handlers. + This signal supports returning deferreds from its handlers. :param spider: the spider which has been opened :type spider: :class:`~scrapy.spiders.Spider` object @@ -215,7 +232,7 @@ spider_idle You may raise a :exc:`~scrapy.exceptions.DontCloseSpider` exception to prevent the spider from being closed. - This signal does not support returning deferreds from their handlers. + This signal does not support returning deferreds from its handlers. :param spider: the spider which has gone idle :type spider: :class:`~scrapy.spiders.Spider` object @@ -234,7 +251,7 @@ spider_error Sent when a spider callback generates an error (ie. raises an exception). - This signal does not support returning deferreds from their handlers. + This signal does not support returning deferreds from its handlers. :param failure: the exception raised :type failure: twisted.python.failure.Failure @@ -254,7 +271,7 @@ request_scheduled Sent when the engine schedules a :class:`~scrapy.http.Request`, to be downloaded later. - The signal does not support returning deferreds from their handlers. + The signal does not support returning deferreds from its handlers. :param request: the request that reached the scheduler :type request: :class:`~scrapy.http.Request` object @@ -271,7 +288,7 @@ request_dropped Sent when a :class:`~scrapy.http.Request`, scheduled by the engine to be downloaded later, is rejected by the scheduler. - The signal does not support returning deferreds from their handlers. + The signal does not support returning deferreds from its handlers. :param request: the request that reached the scheduler :type request: :class:`~scrapy.http.Request` object @@ -287,7 +304,7 @@ request_reached_downloader Sent when a :class:`~scrapy.http.Request` reached downloader. - The signal does not support returning deferreds from their handlers. + The signal does not support returning deferreds from its handlers. :param request: the request that reached downloader :type request: :class:`~scrapy.http.Request` object @@ -304,7 +321,7 @@ response_received Sent when the engine receives a new :class:`~scrapy.http.Response` from the downloader. - This signal does not support returning deferreds from their handlers. + This signal does not support returning deferreds from its handlers. :param response: the response received :type response: :class:`~scrapy.http.Response` object @@ -323,7 +340,7 @@ response_downloaded Sent by the downloader right after a ``HTTPResponse`` is downloaded. - This signal does not support returning deferreds from their handlers. + This signal does not support returning deferreds from its handlers. :param response: the response downloaded :type response: :class:`~scrapy.http.Response` object diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 5a5f6cf0a..92c3d5f5c 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -16,6 +16,7 @@ from twisted.web.http_headers import Headers as TxHeaders from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH from zope.interface import implementer +from scrapy import signals from scrapy.core.downloader.tls import openssl_methods from scrapy.core.downloader.webclient import _parse from scrapy.exceptions import ScrapyDeprecationWarning @@ -32,6 +33,7 @@ class HTTP11DownloadHandler: lazy = False def __init__(self, settings, crawler=None): + self.crawler = crawler self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN') self._pool._factory.noisy = False @@ -76,6 +78,7 @@ class HTTP11DownloadHandler: maxsize=getattr(spider, 'download_maxsize', self._default_maxsize), warnsize=getattr(spider, 'download_warnsize', self._default_warnsize), fail_on_dataloss=self._fail_on_dataloss, + crawler=self.crawler, ) return agent.download_request(request) @@ -272,7 +275,7 @@ class ScrapyAgent(object): _TunnelingAgent = TunnelingAgent def __init__(self, contextFactory=None, connectTimeout=10, bindAddress=None, pool=None, - maxsize=0, warnsize=0, fail_on_dataloss=True): + maxsize=0, warnsize=0, fail_on_dataloss=True, crawler=None): self._contextFactory = contextFactory self._connectTimeout = connectTimeout self._bindAddress = bindAddress @@ -281,6 +284,7 @@ class ScrapyAgent(object): self._warnsize = warnsize self._fail_on_dataloss = fail_on_dataloss self._txresponse = None + self._crawler = crawler def _get_agent(self, request, timeout): bindaddress = request.meta.get('bindaddress') or self._bindAddress @@ -409,7 +413,15 @@ class ScrapyAgent(object): d = defer.Deferred(_cancel) txresponse.deliverBody( - _ResponseReader(d, txresponse, request, maxsize, warnsize, fail_on_dataloss) + _ResponseReader( + d, + txresponse, + request, + maxsize, + warnsize, + fail_on_dataloss, + self._crawler, + ) ) # save response for timeouts @@ -445,7 +457,7 @@ class _RequestBodyProducer(object): class _ResponseReader(protocol.Protocol): - def __init__(self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss): + def __init__(self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss, crawler): self._finished = finished self._txresponse = txresponse self._request = request @@ -456,6 +468,7 @@ class _ResponseReader(protocol.Protocol): self._fail_on_dataloss_warned = False self._reached_warnsize = False self._bytes_received = 0 + self._crawler = crawler def dataReceived(self, bodyBytes): # This maybe called several times after cancel was called with buffered data. @@ -465,6 +478,12 @@ class _ResponseReader(protocol.Protocol): self._bodybuf.write(bodyBytes) self._bytes_received += len(bodyBytes) + self._crawler.signals.send_catch_log( + signal=signals.bytes_received, + data=bodyBytes, + request=self._request, + ) + if self._maxsize and self._bytes_received > self._maxsize: logger.error("Received (%(bytes)s) bytes larger than download " "max size (%(maxsize)s) in request %(request)s.", diff --git a/scrapy/signals.py b/scrapy/signals.py index 6b9125302..590421893 100644 --- a/scrapy/signals.py +++ b/scrapy/signals.py @@ -16,6 +16,7 @@ request_dropped = object() request_reached_downloader = object() response_received = object() response_downloaded = object() +bytes_received = object() item_scraped = object() item_dropped = object() item_error = object() From cab449b1952020b86fbe2915a537150fc885c567 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 29 Nov 2019 11:37:40 -0300 Subject: [PATCH 02/20] Typo fix --- tests/test_engine.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index 25dee7c1f..9d68836cc 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -107,7 +107,7 @@ class CrawlerRun(object): self.reqreached = [] self.itemerror = [] self.itemresp = [] - self.signals_catched = {} + self.signals_caught = {} self.spider_class = spider_class def run(self): @@ -172,7 +172,7 @@ class CrawlerRun(object): signalargs = kwargs.copy() sig = signalargs.pop('signal') signalargs.pop('sender', None) - self.signals_catched[sig] = signalargs + self.signals_caught[sig] = signalargs class EngineTest(unittest.TestCase): @@ -186,7 +186,7 @@ class EngineTest(unittest.TestCase): self._assert_scheduled_requests(urls_to_visit=8) self._assert_downloaded_responses() self._assert_scraped_items() - self._assert_signals_catched() + self._assert_signals_caught() @defer.inlineCallbacks def test_crawler_dupefilter(self): @@ -263,19 +263,19 @@ class EngineTest(unittest.TestCase): self.assertEqual('Item 2 name', item['name']) self.assertEqual('200', item['price']) - def _assert_signals_catched(self): - assert signals.engine_started in self.run.signals_catched - assert signals.engine_stopped in self.run.signals_catched - assert signals.spider_opened in self.run.signals_catched - assert signals.spider_idle in self.run.signals_catched - assert signals.spider_closed in self.run.signals_catched + def _assert_signals_caught(self): + assert signals.engine_started in self.run.signals_caught + assert signals.engine_stopped in self.run.signals_caught + assert signals.spider_opened in self.run.signals_caught + assert signals.spider_idle in self.run.signals_caught + assert signals.spider_closed in self.run.signals_caught self.assertEqual({'spider': self.run.spider}, - self.run.signals_catched[signals.spider_opened]) + self.run.signals_caught[signals.spider_opened]) self.assertEqual({'spider': self.run.spider}, - self.run.signals_catched[signals.spider_idle]) + self.run.signals_caught[signals.spider_idle]) self.assertEqual({'spider': self.run.spider, 'reason': 'finished'}, - self.run.signals_catched[signals.spider_closed]) + self.run.signals_caught[signals.spider_closed]) @defer.inlineCallbacks def test_close_downloader(self): From bda37e38bd53d5aae691b56d4136fbff99f78158 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 29 Nov 2019 12:02:27 -0300 Subject: [PATCH 03/20] [Tests] bytes_received signal --- tests/test_engine.py | 53 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index 9d68836cc..b63c7e232 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -13,22 +13,24 @@ module with the ``runserver`` argument:: import os import re import sys +from collections import defaultdict from urllib.parse import urlparse from twisted.internet import reactor, defer -from twisted.web import server, static, util from twisted.trial import unittest +from twisted.web import server, static, util +from pydispatch import dispatcher from scrapy import signals from scrapy.core.engine import ExecutionEngine -from scrapy.utils.test import get_crawler -from pydispatch import dispatcher -from tests import tests_datadir -from scrapy.spiders import Spider +from scrapy.http import Request from scrapy.item import Item, Field from scrapy.linkextractors import LinkExtractor -from scrapy.http import Request +from scrapy.spiders import Spider from scrapy.utils.signal import disconnect_all +from scrapy.utils.test import get_crawler + +from tests import tests_datadir, get_testdata class TestItem(Item): @@ -107,6 +109,7 @@ class CrawlerRun(object): self.reqreached = [] self.itemerror = [] self.itemresp = [] + self.bytes = defaultdict(lambda: b"") self.signals_caught = {} self.spider_class = spider_class @@ -124,6 +127,7 @@ class CrawlerRun(object): self.crawler = get_crawler(self.spider_class) self.crawler.signals.connect(self.item_scraped, signals.item_scraped) self.crawler.signals.connect(self.item_error, signals.item_error) + self.crawler.signals.connect(self.bytes_received, signals.bytes_received) self.crawler.signals.connect(self.request_scheduled, signals.request_scheduled) self.crawler.signals.connect(self.request_dropped, signals.request_dropped) self.crawler.signals.connect(self.request_reached, signals.request_reached_downloader) @@ -155,6 +159,9 @@ class CrawlerRun(object): def item_scraped(self, item, spider, response): self.itemresp.append((item, response)) + def bytes_received(self, data, request): + self.bytes[request] += data + def request_scheduled(self, request, spider): self.reqplug.append((request, spider)) @@ -187,6 +194,7 @@ class EngineTest(unittest.TestCase): self._assert_downloaded_responses() self._assert_scraped_items() self._assert_signals_caught() + self._assert_bytes_received() @defer.inlineCallbacks def test_crawler_dupefilter(self): @@ -263,6 +271,39 @@ class EngineTest(unittest.TestCase): self.assertEqual('Item 2 name', item['name']) self.assertEqual('200', item['price']) + def _assert_bytes_received(self): + self.assertEqual(8, len(self.run.bytes)) + for request, data in self.run.bytes.items(): + if self.run.getpath(request.url) == "/": + self.assertEqual(data, get_testdata("test_site", "index.html")) + elif self.run.getpath(request.url) == "/item1.html": + self.assertEqual(data, get_testdata("test_site", "item1.html")) + elif self.run.getpath(request.url) == "/item2.html": + self.assertEqual(data, get_testdata("test_site", "item2.html")) + elif self.run.getpath(request.url) == "/redirected": + self.assertEqual(data, b"Redirected here") + elif self.run.getpath(request.url) == '/redirect': + self.assertEqual(data, + b"\n\n" + b" \n" + b" \n" + b" \n" + b" \n" + b" click here\n" + b" \n" + b"\n" + ) + elif self.run.getpath(request.url) == "/tem999.html": + self.assertEqual(data, + b"\n\n" + b" 404 - No Such Resource\n" + b" \n" + b"

No Such Resource

\n" + b"

File not found.

\n" + b" \n" + b"\n" + ) + def _assert_signals_caught(self): assert signals.engine_started in self.run.signals_caught assert signals.engine_stopped in self.run.signals_caught From 89483ce9f709e230ee5ff9050d206430d2d17c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 3 Dec 2019 12:06:08 +0100 Subject: [PATCH 04/20] Fix Flake8 issues --- tests/test_engine.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index b63c7e232..c0769c992 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -283,7 +283,8 @@ class EngineTest(unittest.TestCase): elif self.run.getpath(request.url) == "/redirected": self.assertEqual(data, b"Redirected here") elif self.run.getpath(request.url) == '/redirect': - self.assertEqual(data, + self.assertEqual( + data, b"\n\n" b" \n" b" \n" @@ -294,7 +295,8 @@ class EngineTest(unittest.TestCase): b"\n" ) elif self.run.getpath(request.url) == "/tem999.html": - self.assertEqual(data, + self.assertEqual( + data, b"\n\n" b" 404 - No Such Resource\n" b" \n" From dbe20a863ff63dce937b2d3b159782d8268e6838 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 27 Jan 2020 12:21:18 -0300 Subject: [PATCH 05/20] bytes_received signal: send spider argument --- docs/topics/signals.rst | 5 ++++- scrapy/core/downloader/handlers/http11.py | 1 + tests/test_engine.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 6efb73abb..3e70ca067 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -162,7 +162,7 @@ bytes_received -------------- .. signal:: bytes_received -.. function:: bytes_received(data, request) +.. function:: bytes_received(data, request, spider) Sent by the HTTP 1.1 download handler when a group of bytes is received for a specific request. @@ -175,6 +175,9 @@ bytes_received :param request: the request that generated the response :type request: :class:`~scrapy.http.Request` object + :param spider: the spider associated with the response + :type spider: :class:`~scrapy.spiders.Spider` object + spider_closed ------------- diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 92c3d5f5c..c53c9bb2d 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -482,6 +482,7 @@ class _ResponseReader(protocol.Protocol): signal=signals.bytes_received, data=bodyBytes, request=self._request, + spider=self._crawler.spider, ) if self._maxsize and self._bytes_received > self._maxsize: diff --git a/tests/test_engine.py b/tests/test_engine.py index c0769c992..57cc89ba3 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -159,7 +159,7 @@ class CrawlerRun(object): def item_scraped(self, item, spider, response): self.itemresp.append((item, response)) - def bytes_received(self, data, request): + def bytes_received(self, data, request, spider): self.bytes[request] += data def request_scheduled(self, request, spider): From 613fd41f44d1455f9c9369087958674f3fdfcc8d Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 27 Jan 2020 12:30:26 -0300 Subject: [PATCH 06/20] bytes_received signal: improve test performance --- tests/test_engine.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index 57cc89ba3..bb475958e 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -109,7 +109,7 @@ class CrawlerRun(object): self.reqreached = [] self.itemerror = [] self.itemresp = [] - self.bytes = defaultdict(lambda: b"") + self.bytes = defaultdict(lambda: list()) self.signals_caught = {} self.spider_class = spider_class @@ -160,7 +160,7 @@ class CrawlerRun(object): self.itemresp.append((item, response)) def bytes_received(self, data, request, spider): - self.bytes[request] += data + self.bytes[request].append(data) def request_scheduled(self, request, spider): self.reqplug.append((request, spider)) @@ -274,17 +274,18 @@ class EngineTest(unittest.TestCase): def _assert_bytes_received(self): self.assertEqual(8, len(self.run.bytes)) for request, data in self.run.bytes.items(): + joined_data = b"".join(data) if self.run.getpath(request.url) == "/": - self.assertEqual(data, get_testdata("test_site", "index.html")) + self.assertEqual(joined_data, get_testdata("test_site", "index.html")) elif self.run.getpath(request.url) == "/item1.html": - self.assertEqual(data, get_testdata("test_site", "item1.html")) + self.assertEqual(joined_data, get_testdata("test_site", "item1.html")) elif self.run.getpath(request.url) == "/item2.html": - self.assertEqual(data, get_testdata("test_site", "item2.html")) + self.assertEqual(joined_data, get_testdata("test_site", "item2.html")) elif self.run.getpath(request.url) == "/redirected": - self.assertEqual(data, b"Redirected here") + self.assertEqual(joined_data, b"Redirected here") elif self.run.getpath(request.url) == '/redirect': self.assertEqual( - data, + joined_data, b"\n\n" b" \n" b" \n" @@ -296,7 +297,7 @@ class EngineTest(unittest.TestCase): ) elif self.run.getpath(request.url) == "/tem999.html": self.assertEqual( - data, + joined_data, b"\n\n" b" 404 - No Such Resource\n" b" \n" From 4ffd18fb11ff89863569b8b4de44241e3ca2f86e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 27 Jan 2020 13:29:33 -0300 Subject: [PATCH 07/20] [docs] Mention that signals.bytes_received could be fired multiple times --- docs/topics/signals.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 3e70ca067..f490911f3 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -165,7 +165,8 @@ bytes_received .. function:: bytes_received(data, request, spider) Sent by the HTTP 1.1 download handler when a group of bytes is - received for a specific request. + received for a specific request. This signal might be fired + multiple times for the same request. This signal does not support returning deferreds from its handlers. From 2c9643d38cc076c4d2032efd994fda4cfcc9f88a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 29 Jan 2020 14:11:56 -0300 Subject: [PATCH 08/20] Test: bytes_received signal fired multiple times --- tests/test_engine.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index bb475958e..3c5cc403b 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -12,6 +12,7 @@ module with the ``runserver`` argument:: import os import re +import string import sys from collections import defaultdict from urllib.parse import urlparse @@ -90,6 +91,7 @@ def start_test_site(debug=False): r = static.File(root_dir) r.putChild(b"redirect", util.Redirect(b"/redirected")) r.putChild(b"redirected", static.Data(b"Redirected here", "text/plain")) + r.putChild(b"random", static.Data(string.ascii_letters.encode("utf8") * 2**14, "text/plain")) port = reactor.listenTCP(0, server.Site(r), interface="127.0.0.1") if debug: @@ -117,8 +119,12 @@ class CrawlerRun(object): self.port = start_test_site() self.portno = self.port.getHost().port - start_urls = [self.geturl("/"), self.geturl("/redirect"), - self.geturl("/redirect")] # a duplicate + start_urls = [ + self.geturl("/"), + self.geturl("/redirect"), + self.geturl("/redirect"), # duplicate + self.geturl("/random"), + ] for name, signal in vars(signals).items(): if not name.startswith('_'): @@ -190,7 +196,7 @@ class EngineTest(unittest.TestCase): self.run = CrawlerRun(spider) yield self.run.run() self._assert_visited_urls() - self._assert_scheduled_requests(urls_to_visit=8) + self._assert_scheduled_requests(urls_to_visit=9) self._assert_downloaded_responses() self._assert_scraped_items() self._assert_signals_caught() @@ -200,7 +206,7 @@ class EngineTest(unittest.TestCase): def test_crawler_dupefilter(self): self.run = CrawlerRun(TestDupeFilterSpider) yield self.run.run() - self._assert_scheduled_requests(urls_to_visit=7) + self._assert_scheduled_requests(urls_to_visit=8) self._assert_dropped_requests() @defer.inlineCallbacks @@ -237,8 +243,8 @@ class EngineTest(unittest.TestCase): def _assert_downloaded_responses(self): # response tests - self.assertEqual(8, len(self.run.respplug)) - self.assertEqual(8, len(self.run.reqreached)) + self.assertEqual(9, len(self.run.respplug)) + self.assertEqual(9, len(self.run.reqreached)) for response, _ in self.run.respplug: if self.run.getpath(response.url) == '/item999.html': @@ -272,7 +278,7 @@ class EngineTest(unittest.TestCase): self.assertEqual('200', item['price']) def _assert_bytes_received(self): - self.assertEqual(8, len(self.run.bytes)) + self.assertEqual(9, len(self.run.bytes)) for request, data in self.run.bytes.items(): joined_data = b"".join(data) if self.run.getpath(request.url) == "/": @@ -306,6 +312,8 @@ class EngineTest(unittest.TestCase): b" \n" b"\n" ) + elif self.run.getpath(request.url) == "/random": + self.assertTrue(len(data) > 1) # signal was fired multiple times def _assert_signals_caught(self): assert signals.engine_started in self.run.signals_caught From a499f38b14d16338d20084c0dcb24528a1f1f22f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 29 Jan 2020 14:35:17 -0300 Subject: [PATCH 09/20] Remove object parent class --- scrapy/core/downloader/handlers/http11.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index c53c9bb2d..6f1bd9ad6 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -268,7 +268,7 @@ class ScrapyProxyAgent(Agent): ) -class ScrapyAgent(object): +class ScrapyAgent: _Agent = Agent _ProxyAgent = ScrapyProxyAgent @@ -438,7 +438,7 @@ class ScrapyAgent(object): @implementer(IBodyProducer) -class _RequestBodyProducer(object): +class _RequestBodyProducer: def __init__(self, body): self.body = body From 6f02a8dccb95373f22bac18c08d9fda8169dcb02 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 29 Jan 2020 14:53:23 -0300 Subject: [PATCH 10/20] Add source parameter to bytes_received signal --- docs/topics/signals.rst | 12 ++++++++---- scrapy/core/downloader/handlers/http11.py | 18 +++++++++++++----- scrapy/core/downloader/handlers/s3.py | 1 + tests/test_downloader_handlers.py | 3 +++ tests/test_engine.py | 5 ++++- 5 files changed, 29 insertions(+), 10 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index f490911f3..3a15bf95c 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -162,11 +162,11 @@ bytes_received -------------- .. signal:: bytes_received -.. function:: bytes_received(data, request, spider) +.. function:: bytes_received(data, request, spider, source) - Sent by the HTTP 1.1 download handler when a group of bytes is - received for a specific request. This signal might be fired - multiple times for the same request. + Sent by the HTTP 1.1 and S3 download handlers when a group of bytes is + received for a specific request. This signal might be fired multiple + times for the same request, with partial data each time. This signal does not support returning deferreds from its handlers. @@ -179,6 +179,10 @@ bytes_received :param spider: the spider associated with the response :type spider: :class:`~scrapy.spiders.Spider` object + :param source: a string to identify which handler sent the signal + (current values could be "http11" or "s3") + :type source: :class:`str` object + spider_closed ------------- diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 6f1bd9ad6..49c9eacac 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -32,8 +32,9 @@ logger = logging.getLogger(__name__) class HTTP11DownloadHandler: lazy = False - def __init__(self, settings, crawler=None): + def __init__(self, settings, crawler=None, source="http11"): self.crawler = crawler + self.source = source self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN') self._pool._factory.noisy = False @@ -67,8 +68,8 @@ class HTTP11DownloadHandler: self._disconnect_timeout = 1 @classmethod - def from_crawler(cls, crawler): - return cls(crawler.settings, crawler) + def from_crawler(cls, crawler, **kwargs): + return cls(crawler.settings, crawler, **kwargs) def download_request(self, request, spider): """Return a deferred for the HTTP download""" @@ -79,6 +80,7 @@ class HTTP11DownloadHandler: warnsize=getattr(spider, 'download_warnsize', self._default_warnsize), fail_on_dataloss=self._fail_on_dataloss, crawler=self.crawler, + source=self.source, ) return agent.download_request(request) @@ -275,7 +277,7 @@ class ScrapyAgent: _TunnelingAgent = TunnelingAgent def __init__(self, contextFactory=None, connectTimeout=10, bindAddress=None, pool=None, - maxsize=0, warnsize=0, fail_on_dataloss=True, crawler=None): + maxsize=0, warnsize=0, fail_on_dataloss=True, crawler=None, source=None): self._contextFactory = contextFactory self._connectTimeout = connectTimeout self._bindAddress = bindAddress @@ -285,6 +287,7 @@ class ScrapyAgent: self._fail_on_dataloss = fail_on_dataloss self._txresponse = None self._crawler = crawler + self._source = source def _get_agent(self, request, timeout): bindaddress = request.meta.get('bindaddress') or self._bindAddress @@ -421,6 +424,7 @@ class ScrapyAgent: warnsize, fail_on_dataloss, self._crawler, + self._source, ) ) @@ -457,7 +461,9 @@ class _RequestBodyProducer: class _ResponseReader(protocol.Protocol): - def __init__(self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss, crawler): + def __init__( + self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss, crawler, source + ): self._finished = finished self._txresponse = txresponse self._request = request @@ -469,6 +475,7 @@ class _ResponseReader(protocol.Protocol): self._reached_warnsize = False self._bytes_received = 0 self._crawler = crawler + self._source = source def dataReceived(self, bodyBytes): # This maybe called several times after cancel was called with buffered data. @@ -483,6 +490,7 @@ class _ResponseReader(protocol.Protocol): data=bodyBytes, request=self._request, spider=self._crawler.spider, + source=self._source, ) if self._maxsize and self._bytes_received > self._maxsize: diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 40a1fa48e..2366b6394 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -73,6 +73,7 @@ class S3DownloadHandler: objcls=httpdownloadhandler, settings=settings, crawler=crawler, + source="s3", ) self._download_http = _http_handler.download_request diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 8d95d7cac..22a813647 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -730,6 +730,9 @@ class Http11ProxyTestCase(HttpProxyTestCase): class HttpDownloadHandlerMock: + def __init__(self, *args, **kwargs): + pass + def download_request(self, request, spider): return request diff --git a/tests/test_engine.py b/tests/test_engine.py index 3c5cc403b..c83a23b55 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -112,6 +112,7 @@ class CrawlerRun(object): self.itemerror = [] self.itemresp = [] self.bytes = defaultdict(lambda: list()) + self.bytes_source = set() self.signals_caught = {} self.spider_class = spider_class @@ -165,8 +166,9 @@ class CrawlerRun(object): def item_scraped(self, item, spider, response): self.itemresp.append((item, response)) - def bytes_received(self, data, request, spider): + def bytes_received(self, data, request, spider, source): self.bytes[request].append(data) + self.bytes_source.add(source) def request_scheduled(self, request, spider): self.reqplug.append((request, spider)) @@ -279,6 +281,7 @@ class EngineTest(unittest.TestCase): def _assert_bytes_received(self): self.assertEqual(9, len(self.run.bytes)) + self.assertEqual(self.run.bytes_source, set(["http11"])) for request, data in self.run.bytes.items(): joined_data = b"".join(data) if self.run.getpath(request.url) == "/": From a64fa2f0866c10594f1e5cf00a0161f9fea1eb62 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 10 Feb 2020 10:16:05 -0300 Subject: [PATCH 11/20] Keyword arguments when creating a _ResponseReader --- scrapy/core/downloader/handlers/http11.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 49c9eacac..7a1a77b23 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -417,14 +417,14 @@ class ScrapyAgent: d = defer.Deferred(_cancel) txresponse.deliverBody( _ResponseReader( - d, - txresponse, - request, - maxsize, - warnsize, - fail_on_dataloss, - self._crawler, - self._source, + finished=d, + txresponse=txresponse, + request=request, + maxsize=maxsize, + warnsize=warnsize, + fail_on_dataloss=fail_on_dataloss, + crawler=self._crawler, + source=self._source, ) ) From 122ce6d6fb3861d99ba2f2810b2370056bae1190 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 10 Feb 2020 10:20:26 -0300 Subject: [PATCH 12/20] Check bytes are received in order (bytes_received signal) --- tests/test_engine.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/test_engine.py b/tests/test_engine.py index c83a23b55..0d970928b 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -12,7 +12,6 @@ module with the ``runserver`` argument:: import os import re -import string import sys from collections import defaultdict from urllib.parse import urlparse @@ -91,7 +90,8 @@ def start_test_site(debug=False): r = static.File(root_dir) r.putChild(b"redirect", util.Redirect(b"/redirected")) r.putChild(b"redirected", static.Data(b"Redirected here", "text/plain")) - r.putChild(b"random", static.Data(string.ascii_letters.encode("utf8") * 2**14, "text/plain")) + numbers = [str(x).encode("utf8") for x in range(2**14)] + r.putChild(b"numbers", static.Data(b"".join(numbers), "text/plain")) port = reactor.listenTCP(0, server.Site(r), interface="127.0.0.1") if debug: @@ -124,7 +124,7 @@ class CrawlerRun(object): self.geturl("/"), self.geturl("/redirect"), self.geturl("/redirect"), # duplicate - self.geturl("/random"), + self.geturl("/numbers"), ] for name, signal in vars(signals).items(): @@ -315,8 +315,12 @@ class EngineTest(unittest.TestCase): b" \n" b"\n" ) - elif self.run.getpath(request.url) == "/random": - self.assertTrue(len(data) > 1) # signal was fired multiple times + elif self.run.getpath(request.url) == "/numbers": + # signal was fired multiple times + self.assertTrue(len(data) > 1) + # bytes were received in order + numbers = [str(x).encode("utf8") for x in range(2**14)] + self.assertEqual(joined_data, b"".join(numbers)) def _assert_signals_caught(self): assert signals.engine_started in self.run.signals_caught From 42b4e9b3372ce3f9da57c7512b31a3c455b8a161 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 10 Feb 2020 11:23:38 -0300 Subject: [PATCH 13/20] Reword signal docs --- docs/topics/signals.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 3a15bf95c..dfb87cef3 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -279,7 +279,7 @@ request_scheduled Sent when the engine schedules a :class:`~scrapy.http.Request`, to be downloaded later. - The signal does not support returning deferreds from its handlers. + This signal does not support returning deferreds from its handlers. :param request: the request that reached the scheduler :type request: :class:`~scrapy.http.Request` object @@ -296,7 +296,7 @@ request_dropped Sent when a :class:`~scrapy.http.Request`, scheduled by the engine to be downloaded later, is rejected by the scheduler. - The signal does not support returning deferreds from its handlers. + This signal does not support returning deferreds from its handlers. :param request: the request that reached the scheduler :type request: :class:`~scrapy.http.Request` object @@ -312,7 +312,7 @@ request_reached_downloader Sent when a :class:`~scrapy.http.Request` reached downloader. - The signal does not support returning deferreds from its handlers. + This signal does not support returning deferreds from its handlers. :param request: the request that reached downloader :type request: :class:`~scrapy.http.Request` object From c4a5e3f0da3e674ecb7393c0894098984d6aa571 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 6 Apr 2020 09:26:13 -0300 Subject: [PATCH 14/20] Simplify bytes_received signal Remove "source" parameter --- docs/topics/signals.rst | 6 +----- scrapy/core/downloader/handlers/http11.py | 18 +++++------------- scrapy/core/downloader/handlers/s3.py | 1 - tests/test_engine.py | 5 +---- 4 files changed, 7 insertions(+), 23 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 97be46f2a..02fa6e287 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -163,7 +163,7 @@ bytes_received -------------- .. signal:: bytes_received -.. function:: bytes_received(data, request, spider, source) +.. function:: bytes_received(data, request, spider) Sent by the HTTP 1.1 and S3 download handlers when a group of bytes is received for a specific request. This signal might be fired multiple @@ -180,10 +180,6 @@ bytes_received :param spider: the spider associated with the response :type spider: :class:`~scrapy.spiders.Spider` object - :param source: a string to identify which handler sent the signal - (current values could be "http11" or "s3") - :type source: :class:`str` object - spider_closed ------------- diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index c350cd3c2..bda21b6b9 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -33,9 +33,8 @@ logger = logging.getLogger(__name__) class HTTP11DownloadHandler: lazy = False - def __init__(self, settings, crawler=None, source="http11"): + def __init__(self, settings, crawler=None): self._crawler = crawler - self._source = source from twisted.internet import reactor self._pool = HTTPConnectionPool(reactor, persistent=True) @@ -71,8 +70,8 @@ class HTTP11DownloadHandler: self._disconnect_timeout = 1 @classmethod - def from_crawler(cls, crawler, **kwargs): - return cls(crawler.settings, crawler, **kwargs) + def from_crawler(cls, crawler): + return cls(crawler.settings, crawler) def download_request(self, request, spider): """Return a deferred for the HTTP download""" @@ -83,7 +82,6 @@ class HTTP11DownloadHandler: warnsize=getattr(spider, 'download_warnsize', self._default_warnsize), fail_on_dataloss=self._fail_on_dataloss, crawler=self._crawler, - source=self._source, ) return agent.download_request(request) @@ -281,7 +279,7 @@ class ScrapyAgent: _TunnelingAgent = TunnelingAgent def __init__(self, contextFactory=None, connectTimeout=10, bindAddress=None, pool=None, - maxsize=0, warnsize=0, fail_on_dataloss=True, crawler=None, source=None): + maxsize=0, warnsize=0, fail_on_dataloss=True, crawler=None): self._contextFactory = contextFactory self._connectTimeout = connectTimeout self._bindAddress = bindAddress @@ -291,7 +289,6 @@ class ScrapyAgent: self._fail_on_dataloss = fail_on_dataloss self._txresponse = None self._crawler = crawler - self._source = source def _get_agent(self, request, timeout): from twisted.internet import reactor @@ -430,7 +427,6 @@ class ScrapyAgent: warnsize=warnsize, fail_on_dataloss=fail_on_dataloss, crawler=self._crawler, - source=self._source, ) ) @@ -468,9 +464,7 @@ class _RequestBodyProducer: class _ResponseReader(protocol.Protocol): - def __init__( - self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss, crawler, source - ): + def __init__(self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss, crawler): self._finished = finished self._txresponse = txresponse self._request = request @@ -483,7 +477,6 @@ class _ResponseReader(protocol.Protocol): self._bytes_received = 0 self._certificate = None self._crawler = crawler - self._source = source def connectionMade(self): if self._certificate is None: @@ -503,7 +496,6 @@ class _ResponseReader(protocol.Protocol): data=bodyBytes, request=self._request, spider=self._crawler.spider, - source=self._source, ) if self._maxsize and self._bytes_received > self._maxsize: diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 2366b6394..40a1fa48e 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -73,7 +73,6 @@ class S3DownloadHandler: objcls=httpdownloadhandler, settings=settings, crawler=crawler, - source="s3", ) self._download_http = _http_handler.download_request diff --git a/tests/test_engine.py b/tests/test_engine.py index 26f3163cf..acfe94f63 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -112,7 +112,6 @@ class CrawlerRun: self.itemerror = [] self.itemresp = [] self.bytes = defaultdict(lambda: list()) - self.bytes_source = set() self.signals_caught = {} self.spider_class = spider_class @@ -166,9 +165,8 @@ class CrawlerRun: def item_scraped(self, item, spider, response): self.itemresp.append((item, response)) - def bytes_received(self, data, request, spider, source): + def bytes_received(self, data, request, spider): self.bytes[request].append(data) - self.bytes_source.add(source) def request_scheduled(self, request, spider): self.reqplug.append((request, spider)) @@ -281,7 +279,6 @@ class EngineTest(unittest.TestCase): def _assert_bytes_received(self): self.assertEqual(9, len(self.run.bytes)) - self.assertEqual(self.run.bytes_source, set(["http11"])) for request, data in self.run.bytes.items(): joined_data = b"".join(data) if self.run.getpath(request.url) == "/": From e1948b492317eb5b11550d119d91c61b74b3a37f Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 4 May 2020 09:07:27 -0300 Subject: [PATCH 15/20] Add example about bytes_received signal --- docs/topics/signals.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index bc04faad5..7fe63a7b0 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -378,7 +378,9 @@ bytes_received Sent by the HTTP 1.1 and S3 download handlers when a group of bytes is received for a specific request. This signal might be fired multiple - times for the same request, with partial data each time. + times for the same request, with partial data each time. For instance, + a possible scenario for a 25 kb response would be two signals fired + with 10 kb of data, and a final one with 5 kb of data. This signal does not support returning deferreds from its handlers. From d0bb04f08936435202488404d08c0b82f25aa1e5 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 6 May 2020 13:37:23 -0300 Subject: [PATCH 16/20] Switch to pickle protocol 4 --- scrapy/exporters.py | 2 +- scrapy/extensions/httpcache.py | 4 ++-- scrapy/extensions/spiderstate.py | 2 +- scrapy/squeues.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 0cb6cef98..349a9586b 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -250,7 +250,7 @@ class CsvItemExporter(BaseItemExporter): class PickleItemExporter(BaseItemExporter): - def __init__(self, file, protocol=2, **kwargs): + def __init__(self, file, protocol=4, **kwargs): super().__init__(**kwargs) self.file = file self.protocol = protocol diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index 8546628a8..7972b58b1 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -250,7 +250,7 @@ class DbmCacheStorage: 'headers': dict(response.headers), 'body': response.body, } - self.db['%s_data' % key] = pickle.dumps(data, protocol=2) + self.db['%s_data' % key] = pickle.dumps(data, protocol=4) self.db['%s_time' % key] = str(time()) def _read_data(self, spider, request): @@ -317,7 +317,7 @@ class FilesystemCacheStorage: with self._open(os.path.join(rpath, 'meta'), 'wb') as f: f.write(to_bytes(repr(metadata))) with self._open(os.path.join(rpath, 'pickled_meta'), 'wb') as f: - pickle.dump(metadata, f, protocol=2) + pickle.dump(metadata, f, protocol=4) with self._open(os.path.join(rpath, 'response_headers'), 'wb') as f: f.write(headers_dict_to_raw(response.headers)) with self._open(os.path.join(rpath, 'response_body'), 'wb') as f: diff --git a/scrapy/extensions/spiderstate.py b/scrapy/extensions/spiderstate.py index 2e5ff569f..bea00596e 100644 --- a/scrapy/extensions/spiderstate.py +++ b/scrapy/extensions/spiderstate.py @@ -26,7 +26,7 @@ class SpiderState: def spider_closed(self, spider): if self.jobdir: with open(self.statefn, 'wb') as f: - pickle.dump(spider.state, f, protocol=2) + pickle.dump(spider.state, f, protocol=4) def spider_opened(self, spider): if self.jobdir and os.path.exists(self.statefn): diff --git a/scrapy/squeues.py b/scrapy/squeues.py index d0686dac3..8d05bd0d0 100644 --- a/scrapy/squeues.py +++ b/scrapy/squeues.py @@ -81,7 +81,7 @@ def _scrapy_non_serialization_queue(queue_class): def _pickle_serialize(obj): try: - return pickle.dumps(obj, protocol=2) + return pickle.dumps(obj, protocol=4) # Python <= 3.4 raises pickle.PicklingError here while # 3.5 <= Python < 3.6 raises AttributeError and # Python >= 3.6 raises TypeError From b1ddd7bd7b84d8d8417228aa7392d418463c9728 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 6 May 2020 13:44:02 -0300 Subject: [PATCH 17/20] Refactor test_squeues.py --- tests/test_squeues.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/test_squeues.py b/tests/test_squeues.py index 5ad8035f7..7e997a25e 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -47,12 +47,7 @@ def nonserializable_object_test(self): self.assertRaises(ValueError, q.push, sel) -class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest): - - chunksize = 100000 - - def queue(self): - return MarshalFifoDiskQueue(self.qpath, chunksize=self.chunksize) +class FifoDiskQueueTestMixin: def test_serialize(self): q = self.queue() @@ -66,6 +61,13 @@ class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest): test_nonserializable_object = nonserializable_object_test +class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): + chunksize = 100000 + + def queue(self): + return MarshalFifoDiskQueue(self.qpath, chunksize=self.chunksize) + + class ChunkSize1MarshalFifoDiskQueueTest(MarshalFifoDiskQueueTest): chunksize = 1 @@ -82,7 +84,7 @@ class ChunkSize4MarshalFifoDiskQueueTest(MarshalFifoDiskQueueTest): chunksize = 4 -class PickleFifoDiskQueueTest(MarshalFifoDiskQueueTest): +class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): chunksize = 100000 @@ -133,10 +135,7 @@ class ChunkSize4PickleFifoDiskQueueTest(PickleFifoDiskQueueTest): chunksize = 4 -class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest): - - def queue(self): - return MarshalLifoDiskQueue(self.qpath) +class LifoDiskQueueTestMixin: def test_serialize(self): q = self.queue() @@ -150,7 +149,13 @@ class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest): test_nonserializable_object = nonserializable_object_test -class PickleLifoDiskQueueTest(MarshalLifoDiskQueueTest): +class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin): + + def queue(self): + return MarshalLifoDiskQueue(self.qpath) + + +class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin): def queue(self): return PickleLifoDiskQueue(self.qpath) From 93436f9d3a67cd8abe3b321321c2d36d94f75b8b Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 6 May 2020 14:05:27 -0300 Subject: [PATCH 18/20] Chain pickling exception, test_squeues.py updates --- scrapy/squeues.py | 7 +++---- tests/test_squeues.py | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/scrapy/squeues.py b/scrapy/squeues.py index 8d05bd0d0..c7ad4d53d 100644 --- a/scrapy/squeues.py +++ b/scrapy/squeues.py @@ -82,11 +82,10 @@ def _scrapy_non_serialization_queue(queue_class): def _pickle_serialize(obj): try: return pickle.dumps(obj, protocol=4) - # Python <= 3.4 raises pickle.PicklingError here while - # 3.5 <= Python < 3.6 raises AttributeError and - # Python >= 3.6 raises TypeError + # Both pickle.PicklingError and AttributeError can be raised by pickle.dump(s) + # TypeError is raised from parsel.Selector except (pickle.PicklingError, AttributeError, TypeError) as e: - raise ValueError(str(e)) + raise ValueError(str(e)) from e PickleFifoDiskQueueNonRequest = _serializable_queue( diff --git a/tests/test_squeues.py b/tests/test_squeues.py index 7e997a25e..a20d242f4 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -28,20 +28,7 @@ class TestLoader(ItemLoader): def nonserializable_object_test(self): q = self.queue() - try: - pickle.dumps(lambda x: x) - except Exception: - # Trigger Twisted bug #7989 - import twisted.persisted.styles # NOQA - self.assertRaises(ValueError, q.push, lambda x: x) - else: - # Use a different unpickleable object - class A: - pass - - a = A() - a.__reduce__ = a.__reduce_ex__ = None - self.assertRaises(ValueError, q.push, a) + self.assertRaises(ValueError, q.push, lambda x: x) # Selectors should fail (lxml.html.HtmlElement objects can't be pickled) sel = Selector(text='

some text

') self.assertRaises(ValueError, q.push, sel) @@ -118,6 +105,19 @@ class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): self.assertEqual(r.url, r2.url) assert r2.meta['request'] is r2 + def test_non_pickable_object(self): + q = self.queue() + try: + q.push(lambda x: x) + except ValueError as exc: + self.assertIsInstance(exc.__context__, AttributeError) + + sel = Selector(text='

some text

') + try: + q.push(sel) + except ValueError as exc: + self.assertIsInstance(exc.__context__, TypeError) + class ChunkSize1PickleFifoDiskQueueTest(PickleFifoDiskQueueTest): chunksize = 1 From 0e382c816024baffca05b0da29def95f723d27fd Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 6 May 2020 14:09:10 -0300 Subject: [PATCH 19/20] Remove unused import --- tests/test_squeues.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_squeues.py b/tests/test_squeues.py index a20d242f4..51c0c028a 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -1,5 +1,3 @@ -import pickle - from queuelib.tests import test_queue as t from scrapy.squeues import ( MarshalFifoDiskQueueNonRequest as MarshalFifoDiskQueue, From d472402a0232781753515d9552b7a1997b43543a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 6 May 2020 14:39:17 -0300 Subject: [PATCH 20/20] Fix pickle test for pypy --- pytest.ini | 1 + tests/test_squeues.py | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pytest.ini b/pytest.ini index 4f3494e0e..d107c1fbe 100644 --- a/pytest.ini +++ b/pytest.ini @@ -166,6 +166,7 @@ flake8-ignore = scrapy/signalmanager.py E501 scrapy/spiderloader.py F841 E501 E126 scrapy/squeues.py E128 + scrapy/squeues.py E501 scrapy/statscollectors.py E501 # tests tests/__init__.py E402 E501 diff --git a/tests/test_squeues.py b/tests/test_squeues.py index 51c0c028a..d2cf9135f 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -1,3 +1,6 @@ +import pickle +import sys + from queuelib.tests import test_queue as t from scrapy.squeues import ( MarshalFifoDiskQueueNonRequest as MarshalFifoDiskQueue, @@ -108,8 +111,10 @@ class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): try: q.push(lambda x: x) except ValueError as exc: - self.assertIsInstance(exc.__context__, AttributeError) - + if hasattr(sys, "pypy_version_info"): + self.assertIsInstance(exc.__context__, pickle.PicklingError) + else: + self.assertIsInstance(exc.__context__, AttributeError) sel = Selector(text='

some text

') try: q.push(sel)