From 17e648182332a1d231383c7416e08e89280cb1d0 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 27 Nov 2019 18:42:42 -0300 Subject: [PATCH 01/26] [Docs] Fix Twisted links --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index eab366efd..a79f3a8cb 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -279,7 +279,7 @@ intersphinx_mapping = { 'python': ('https://docs.python.org/3', None), 'sphinx': ('https://www.sphinx-doc.org/en/master', None), 'tox': ('https://tox.readthedocs.io/en/latest', None), - 'twisted': ('https://twistedmatrix.com/documents/current', None), + 'twisted': ('https://twistedmatrix.com/documents/current/api', None), } From 048cd74ae594f449ba97d07c927d7640f32a6770 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 27 Nov 2019 19:16:18 -0300 Subject: [PATCH 02/26] Add separate mapping for Twisted API docs --- docs/conf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index a79f3a8cb..40e69c8ac 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -279,7 +279,8 @@ intersphinx_mapping = { 'python': ('https://docs.python.org/3', None), 'sphinx': ('https://www.sphinx-doc.org/en/master', None), 'tox': ('https://tox.readthedocs.io/en/latest', None), - 'twisted': ('https://twistedmatrix.com/documents/current/api', None), + 'twisted': ('https://twistedmatrix.com/documents/current', None), + 'twistedapi': ('https://twistedmatrix.com/documents/current/api', None), } From 6ce1ad31071326d22387f8444abefd6f3e18ed86 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 10 Jan 2020 04:20:37 -0300 Subject: [PATCH 03/26] [test] Spider middleware: catch exceptions right after the spider callback --- tests/test_spidermiddleware_output_chain.py | 50 +++++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index 739cf1c2d..b19a74609 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/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): """ From c088c04f449c3383b7867c25b387f13949b1d6c0 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 10 Jan 2020 04:20:55 -0300 Subject: [PATCH 04/26] Spider middleware: catch exceptions right after the spider callback --- scrapy/core/spidermw.py | 71 +++++++++++++++++++++++++---------------- scrapy/utils/python.py | 1 + 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 097a374bf..180a0b1fe 100644 --- a/scrapy/core/spidermw.py +++ b/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, method_index, recover_to): + try: + for r in iterable: + yield r + except Exception as ex: + exception_result = process_spider_exception(Failure(ex), method_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,38 +95,36 @@ 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) if isinstance(exception_result, Failure): raise return exception_result - if _isiterable(result): - result = evaluate_iterable(result, method_index) else: - raise _InvalidOutput('Middleware {} must return an iterable, got {}' - .format(fname(method), type(result))) + if _isiterable(result): + result = _evaluate_iterable(result, method_index+1, recovered) + else: + 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): + if isinstance(result, Failure): + return process_spider_exception(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_callback_output) return dfd def process_start_requests(self, start_requests, spider): diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 8d829c5a5..875650f3e 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -375,6 +375,7 @@ class MutableChain(object): """ Thin wrapper around itertools.chain, allowing to add iterables "in-place" """ + def __init__(self, *args): self.data = chain(*args) From d6e928f47209396d0f6c0b155eb07115c4855c93 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 10 Jan 2020 04:40:03 -0300 Subject: [PATCH 05/26] Remove object as base class for MutableChain Plus some minor styling adjustments --- scrapy/utils/python.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 875650f3e..e5582cc18 100644 --- a/scrapy/utils/python.py +++ b/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,7 +371,7 @@ else: gc.collect() -class MutableChain(object): +class MutableChain: """ Thin wrapper around itertools.chain, allowing to add iterables "in-place" """ From 9770ca35fb494503298270902569ed897a365d32 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 10 Jan 2020 18:45:39 -0300 Subject: [PATCH 06/26] Spider middleware: simplify deferred errback handling --- scrapy/core/spidermw.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 180a0b1fe..ed02b306b 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -117,14 +117,12 @@ class SpiderMiddlewareManager(MiddlewareManager): return MutableChain(result, recovered) def process_callback_output(result): - if isinstance(result, Failure): - return process_spider_exception(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_callback_output, errback=process_callback_output) + dfd.addCallbacks(callback=process_callback_output, errback=process_spider_exception) return dfd def process_start_requests(self, start_requests, spider): From 03241aa4a66f9b0e8be4dc104807011f546840e8 Mon Sep 17 00:00:00 2001 From: abhishekh2001 <53903855+abhishekh2001@users.noreply.github.com> Date: Wed, 15 Jan 2020 08:54:25 +0400 Subject: [PATCH 07/26] Fixed artwork/README formatting --- artwork/README.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/artwork/README.rst b/artwork/README.rst index 92f6ecb7e..8a1028cde 100644 --- a/artwork/README.rst +++ b/artwork/README.rst @@ -1,5 +1,4 @@ -:orphan: - +============== Scrapy artwork ============== From c9d36522302ab73552d804137a3625552275a771 Mon Sep 17 00:00:00 2001 From: "Matsievskiy S.V" Date: Mon, 27 Jan 2020 18:24:57 +0300 Subject: [PATCH 08/26] add zsh -h autocomplete option --- extras/scrapy_zsh_completion | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/extras/scrapy_zsh_completion b/extras/scrapy_zsh_completion index e995947cb..33f46eda8 100644 --- a/extras/scrapy_zsh_completion +++ b/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() { From ad4477d335bee8b10bc3bbca969defddd9b316f8 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 27 Jan 2020 14:16:43 -0300 Subject: [PATCH 09/26] Remove unnecessary else --- scrapy/core/spidermw.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index ed02b306b..8b36cbb04 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -107,12 +107,11 @@ class SpiderMiddlewareManager(MiddlewareManager): if isinstance(exception_result, Failure): raise return exception_result + if _isiterable(result): + result = _evaluate_iterable(result, method_index+1, recovered) else: - if _isiterable(result): - result = _evaluate_iterable(result, method_index+1, recovered) - else: - msg = "Middleware {} must return an iterable, got {}" - raise _InvalidOutput(msg.format(_fname(method), type(result))) + msg = "Middleware {} must return an iterable, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) return MutableChain(result, recovered) From 752e8f7018cbfac9cbdf486046d6bd8171cca0e8 Mon Sep 17 00:00:00 2001 From: Daniel Kimsey Date: Sun, 26 Jan 2020 13:21:31 -0600 Subject: [PATCH 10/26] FilesPipeline.file_path has optional arguments Documented signature doesn't match the actual interface in [files.py](https://github.com/scrapy/scrapy/blob/master/scrapy/pipelines/files.py#L520). Specifically, it looks like it may be [called](https://github.com/scrapy/scrapy/blob/master/scrapy/pipelines/files.py#L422) without a response value. I found this when I was implementing the pipeline with the signature `file_path(self, request, response, info)` and the following error was being return in my results : [(False, )] Scrapy==1.8.0 --- docs/topics/media-pipeline.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 1e0e0f18f..67a0bfdba 100644 --- a/docs/topics/media-pipeline.rst +++ b/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 From fbea370c58c1d82b52fd9c1f7d3a6cee94477c7a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 5 Feb 2020 01:35:13 -0300 Subject: [PATCH 11/26] Rename function parameter --- scrapy/core/spidermw.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 8b36cbb04..dd9b3c376 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -59,12 +59,12 @@ class SpiderMiddlewareManager(MiddlewareManager): return scrape_func(Failure(), request, spider) return scrape_func(response, request, spider) - def _evaluate_iterable(iterable, method_index, recover_to): + 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), method_index) + exception_result = process_spider_exception(Failure(ex), exception_processor_index) if isinstance(exception_result, Failure): raise recover_to.extend(exception_result) From 898bc00811aac9d3e38d1863b95a10c2e8effb02 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Feb 2020 11:31:27 +0000 Subject: [PATCH 12/26] new signal --- scrapy/signals.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scrapy/signals.py b/scrapy/signals.py index 6b9125302..cd7ed7fb1 100644 --- a/scrapy/signals.py +++ b/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() From ae04174884eeb777d7b3caceed52bf522944ceb1 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Feb 2020 11:32:31 +0000 Subject: [PATCH 13/26] emit new signal --- scrapy/core/downloader/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 157dc3418..5a2fdadf5 100644 --- a/scrapy/core/downloader/__init__.py +++ b/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) From 9916f6e556f9d4a41ea86d4a73687af1a40e43ba Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Feb 2020 11:32:54 +0000 Subject: [PATCH 14/26] tests for new signal --- tests/test_request_left.py | 59 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/test_request_left.py diff --git a/tests/test_request_left.py b/tests/test_request_left.py new file mode 100644 index 000000000..ddeca0499 --- /dev/null +++ b/tests/test_request_left.py @@ -0,0 +1,59 @@ +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_response_download, + signal=request_left_downloader) + self.catched_times = 0 + self.start_urls = [url] + + @classmethod + def from_crawler(cls, crawler, *args, **kwargs): + spider = cls(crawler, *args, **kwargs) + return spider + + def on_response_download(self, request, spider): + self.catched_times = self.catched_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.catched_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.catched_times, 1) + + @defer.inlineCallbacks + def test_disconnect(self): + crawler = get_crawler(SignalCatcherSpider) + yield crawler.crawl(self.mockserver.url("/drop")) + self.assertEqual(crawler.spider.catched_times, 1) + + @defer.inlineCallbacks + def test_noconnect(self): + crawler = get_crawler(SignalCatcherSpider) + yield crawler.crawl('http://thereisdefinetelynosuchdomain.com') + self.assertEqual(crawler.spider.catched_times, 1) From aab39f63412b4b7a0ae2713446859d6d8103e5f7 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Feb 2020 11:35:03 +0000 Subject: [PATCH 15/26] docummentation for new signal --- docs/topics/signals.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 3f29aa323..7fa5bc030 100644 --- a/docs/topics/signals.rst +++ b/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` left downloader even in case of + failure. + + The signal does not support returning deferreds from their handlers. + + :param request: the request that reached 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 ----------------- From 3769f75386104c1a3072894b302d3c3239ff8c37 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Wed, 5 Feb 2020 12:08:08 +0000 Subject: [PATCH 16/26] pep8 E302 --- tests/test_request_left.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_request_left.py b/tests/test_request_left.py index ddeca0499..5d271190d 100644 --- a/tests/test_request_left.py +++ b/tests/test_request_left.py @@ -5,6 +5,7 @@ from scrapy.spiders import Spider from scrapy.utils.test import get_crawler from tests.mockserver import MockServer + class SignalCatcherSpider(Spider): name = 'signal_catcher' From 6733f4d976150e0e5352d4ae9697880ae60ad638 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Thu, 6 Feb 2020 18:40:42 +0500 Subject: [PATCH 17/26] Update docs/topics/signals.rst Co-Authored-By: elacuesta --- docs/topics/signals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 7fa5bc030..47be6b603 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -301,7 +301,7 @@ request_left_downloader .. signal:: request_left_downloader .. function:: request_left_downloader(request, spider) - Sent when a :class:`~scrapy.http.Request` left downloader even in case of + Sent when a :class:`~scrapy.http.Request` leaves the downloader even in case of failure. The signal does not support returning deferreds from their handlers. From 4a91a5427df4846ed9fa11612cfeb9e31f34a1c8 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Thu, 6 Feb 2020 13:44:51 +0000 Subject: [PATCH 18/26] fix typo --- tests/test_request_left.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_request_left.py b/tests/test_request_left.py index 5d271190d..8256d1c92 100644 --- a/tests/test_request_left.py +++ b/tests/test_request_left.py @@ -13,7 +13,7 @@ class SignalCatcherSpider(Spider): super(SignalCatcherSpider, self).__init__(*args, **kwargs) crawler.signals.connect(self.on_response_download, signal=request_left_downloader) - self.catched_times = 0 + self.caught_times = 0 self.start_urls = [url] @classmethod @@ -22,7 +22,7 @@ class SignalCatcherSpider(Spider): return spider def on_response_download(self, request, spider): - self.catched_times = self.catched_times + 1 + self.caught_times = self.caught_times + 1 class TestCatching(TestCase): @@ -38,23 +38,23 @@ class TestCatching(TestCase): def test_success(self): crawler = get_crawler(SignalCatcherSpider) yield crawler.crawl(self.mockserver.url("/status?n=200")) - self.assertEqual(crawler.spider.catched_times, 1) + 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.catched_times, 1) + 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.catched_times, 1) + 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.catched_times, 1) + self.assertEqual(crawler.spider.caught_times, 1) From 4be19e443e9c101a248c21509ae8000ce500d51a Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Thu, 6 Feb 2020 13:46:23 +0000 Subject: [PATCH 19/26] name signla catcher in accord with signal name --- tests/test_request_left.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_request_left.py b/tests/test_request_left.py index 8256d1c92..5cfef8e7d 100644 --- a/tests/test_request_left.py +++ b/tests/test_request_left.py @@ -11,7 +11,7 @@ class SignalCatcherSpider(Spider): def __init__(self, crawler, url, *args, **kwargs): super(SignalCatcherSpider, self).__init__(*args, **kwargs) - crawler.signals.connect(self.on_response_download, + crawler.signals.connect(self.on_request_left, signal=request_left_downloader) self.caught_times = 0 self.start_urls = [url] @@ -21,7 +21,7 @@ class SignalCatcherSpider(Spider): spider = cls(crawler, *args, **kwargs) return spider - def on_response_download(self, request, spider): + def on_request_left(self, request, spider): self.caught_times = self.caught_times + 1 From 3263441fbcec8f46d363926d9106572cb0ecac5e Mon Sep 17 00:00:00 2001 From: Lane Shaw Date: Thu, 6 Feb 2020 16:14:40 -0500 Subject: [PATCH 20/26] Update RFPDupeFilter line separator for correct universal newlines mode usage (#4283) --- scrapy/dupefilters.py | 2 +- tests/test_dupefilters.py | 48 +++++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index ea6a4cfc3..a36c8304f 100644 --- a/scrapy/dupefilters.py +++ b/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) diff --git a/tests/test_dupefilters.py b/tests/test_dupefilters.py index 0546558bc..88ce9627f 100644 --- a/tests/test_dupefilters.py +++ b/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, From 4f31c3ce017db2b4f69949a81efd56fee60ee32d Mon Sep 17 00:00:00 2001 From: Joy Bhalla Date: Fri, 7 Feb 2020 02:51:33 +0530 Subject: [PATCH 21/26] Document a backward incompatibility that may affect custom schedulers (#4274) --- docs/news.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index 6d0d4b4ee..e4b985c77 100644 --- a/docs/news.rst +++ b/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. From 84b55b73646acca71461366ef98a1a501331f5d8 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Fri, 7 Feb 2020 11:07:35 +0500 Subject: [PATCH 22/26] Update docs/topics/signals.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- docs/topics/signals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 47be6b603..60d9ce2bc 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -306,7 +306,7 @@ request_left_downloader The signal does not support returning deferreds from their handlers. - :param request: the request that reached downloader + :param request: the request that reached the downloader :type request: :class:`~scrapy.http.Request` object :param spider: the spider that yielded the request From 2f83f3e2cb3497e89d42533b8f20f8398a696f46 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Fri, 7 Feb 2020 11:07:43 +0500 Subject: [PATCH 23/26] Update docs/topics/signals.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- docs/topics/signals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 60d9ce2bc..49475c1af 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -301,7 +301,7 @@ 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 + Sent when a :class:`~scrapy.http.Request` leaves the downloader, even in case of failure. The signal does not support returning deferreds from their handlers. From 8817b9e8e92f01147e7e44dd767165766093f408 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Fri, 7 Feb 2020 11:07:53 +0500 Subject: [PATCH 24/26] Update docs/topics/signals.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Adrián Chaves --- docs/topics/signals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 49475c1af..a7d60e9cb 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -296,7 +296,7 @@ request_reached_downloader :type spider: :class:`~scrapy.spiders.Spider` object request_left_downloader ---------------------------- +----------------------- .. signal:: request_left_downloader .. function:: request_left_downloader(request, spider) From 153b78e53f5c0f4630d09d560e111ca68f357905 Mon Sep 17 00:00:00 2001 From: Vostretsov Nikita Date: Fri, 7 Feb 2020 11:08:55 +0500 Subject: [PATCH 25/26] Update docs/topics/signals.rst Co-Authored-By: elacuesta --- docs/topics/signals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index a7d60e9cb..886d1b866 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -304,7 +304,7 @@ request_left_downloader Sent when a :class:`~scrapy.http.Request` leaves the downloader, even in case of failure. - The signal does not support returning deferreds from their handlers. + 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 From 31f6c7112fe8efce2105983b8350b1dabdce7a1c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 7 Feb 2020 17:14:52 +0500 Subject: [PATCH 26/26] Add a test for an async callbacks that returns requests. --- tests/spiders.py | 18 ++++++++++++++++++ tests/test_crawl.py | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/spiders.py b/tests/spiders.py index 3b1ee94b8..284c77829 100644 --- a/tests/spiders.py +++ b/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' diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 85005eba4..b4b5bac1c 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -13,7 +13,8 @@ from scrapy.utils.python import to_unicode from tests.mockserver import MockServer from tests.spiders import (FollowAllSpider, DelaySpider, SimpleSpider, BrokenStartRequestsSpider, SingleRequestSpider, DuplicateStartRequestsSpider, CrawlSpiderWithErrback, - AsyncDefSpider, AsyncDefAsyncioSpider, AsyncDefAsyncioReturnSpider) + AsyncDefSpider, AsyncDefAsyncioSpider, AsyncDefAsyncioReturnSpider, + AsyncDefAsyncioReqsReturnSpider) class CrawlTestCase(TestCase): @@ -330,7 +331,7 @@ with multiples lines @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): @@ -343,3 +344,12 @@ with multiples lines 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))