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 ============== diff --git a/docs/conf.py b/docs/conf.py index 458bc6087..c3418cfb3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -286,6 +286,7 @@ intersphinx_mapping = { 'sphinx': ('https://www.sphinx-doc.org/en/master', None), 'tox': ('https://tox.readthedocs.io/en/latest', None), 'twisted': ('https://twistedmatrix.com/documents/current', None), + 'twistedapi': ('https://twistedmatrix.com/documents/current/api', None), } 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. 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 diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 3f29aa323..886d1b866 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` leaves the downloader, even in case of + failure. + + This signal does not support returning deferreds from its handlers. + + :param request: the request that reached the downloader + :type request: :class:`~scrapy.http.Request` object + + :param spider: the spider that yielded the request + :type spider: :class:`~scrapy.spiders.Spider` object + response_received ----------------- 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() { 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) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 097a374bf..dd9b3c376 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, exception_processor_index, recover_to): + try: + for r in iterable: + yield r + except Exception as ex: + exception_result = process_spider_exception(Failure(ex), exception_processor_index) + if isinstance(exception_result, Failure): + raise + recover_to.extend(exception_result) + def process_spider_exception(_failure, start_index=0): exception = _failure.value # don't handle _InvalidOutput exception @@ -69,8 +86,8 @@ class SpiderMiddlewareManager(MiddlewareManager): elif result is None: continue else: - raise _InvalidOutput('Middleware {} must return None or an iterable, got {}' - .format(fname(method), type(result))) + msg = "Middleware {} must return None or an iterable, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) return _failure def process_spider_output(result, start_index=0): @@ -78,22 +95,12 @@ class SpiderMiddlewareManager(MiddlewareManager): # chain, they went through it already from the process_spider_exception method recovered = MutableChain() - def evaluate_iterable(iterable, index): - try: - for r in iterable: - yield r - except Exception as ex: - exception_result = process_spider_exception(Failure(ex), index+1) - if isinstance(exception_result, Failure): - raise - recovered.extend(exception_result) - method_list = islice(self.methods['process_spider_output'], start_index, None) for method_index, method in enumerate(method_list, start=start_index): if method is None: continue - # the following might fail directly if the output value is not a generator try: + # might fail directly if the output value is not a generator result = method(response=response, result=result, spider=spider) except Exception as ex: exception_result = process_spider_exception(Failure(ex), method_index+1) @@ -101,15 +108,20 @@ class SpiderMiddlewareManager(MiddlewareManager): raise return exception_result if _isiterable(result): - result = evaluate_iterable(result, method_index) + result = _evaluate_iterable(result, method_index+1, recovered) else: - raise _InvalidOutput('Middleware {} must return an iterable, got {}' - .format(fname(method), type(result))) + msg = "Middleware {} must return an iterable, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) - return chain(result, recovered) + return MutableChain(result, recovered) + + def process_callback_output(result): + recovered = MutableChain() + result = _evaluate_iterable(result, 0, recovered) + return MutableChain(process_spider_output(result), recovered) dfd = mustbe_deferred(process_spider_input, response) - dfd.addCallbacks(callback=process_spider_output, errback=process_spider_exception) + dfd.addCallbacks(callback=process_callback_output, errback=process_spider_exception) return dfd def process_start_requests(self, start_requests, spider): 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/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() diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 8d829c5a5..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,10 +371,11 @@ else: gc.collect() -class MutableChain(object): +class MutableChain: """ Thin wrapper around itertools.chain, allowing to add iterables "in-place" """ + def __init__(self, *args): self.data = chain(*args) diff --git a/tests/spiders.py b/tests/spiders.py index b115cc8e7..8a13c50c7 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 225fe7a0e..5980b37f4 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -12,6 +12,7 @@ from scrapy.http import Request from scrapy.utils.python import to_unicode from tests.mockserver import MockServer from tests.spiders import ( + AsyncDefAsyncioReqsReturnSpider, AsyncDefAsyncioReturnSpider, AsyncDefAsyncioSpider, AsyncDefSpider, @@ -364,7 +365,7 @@ class CrawlSpiderTestCase(TestCase): @mark.only_asyncio() @defer.inlineCallbacks - def test_async_def_asyncio_parse_list(self): + def test_async_def_asyncio_parse_items_list(self): items = [] def _on_item_scraped(item): @@ -377,3 +378,12 @@ class CrawlSpiderTestCase(TestCase): self.assertIn("Got response 200", str(log)) self.assertIn({'id': 1}, items) self.assertIn({'id': 2}, items) + + @mark.only_asyncio() + @defer.inlineCallbacks + def test_async_def_asyncio_parse_reqs_list(self): + crawler = self.runner.create_crawler(AsyncDefAsyncioReqsReturnSpider) + with LogCapture() as log: + yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver) + for req_id in range(3): + self.assertIn("Got response 200, req_id %d" % req_id, str(log)) 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, diff --git a/tests/test_request_left.py b/tests/test_request_left.py new file mode 100644 index 000000000..5cfef8e7d --- /dev/null +++ b/tests/test_request_left.py @@ -0,0 +1,60 @@ +from twisted.internet import defer +from twisted.trial.unittest import TestCase +from scrapy.signals import request_left_downloader +from scrapy.spiders import Spider +from scrapy.utils.test import get_crawler +from tests.mockserver import MockServer + + +class SignalCatcherSpider(Spider): + name = 'signal_catcher' + + def __init__(self, crawler, url, *args, **kwargs): + super(SignalCatcherSpider, self).__init__(*args, **kwargs) + crawler.signals.connect(self.on_request_left, + signal=request_left_downloader) + self.caught_times = 0 + self.start_urls = [url] + + @classmethod + def from_crawler(cls, crawler, *args, **kwargs): + spider = cls(crawler, *args, **kwargs) + return spider + + def on_request_left(self, request, spider): + self.caught_times = self.caught_times + 1 + + +class TestCatching(TestCase): + + def setUp(self): + self.mockserver = MockServer() + self.mockserver.__enter__() + + def tearDown(self): + self.mockserver.__exit__(None, None, None) + + @defer.inlineCallbacks + def test_success(self): + crawler = get_crawler(SignalCatcherSpider) + yield crawler.crawl(self.mockserver.url("/status?n=200")) + self.assertEqual(crawler.spider.caught_times, 1) + + @defer.inlineCallbacks + def test_timeout(self): + crawler = get_crawler(SignalCatcherSpider, + {'DOWNLOAD_TIMEOUT': 0.1}) + yield crawler.crawl(self.mockserver.url("/delay?n=0.2")) + self.assertEqual(crawler.spider.caught_times, 1) + + @defer.inlineCallbacks + def test_disconnect(self): + crawler = get_crawler(SignalCatcherSpider) + yield crawler.crawl(self.mockserver.url("/drop")) + self.assertEqual(crawler.spider.caught_times, 1) + + @defer.inlineCallbacks + def test_noconnect(self): + crawler = get_crawler(SignalCatcherSpider) + yield crawler.crawl('http://thereisdefinetelynosuchdomain.com') + self.assertEqual(crawler.spider.caught_times, 1) 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): """