From 484bd0d22a11a04ab775ac4f72c75f3ec9050d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 22 Mar 2019 16:59:29 +0100 Subject: [PATCH 01/54] Allow customizing export column names --- docs/topics/exporters.rst | 35 +++++++++++++++----- docs/topics/feed-exports.rst | 17 +++------- scrapy/exporters.py | 32 +++++++++++++++--- scrapy/extensions/feedexport.py | 3 +- scrapy/settings/__init__.py | 35 +++++++++++++++++++- tests/test_exporters.py | 15 ++++++++- tests/test_feedexport.py | 57 +++++++++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 29 deletions(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index f5048d2da..42a93e459 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -190,14 +190,33 @@ BaseItemExporter .. attribute:: fields_to_export - A list with the name of the fields that will be exported, or None if you - want to export all fields. Defaults to None. + Fields to export, their order [1]_ and their output names. - Some exporters (like :class:`CsvItemExporter`) respect the order of the - fields defined in this attribute. + Possible values are: - Some exporters may require fields_to_export list in order to export the - data properly when spiders return dicts (not :class:`~Item` instances). + - ``None`` (all fields [2]_, default) + + - A list of fields:: + + ['field1', 'field2'] + + - A dict [3]_ where keys are fields and values are output names:: + + {'field1': 'Field 1', 'field2': 'Field 2'} + + .. [1] Not all exporters respect the specified field order. + .. [2] If you yield items as dicts (not :class:`Item` instances), + exporters that need to know the fields to export beforehand, like + :class:`CsvItemExporter`, only export the fields found in the + first item. + .. [3] Dicts preserve insertion order since `Python 3.7`_ + (`CPython 3.6`_, `PyPy 2.5`_). If you are using an older version + of Python, use an OrderedDict_ to enforce a specific field order. + + .. _Python 3.7: https://docs.python.org/whatsnew/3.7.html + .. _CPython 3.6: https://docs.python.org/whatsnew/3.6.html#new-dict-implementation + .. _PyPy 2.5: https://morepypy.blogspot.com/2015/02/pypy-250-released.html + .. _OrderedDict: https://docs.python.org/library/collections.html#collections.OrderedDict .. attribute:: export_empty_fields @@ -286,8 +305,8 @@ CsvItemExporter Exports Items in CSV format to the given file-like object. If the :attr:`fields_to_export` attribute is set, it will be used to define the - CSV columns and their order. The :attr:`export_empty_fields` attribute has - no effect on this exporter. + CSV columns, their order and their column names. The + :attr:`export_empty_fields` attribute has no effect on this exporter. :param file: the file-like object to use for exporting the data. Its ``write`` method should accept ``bytes`` (a disk file opened in binary mode, a ``io.BytesIO`` object, etc) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index cf70b8aca..968cb8884 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -56,7 +56,7 @@ CSV * :setting:`FEED_FORMAT`: ``csv`` * Exporter used: :class:`~scrapy.exporters.CsvItemExporter` - * To specify columns to export and their order use + * To specify columns to export, their order and their column names, use :setting:`FEED_EXPORT_FIELDS`. Other feed exporters can also use this option, but it is important for CSV because unlike many other export formats CSV uses a fixed header. @@ -259,18 +259,9 @@ FEED_EXPORT_FIELDS Default: ``None`` -A list of fields to export, optional. -Example: ``FEED_EXPORT_FIELDS = ["foo", "bar", "baz"]``. - -Use FEED_EXPORT_FIELDS option to define fields to export and their order. - -When FEED_EXPORT_FIELDS is empty or None (default), Scrapy uses fields -defined in dicts or :class:`~.Item` subclasses a spider is yielding. - -If an exporter requires a fixed set of fields (this is the case for -:ref:`CSV ` export format) and FEED_EXPORT_FIELDS -is empty or None, then Scrapy tries to infer field names from the -exported data - currently it uses field names from the first item. +Use the ``FEED_EXPORT_FIELDS`` setting to define the fields to export, their +order and their output names. See :attr:`BaseItemExporter.fields_to_export +` for more information. .. setting:: FEED_EXPORT_INDENT diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 695c74fec..c05acaca5 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -2,6 +2,7 @@ Item Exporters are used to export/serialize items into different formats. """ +from collections import Mapping import csv import io import sys @@ -64,6 +65,14 @@ class BaseItemExporter(object): field_iter = six.iterkeys(item.fields) else: field_iter = six.iterkeys(item) + elif isinstance(self.fields_to_export, Mapping): + if include_empty: + field_iter = self.fields_to_export.items() + else: + field_iter = ( + (x, y) for x, y in self.fields_to_export.items() + if x in item + ) else: if include_empty: field_iter = self.fields_to_export @@ -71,13 +80,22 @@ class BaseItemExporter(object): field_iter = (x for x in self.fields_to_export if x in item) for field_name in field_iter: - if field_name in item: - field = {} if isinstance(item, dict) else item.fields[field_name] - value = self.serialize_field(field, field_name, item[field_name]) + if isinstance(field_name, six.string_types): + item_field, output_field = field_name, field_name + else: + item_field, output_field = field_name + if item_field in item: + if isinstance(item, dict): + field = {} + else: + field = item.fields[item_field] + value = self.serialize_field( + field, output_field, item[item_field] + ) else: value = default_value - yield field_name, value + yield output_field, value class JsonLinesItemExporter(BaseItemExporter): @@ -259,7 +277,11 @@ class CsvItemExporter(BaseItemExporter): else: # use fields declared in Item self.fields_to_export = list(item.fields.keys()) - row = list(self._build_row(self.fields_to_export)) + if isinstance(self.fields_to_export, Mapping): + fields = self.fields_to_export.values() + else: + fields = self.fields_to_export + row = list(self._build_row(fields)) self.csv_writer.writerow(row) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index b2f7267a2..1ed476d83 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -201,7 +201,8 @@ class FeedExporter(object): raise NotConfigured self.store_empty = settings.getbool('FEED_STORE_EMPTY') self._exporting = False - self.export_fields = settings.getlist('FEED_EXPORT_FIELDS') or None + self.export_fields = settings.getdictorlist('FEED_EXPORT_FIELDS') + self.export_fields = self.export_fields or None self.indent = None if settings.get('FEED_EXPORT_INDENT') is not None: self.indent = settings.getint('FEED_EXPORT_INDENT') diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 14c93bef2..69b324e84 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -1,7 +1,7 @@ import six import json import copy -from collections import MutableMapping +from collections import MutableMapping, OrderedDict from importlib import import_module from pprint import pformat @@ -198,6 +198,39 @@ class BaseSettings(MutableMapping): value = json.loads(value) return dict(value) + def getdictorlist(self, name, default=None): + """Get a setting value as either an ``OrderedDict`` or a list. + + If the setting is already a dict or a list, a copy of it will be + returned. + + If it is a string it will be evaluated as JSON, or as a comma-separated + list of strings as a fallback. + + For example, settings populated through environment variables will + return: + + - ``OrdetedDict([('key1', 'value1'), ('key2', 'value2')])`` if set to + ``'{"key1": "value1", "key2": "value2"}'`` + + - ``['one', 'two']`` if set to ``'["one", "two"]'`` or ``'one,two'`` + + :param name: the setting name + :type name: string + + :param default: the value to return if no setting is found + :type default: any + """ + value = self.get(name, default) + if value is None: + return {} + if isinstance(value, six.string_types): + try: + return json.loads(value, object_pairs_hook=OrderedDict) + except ValueError: + return value.split(',') + return copy.deepcopy(value) + def getwithbase(self, name): """Get a composition of a dictionary-like setting and its `_BASE` counterpart. diff --git a/tests/test_exporters.py b/tests/test_exporters.py index cd72c661a..1b3dc14a1 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -1,4 +1,7 @@ +# -*- coding:utf-8 -*- + from __future__ import absolute_import +from collections import OrderedDict import re import json import marshal @@ -83,6 +86,14 @@ class BaseItemExporterTest(unittest.TestCase): assert isinstance(name, six.text_type) self.assertEqual(name, u'John\xa3') + ie = self._get_exporter( + fields_to_export=OrderedDict([('name', u'名稱')]) + ) + self.assertEqual( + list(ie._get_serialized_fields(self.i)), + [(u'名稱', u'John\xa3')] + ) + def test_field_custom_serializer(self): def custom_serializer(value): return str(int(value) + 2) @@ -214,6 +225,7 @@ class MarshalItemExporterTest(BaseItemExporterTest): class CsvItemExporterTest(BaseItemExporterTest): def _get_exporter(self, **kwargs): + self.output = tempfile.TemporaryFile() return CsvItemExporter(self.output, **kwargs) def assertCsvEqual(self, first, second, msg=None): @@ -224,7 +236,8 @@ class CsvItemExporterTest(BaseItemExporterTest): return self.assertEqual(csvsplit(first), csvsplit(second), msg) def _check_output(self): - self.assertCsvEqual(to_unicode(self.output.getvalue()), u'age,name\r\n22,John\xa3\r\n') + self.output.seek(0) + self.assertCsvEqual(to_unicode(self.output.read()), u'age,name\r\n22,John\xa3\r\n') def assertExportResult(self, item, expected, **kwargs): fp = BytesIO() diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index eef0384cf..4f72c0ff4 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1,4 +1,5 @@ from __future__ import absolute_import +from collections import OrderedDict import os import csv import json @@ -578,6 +579,62 @@ class FeedExportTest(unittest.TestCase): yield self.assertExported(items, header, rows, settings=settings, ordered=True) + # fields may be defined as a comma-separated list + header = ["foo", "baz", "hello"] + settings = {'FEED_EXPORT_FIELDS': ",".join(header)} + rows = [ + {'foo': 'bar1', 'baz': '', 'hello': ''}, + {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, + {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, + {'foo': '', 'baz': '', 'hello': 'world4'}, + ] + yield self.assertExported(items, header, rows, + settings=settings, ordered=True) + + # fields may also be defined as a JSON array + header = ["foo", "baz", "hello"] + settings = {'FEED_EXPORT_FIELDS': json.dumps(header)} + rows = [ + {'foo': 'bar1', 'baz': '', 'hello': ''}, + {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, + {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, + {'foo': '', 'baz': '', 'hello': 'world4'}, + ] + yield self.assertExported(items, header, rows, + settings=settings, ordered=True) + + # custom output field names can be specified + header = OrderedDict(( + ("foo", "Foo"), + ("baz", "Baz"), + ("hello", "Hello"), + )) + settings = {'FEED_EXPORT_FIELDS': header} + rows = [ + {'Foo': 'bar1', 'Baz': '', 'Hello': ''}, + {'Foo': 'bar2', 'Baz': '', 'Hello': 'world2'}, + {'Foo': 'bar3', 'Baz': 'quux3', 'Hello': ''}, + {'Foo': '', 'Baz': '', 'Hello': 'world4'}, + ] + yield self.assertExported(items, list(header.values()), rows, + settings=settings, ordered=True) + + # custom output field names can be specified as a JSON object + header = OrderedDict(( + ("foo", "Foo"), + ("baz", "Baz"), + ("hello", "Hello"), + )) + settings = {'FEED_EXPORT_FIELDS': json.dumps(header)} + rows = [ + {'Foo': 'bar1', 'Baz': '', 'Hello': ''}, + {'Foo': 'bar2', 'Baz': '', 'Hello': 'world2'}, + {'Foo': 'bar3', 'Baz': 'quux3', 'Hello': ''}, + {'Foo': '', 'Baz': '', 'Hello': 'world4'}, + ] + yield self.assertExported(items, list(header.values()), rows, + settings=settings, ordered=True) + @defer.inlineCallbacks def test_export_dicts(self): # When dicts are used, only keys from the first row are used as From 20719bac5cdc4898e9f01fcf4f92aa781a9d0ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 17 Dec 2019 15:09:43 +0100 Subject: [PATCH 02/54] Fix import error --- scrapy/settings/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index c1fff4d95..98421be18 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -224,7 +224,7 @@ class BaseSettings(MutableMapping): value = self.get(name, default) if value is None: return {} - if isinstance(value, six.string_types): + if isinstance(value, str): try: return json.loads(value, object_pairs_hook=OrderedDict) except ValueError: From 5834088e670d93b2a63ad8afb258e687af0a9b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 18 Feb 2020 14:18:15 +0100 Subject: [PATCH 03/54] Apply feedback --- scrapy/settings/__init__.py | 5 +- tests/test_feedexport.py | 109 ++++++++++++++++++------------------ 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 98421be18..6f5b1ef97 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -207,8 +207,7 @@ class BaseSettings(MutableMapping): If it is a string it will be evaluated as JSON, or as a comma-separated list of strings as a fallback. - For example, settings populated through environment variables will - return: + For example, settings populated from the command line will return: - ``OrdetedDict([('key1', 'value1'), ('key2', 'value2')])`` if set to ``'{"key1": "value1", "key2": "value2"}'`` @@ -223,7 +222,7 @@ class BaseSettings(MutableMapping): """ value = self.get(name, default) if value is None: - return {} + return OrderedDict() if isinstance(value, str): try: return json.loads(value, object_pairs_hook=OrderedDict) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 291c47702..781cdc543 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -5,6 +5,7 @@ import warnings import tempfile import shutil import string +import sys from collections import OrderedDict from io import BytesIO from pathlib import Path @@ -12,6 +13,7 @@ from unittest import mock from urllib.parse import urljoin, urlparse, quote from urllib.request import pathname2url +import pytest from zope.interface.verify import verifyObject from twisted.trial import unittest from twisted.internet import defer @@ -590,78 +592,75 @@ class FeedExportTest(unittest.TestCase): yield self.assertExportedCsv(items, header, rows_csv, ordered=False) yield self.assertExportedJsonLines(items, rows_jl) - # edge case: FEED_EXPORT_FIELDS==[] means the same as default None + @defer.inlineCallbacks + def test_export_items_empty_field_list(self): + # FEED_EXPORT_FIELDS==[] means the same as default None + items = [{'foo': 'bar'}] + header = ["foo"] + rows = [{'foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': []} - yield self.assertExportedCsv(items, header, rows_csv, ordered=False) - yield self.assertExportedJsonLines(items, rows_jl, settings) + yield self.assertExportedCsv(items, header, rows, ordered=False) + yield self.assertExportedJsonLines(items, rows, settings) - # it is possible to override fields using FEED_EXPORT_FIELDS - header = ["foo", "baz", "hello"] + @defer.inlineCallbacks + def test_export_items_field_list(self): + items = [{'foo': 'bar'}] + header = ["foo", "baz"] + rows = [{'foo': 'bar', 'baz': ''}] settings = {'FEED_EXPORT_FIELDS': header} - rows = [ - {'foo': 'bar1', 'baz': '', 'hello': ''}, - {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, - {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, - {'foo': '', 'baz': '', 'hello': 'world4'}, - ] - yield self.assertExported(items, header, rows, - settings=settings, ordered=True) + yield self.assertExported(items, header, rows, settings=settings) - # fields may be defined as a comma-separated list - header = ["foo", "baz", "hello"] + @defer.inlineCallbacks + def test_export_items_comma_separated_field_list(self): + items = [{'foo': 'bar'}] + header = ["foo", "baz"] + rows = [{'foo': 'bar', 'baz': ''}] settings = {'FEED_EXPORT_FIELDS': ",".join(header)} - rows = [ - {'foo': 'bar1', 'baz': '', 'hello': ''}, - {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, - {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, - {'foo': '', 'baz': '', 'hello': 'world4'}, - ] - yield self.assertExported(items, header, rows, - settings=settings, ordered=True) + yield self.assertExported(items, header, rows, settings=settings) - # fields may also be defined as a JSON array - header = ["foo", "baz", "hello"] + @defer.inlineCallbacks + def test_export_items_json_field_list(self): + items = [{'foo': 'bar'}] + header = ["foo", "baz"] + rows = [{'foo': 'bar', 'baz': ''}] settings = {'FEED_EXPORT_FIELDS': json.dumps(header)} - rows = [ - {'foo': 'bar1', 'baz': '', 'hello': ''}, - {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, - {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, - {'foo': '', 'baz': '', 'hello': 'world4'}, - ] - yield self.assertExported(items, header, rows, - settings=settings, ordered=True) + yield self.assertExported(items, header, rows, settings=settings) - # custom output field names can be specified + @defer.inlineCallbacks + def test_export_items_field_names(self): + items = [{'foo': 'bar'}] header = OrderedDict(( ("foo", "Foo"), - ("baz", "Baz"), - ("hello", "Hello"), )) + rows = [{'Foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': header} - rows = [ - {'Foo': 'bar1', 'Baz': '', 'Hello': ''}, - {'Foo': 'bar2', 'Baz': '', 'Hello': 'world2'}, - {'Foo': 'bar3', 'Baz': 'quux3', 'Hello': ''}, - {'Foo': '', 'Baz': '', 'Hello': 'world4'}, - ] yield self.assertExported(items, list(header.values()), rows, - settings=settings, ordered=True) + settings=settings) - # custom output field names can be specified as a JSON object + @pytest.mark.skipif(sys.version_info < (3, 7), + reason='Only official in Python 3.7+') + @defer.inlineCallbacks + def test_export_items_dict_field_names(self): + items = [{'foo': 'bar'}] + header = { + 'baz': 'Baz', + 'foo': 'Foo', + } + rows = [{'Baz': '', 'Foo': 'bar'}] + settings = {'FEED_EXPORT_FIELDS': header} + yield self.assertExported(items, ['Baz', 'Foo'], rows, + settings=settings) + + @defer.inlineCallbacks + def test_export_items_json_field_names(self): + items = [{'foo': 'bar'}] header = OrderedDict(( ("foo", "Foo"), - ("baz", "Baz"), - ("hello", "Hello"), )) + rows = [{'Foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': json.dumps(header)} - rows = [ - {'Foo': 'bar1', 'Baz': '', 'Hello': ''}, - {'Foo': 'bar2', 'Baz': '', 'Hello': 'world2'}, - {'Foo': 'bar3', 'Baz': 'quux3', 'Hello': ''}, - {'Foo': '', 'Baz': '', 'Hello': 'world4'}, - ] yield self.assertExported(items, list(header.values()), rows, - settings=settings, ordered=True) + settings=settings) @defer.inlineCallbacks def test_export_dicts(self): @@ -697,7 +696,7 @@ class FeedExportTest(unittest.TestCase): {'egg': 'spam2', 'foo': 'bar2', 'baz': 'quux2'} ] yield self.assertExported(items, ['foo', 'baz', 'egg'], rows, - settings=settings, ordered=True) + settings=settings) # export a subset of columns settings = {'FEED_EXPORT_FIELDS': 'egg,baz'} @@ -706,7 +705,7 @@ class FeedExportTest(unittest.TestCase): {'egg': 'spam2', 'baz': 'quux2'} ] yield self.assertExported(items, ['egg', 'baz'], rows, - settings=settings, ordered=True) + settings=settings) @defer.inlineCallbacks def test_export_encoding(self): From 4605c66a80dabd64924e397580224a667cd73ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 7 May 2020 12:38:51 +0200 Subject: [PATCH 04/54] Fix AttributeError --- scrapy/utils/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index 376c1f992..6a6d38a5c 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -114,7 +114,7 @@ def get_sources(use_closest=True): def feed_complete_default_values_from_settings(feed, settings): out = feed.copy() out.setdefault("encoding", settings["FEED_EXPORT_ENCODING"]) - out.setdefault("fields", settings.settings.getdictorlist("FEED_EXPORT_FIELDS") or None) + out.setdefault("fields", settings.getdictorlist("FEED_EXPORT_FIELDS") or None) out.setdefault("store_empty", settings.getbool("FEED_STORE_EMPTY")) out.setdefault("uri_params", settings["FEED_URI_PARAMS"]) if settings["FEED_EXPORT_INDENT"] is None: From a9dfd85ea6e983f255afc1b5b0f295fccddcbacb Mon Sep 17 00:00:00 2001 From: Burak Can Kahraman Date: Thu, 30 Dec 2021 15:48:53 +0300 Subject: [PATCH 05/54] Document coroutines for signals. --- docs/topics/coroutines.rst | 2 ++ docs/topics/signals.rst | 20 +++++++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index 2aef755c7..549552bd1 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -1,3 +1,5 @@ +.. _topics-coroutines: + ========== Coroutines ========== diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 63ad3a9ad..328fb88d2 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -51,12 +51,12 @@ Deferred signal handlers ======================== Some signals support returning :class:`~twisted.internet.defer.Deferred` -objects from their handlers, allowing you to run asynchronous code that -does not block Scrapy. If a signal handler returns a -:class:`~twisted.internet.defer.Deferred`, Scrapy waits for that -:class:`~twisted.internet.defer.Deferred` to fire. +or :term:`awaitable objects ` from their handlers, allowing +you to run asynchronous code that does not block Scrapy. If a signal +handler returns one of these objects, Scrapy waits for that asynchronous +operation to finish. -Let's take an example:: +Let's take an example using :ref:`coroutines `:: class SignalSpider(scrapy.Spider): name = 'signals' @@ -68,17 +68,15 @@ Let's take an example:: crawler.signals.connect(spider.item_scraped, signal=signals.item_scraped) return spider - def item_scraped(self, item): + async def item_scraped(self, item): # Send the scraped item to the server - d = treq.post( + response = await treq.post( 'http://example.com/post', json.dumps(item).encode('ascii'), headers={b'Content-Type': [b'application/json']} ) - # The next item will be scraped only after - # deferred (d) is fired - return d + return response def parse(self, response): for quote in response.css('div.quote'): @@ -89,7 +87,7 @@ Let's take an example:: } See the :ref:`topics-signals-ref` below to know which signals support -:class:`~twisted.internet.defer.Deferred`. +:class:`~twisted.internet.defer.Deferred` and :term:`awaitable objects `. .. _topics-signals-ref: From 78ba4b033b016be7fbb22bfa9e6d5d389380e6d4 Mon Sep 17 00:00:00 2001 From: Yann Defretin Date: Wed, 16 Mar 2022 15:14:24 +0100 Subject: [PATCH 06/54] fixed detection of extension like ".tar.gz" in URL --- scrapy/utils/url.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index a6a2a9e8b..bae5a9433 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -5,7 +5,6 @@ library. Some of the functions that used to be imported from this module have been moved to the w3lib.url module. Always import those from there instead. """ -import posixpath import re from urllib.parse import ParseResult, urldefrag, urlparse, urlunparse @@ -31,8 +30,8 @@ def url_is_from_spider(url, spider): def url_has_any_extension(url, extensions): - return posixpath.splitext(parse_url(url).path)[1].lower() in extensions - + """Return True if the url ends with one of the extensions provided""" + return any(parse_url(url).path.lower().endswith(ext) for ext in extensions) def parse_url(url, encoding=None): """Return urlparsed url from the given argument (which could be an already From 5b4b8b6fb12874d4a0a11c261341639c9af95b10 Mon Sep 17 00:00:00 2001 From: Yann Defretin Date: Wed, 16 Mar 2022 22:32:05 +0100 Subject: [PATCH 07/54] added test for new url_has_any_extension function --- tests/test_utils_url.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 144c7bd76..58e2be622 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -1,6 +1,8 @@ import unittest +from scrapy.linkextractors import IGNORED_EXTENSIONS from scrapy.spiders import Spider +from scrapy.utils.misc import arg_to_iter from scrapy.utils.url import ( add_http_if_no_scheme, guess_scheme, @@ -8,9 +10,9 @@ from scrapy.utils.url import ( strip_url, url_is_from_any_domain, url_is_from_spider, + url_has_any_extension, ) - __doctests__ = ['scrapy.utils.url'] @@ -81,6 +83,15 @@ class UrlUtilsTest(unittest.TestCase): self.assertTrue(url_is_from_spider('http://www.example.net/some/page.html', MySpider)) self.assertFalse(url_is_from_spider('http://www.example.us/some/page.html', MySpider)) + def test_url_has_any_extension(self): + deny_extensions = {'.' + e for e in arg_to_iter(IGNORED_EXTENSIONS)} + self.assertTrue(url_has_any_extension("http://www.example.com/archive.tar.gz", deny_extensions)) + self.assertTrue(url_has_any_extension("http://www.example.com/page.doc", deny_extensions)) + self.assertTrue(url_has_any_extension("http://www.example.com/page.pdf", deny_extensions)) + self.assertFalse(url_has_any_extension("http://www.example.com/page.htm", deny_extensions)) + self.assertFalse(url_has_any_extension("http://www.example.com/", deny_extensions)) + self.assertFalse(url_has_any_extension("http://www.example.com/page.doc.html", deny_extensions)) + class AddHttpIfNoScheme(unittest.TestCase): From 9a28eb0bad1acf986d997905a410058f77911b7c Mon Sep 17 00:00:00 2001 From: Eugene Date: Thu, 17 Mar 2022 05:39:54 +0100 Subject: [PATCH 08/54] Suggest installing the brotli package instead of brotlipy (#4267) --- docs/topics/downloader-middleware.rst | 5 +++-- tests/requirements.txt | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index a15637ed6..912600428 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -704,14 +704,15 @@ HttpCompressionMiddleware sent/received from web sites. This middleware also supports decoding `brotli-compressed`_ as well as - `zstd-compressed`_ responses, provided that `brotlipy`_ or `zstandard`_ is + `zstd-compressed`_ responses, provided that `brotli`_ or `zstandard`_ is installed, respectively. .. _brotli-compressed: https://www.ietf.org/rfc/rfc7932.txt -.. _brotlipy: https://pypi.org/project/brotlipy/ +.. _brotli: https://pypi.org/project/Brotli/ .. _zstd-compressed: https://www.ietf.org/rfc/rfc8478.txt .. _zstandard: https://pypi.org/project/zstandard/ + HttpCompressionMiddleware Settings ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/requirements.txt b/tests/requirements.txt index 398d1d16d..d2a8aae1b 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -12,7 +12,7 @@ uvloop; platform_system != "Windows" and python_version > '3.6' # optional for shell wrapper tests bpython -brotlipy # optional for HTTP compress downloader middleware tests +brotli # optional for HTTP compress downloader middleware tests zstandard; implementation_name != 'pypy' # optional for HTTP compress downloader middleware tests ipython pywin32; sys_platform == "win32" From 0905d42e33871e976760d880316210d4953cd5df Mon Sep 17 00:00:00 2001 From: Yann Defretin Date: Thu, 17 Mar 2022 11:19:09 +0100 Subject: [PATCH 09/54] refactored url_has_any_extension function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrián Chaves --- scrapy/utils/url.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index bae5a9433..4d5e9ae82 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -31,7 +31,8 @@ def url_is_from_spider(url, spider): def url_has_any_extension(url, extensions): """Return True if the url ends with one of the extensions provided""" - return any(parse_url(url).path.lower().endswith(ext) for ext in extensions) + lowercase_path = parse_url(url).path.lower() + return any(lowercase_path.endswith(ext) for ext in extensions) def parse_url(url, encoding=None): """Return urlparsed url from the given argument (which could be an already From 6a3f2ee6876145bd4bd9ee1ff89d94474a1e85a0 Mon Sep 17 00:00:00 2001 From: FJMonteroInformatica Date: Thu, 17 Mar 2022 20:09:56 +0100 Subject: [PATCH 10/54] HTML Conventions --- docs/_static/selectors-sample1.html | 31 +++++++------- .../link_extractor/linkextractor.html | 40 ++++++++++--------- .../link_extractor/linkextractor_latin1.html | 8 ++-- .../link_extractor/linkextractor_no_href.html | 3 +- .../link_extractor/linkextractor_noenc.html | 23 ++++++----- tests/sample_data/test_site/index.html | 31 +++++++------- tests/sample_data/test_site/item1.html | 27 ++++++------- tests/sample_data/test_site/item2.html | 29 ++++++-------- 8 files changed, 96 insertions(+), 96 deletions(-) diff --git a/docs/_static/selectors-sample1.html b/docs/_static/selectors-sample1.html index 8a79a3381..915718832 100644 --- a/docs/_static/selectors-sample1.html +++ b/docs/_static/selectors-sample1.html @@ -1,16 +1,17 @@ - - - - Example website - - - - - + + + + + Example website + + + + + \ No newline at end of file diff --git a/tests/sample_data/link_extractor/linkextractor.html b/tests/sample_data/link_extractor/linkextractor.html index 2307ea865..e3a2a4145 100644 --- a/tests/sample_data/link_extractor/linkextractor.html +++ b/tests/sample_data/link_extractor/linkextractor.html @@ -1,20 +1,22 @@ + + - - -Sample page with links for testing LinkExtractor - - - - - + + + Sample page with links for testing LinkExtractor + + + + + \ No newline at end of file diff --git a/tests/sample_data/link_extractor/linkextractor_latin1.html b/tests/sample_data/link_extractor/linkextractor_latin1.html index e7eee18de..1e05bf0f0 100644 --- a/tests/sample_data/link_extractor/linkextractor_latin1.html +++ b/tests/sample_data/link_extractor/linkextractor_latin1.html @@ -1,3 +1,5 @@ + + @@ -7,11 +9,11 @@ diff --git a/tests/sample_data/link_extractor/linkextractor_no_href.html b/tests/sample_data/link_extractor/linkextractor_no_href.html index 0b01cede8..2d67ec6ff 100644 --- a/tests/sample_data/link_extractor/linkextractor_no_href.html +++ b/tests/sample_data/link_extractor/linkextractor_no_href.html @@ -1,3 +1,5 @@ + + @@ -21,5 +23,4 @@ - \ No newline at end of file diff --git a/tests/sample_data/link_extractor/linkextractor_noenc.html b/tests/sample_data/link_extractor/linkextractor_noenc.html index f9166adbe..6fa137cd9 100644 --- a/tests/sample_data/link_extractor/linkextractor_noenc.html +++ b/tests/sample_data/link_extractor/linkextractor_noenc.html @@ -1,14 +1,17 @@ + + - - -Sample page without encoding for testing LinkExtractor - + + + Sample page without encoding for testing LinkExtractor + + -
-
- -
-sample € text -
+
+
+ sample2 +
+ sample € text +
diff --git a/tests/sample_data/test_site/index.html b/tests/sample_data/test_site/index.html index d268c846a..afe17d8e2 100644 --- a/tests/sample_data/test_site/index.html +++ b/tests/sample_data/test_site/index.html @@ -1,18 +1,15 @@ + + - - -Scrapy test site - - - - -

Scrapy test site

- - - - - + + Scrapy test site + + +

Scrapy test site

+
+ + \ No newline at end of file diff --git a/tests/sample_data/test_site/item1.html b/tests/sample_data/test_site/item1.html index ceeb6dc87..ee39f16f3 100644 --- a/tests/sample_data/test_site/item1.html +++ b/tests/sample_data/test_site/item1.html @@ -1,17 +1,14 @@ + + - - -Item 1 - Scrapy test site - - - - -

Item 1 name

- -
    -
  • Price: $100
  • -
  • Stock: 12
  • -
- - + + Item 1 - Scrapy test site + + +

Item 1 name

+
    +
  • Price: $100
  • +
  • Stock: 12
  • +
+ diff --git a/tests/sample_data/test_site/item2.html b/tests/sample_data/test_site/item2.html index a64c92810..f40f70750 100644 --- a/tests/sample_data/test_site/item2.html +++ b/tests/sample_data/test_site/item2.html @@ -1,17 +1,14 @@ + + - - -Item 2 - Scrapy test site - - - - -

Item 2 name

- -
    -
  • Price: $200
  • -
  • Stock: 5
  • -
- - - + + Item 2 - Scrapy test site + + +

Item 2 name

+
    +
  • Price: $200
  • +
  • Stock: 5
  • +
+ + \ No newline at end of file From fcf3d8e0a0df447fd7cd81e98c846a16b8b42a73 Mon Sep 17 00:00:00 2001 From: D00399830 Date: Mon, 21 Mar 2022 14:09:31 -0600 Subject: [PATCH 11/54] Updated the documentation for developer tools to have JavaScript instead of Javascript, as JavaScript is the more correct way to write it --- docs/topics/developer-tools.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/developer-tools.rst b/docs/topics/developer-tools.rst index 96475899f..9bf97c628 100644 --- a/docs/topics/developer-tools.rst +++ b/docs/topics/developer-tools.rst @@ -19,14 +19,14 @@ Caveats with inspecting the live browser DOM Since Developer Tools operate on a live browser DOM, what you'll actually see when inspecting the page source is not the original HTML, but a modified one -after applying some browser clean up and executing Javascript code. Firefox, +after applying some browser clean up and executing JavaScript code. Firefox, in particular, is known for adding ```` elements to tables. Scrapy, on the other hand, does not modify the original page HTML, so you won't be able to extract any data if you use ```` in your XPath expressions. Therefore, you should keep in mind the following things: -* Disable Javascript while inspecting the DOM looking for XPaths to be +* Disable JavaScript while inspecting the DOM looking for XPaths to be used in Scrapy (in the Developer Tools settings click `Disable JavaScript`) * Never use full XPath paths, use relative and clever ones based on attributes From 2227be7af6d0504d52bd4c2e299efb1becbd992f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Ruiz?= Date: Tue, 22 Mar 2022 15:21:16 +0100 Subject: [PATCH 12/54] Fix a typo in the HTTP cache documentation (#5455) --- docs/topics/downloader-middleware.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 912600428..29e350651 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -366,7 +366,7 @@ HttpCacheMiddleware This middleware provides low-level cache to all HTTP requests and responses. It has to be combined with a cache storage backend as well as a cache policy. - Scrapy ships with three HTTP cache storage backends: + Scrapy ships with the following HTTP cache storage backends: * :ref:`httpcache-storage-fs` * :ref:`httpcache-storage-dbm` From 319e67f779163df3ad44327bfbc9733edcb34908 Mon Sep 17 00:00:00 2001 From: Yash <76577754+yash-fn@users.noreply.github.com> Date: Sat, 26 Mar 2022 18:17:03 -0500 Subject: [PATCH 13/54] documentation update for multiple spiders i noticed passing settings to configure logging function made weird output go away. checked documentation and it says first parameter is settings file. Is this correct? --- docs/topics/practices.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index d0207fd18..7313c9246 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -180,8 +180,8 @@ Same example but running the spiders sequentially by chaining the deferreds: # Your second spider definition ... - configure_logging() settings = get_project_settings() + configure_logging(settings) runner = CrawlerRunner(settings) @defer.inlineCallbacks From b2afcbfe2bf090827540d072866bef0d1ab3a3e8 Mon Sep 17 00:00:00 2001 From: AngelikiBoura <73474686+AngelikiBoura@users.noreply.github.com> Date: Thu, 5 May 2022 16:49:52 +0300 Subject: [PATCH 14/54] Fix typos in three files for Flake8 check (#5487) * Fix typos in extensions files Made some fixes in files memusage.py and statsmailer.py in order to pass the flake8 check. * Fix typos in twisted_reactor_custom_settings_same.py A small change was needed in order for flake8 check to pass. --- scrapy/extensions/memusage.py | 10 +++++----- scrapy/extensions/statsmailer.py | 1 + .../twisted_reactor_custom_settings_same.py | 1 + 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index 9de119a10..f5081a7d7 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -33,8 +33,8 @@ class MemoryUsage: self.crawler = crawler self.warned = False self.notify_mails = crawler.settings.getlist('MEMUSAGE_NOTIFY_MAIL') - self.limit = crawler.settings.getint('MEMUSAGE_LIMIT_MB')*1024*1024 - self.warning = crawler.settings.getint('MEMUSAGE_WARNING_MB')*1024*1024 + self.limit = crawler.settings.getint('MEMUSAGE_LIMIT_MB') * 1024 * 1024 + self.warning = crawler.settings.getint('MEMUSAGE_WARNING_MB') * 1024 * 1024 self.check_interval = crawler.settings.getfloat('MEMUSAGE_CHECK_INTERVAL_SECONDS') self.mail = MailSender.from_settings(crawler.settings) crawler.signals.connect(self.engine_started, signal=signals.engine_started) @@ -77,7 +77,7 @@ class MemoryUsage: def _check_limit(self): if self.get_virtual_size() > self.limit: self.crawler.stats.set_value('memusage/limit_reached', 1) - mem = self.limit/1024/1024 + mem = self.limit / 1024 / 1024 logger.error("Memory usage exceeded %(memusage)dM. Shutting down Scrapy...", {'memusage': mem}, extra={'crawler': self.crawler}) if self.notify_mails: @@ -94,11 +94,11 @@ class MemoryUsage: self.crawler.stop() def _check_warning(self): - if self.warned: # warn only once + if self.warned: # warn only once return if self.get_virtual_size() > self.warning: self.crawler.stats.set_value('memusage/warning_reached', 1) - mem = self.warning/1024/1024 + mem = self.warning / 1024 / 1024 logger.warning("Memory usage reached %(memusage)dM", {'memusage': mem}, extra={'crawler': self.crawler}) if self.notify_mails: diff --git a/scrapy/extensions/statsmailer.py b/scrapy/extensions/statsmailer.py index bcdbaff24..739e6b958 100644 --- a/scrapy/extensions/statsmailer.py +++ b/scrapy/extensions/statsmailer.py @@ -8,6 +8,7 @@ from scrapy import signals from scrapy.mail import MailSender from scrapy.exceptions import NotConfigured + class StatsMailer: def __init__(self, stats, recipients, mail): diff --git a/tests/CrawlerProcess/twisted_reactor_custom_settings_same.py b/tests/CrawlerProcess/twisted_reactor_custom_settings_same.py index 1f5a44010..72bb986bc 100644 --- a/tests/CrawlerProcess/twisted_reactor_custom_settings_same.py +++ b/tests/CrawlerProcess/twisted_reactor_custom_settings_same.py @@ -8,6 +8,7 @@ class AsyncioReactorSpider1(scrapy.Spider): "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", } + class AsyncioReactorSpider2(scrapy.Spider): name = 'asyncio_reactor2' custom_settings = { From 83c1939281197242511931c6e9f356f2498eb623 Mon Sep 17 00:00:00 2001 From: Andreas Tziortziortziopoulos Date: Fri, 6 May 2022 03:59:30 +0300 Subject: [PATCH 15/54] Issue #3264, fix error handling when spider is not matched Changes Implementation: - Check whether Spider exists or is None, and if it's None skip execution of start_requests() with non existing Spider Testing: - Add a test case with invalid url inside test_command_parse Test proves that non-matched Spider does not throw an AttributeError --- scrapy/commands/parse.py | 3 ++- tests/test_command_parse.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index a3f6b96f4..99fc8f955 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -146,7 +146,8 @@ class Command(BaseRunSpiderCommand): def _start_requests(spider): yield self.prepare_request(spider, Request(url), opts) - self.spidercls.start_requests = _start_requests + if self.spidercls: + self.spidercls.start_requests = _start_requests def start_parsing(self, url, opts): self.crawler_process.crawl(self.spidercls, **opts.spargs) diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index f21ee971d..0622074a3 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -1,6 +1,7 @@ import os import argparse from os.path import join, abspath, isfile, exists + from twisted.internet import defer from scrapy.commands import parse from scrapy.settings import Settings @@ -222,6 +223,10 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}} self.assertRegex(_textmode(out), r"""# Scraped Items -+\n\[\]""") self.assertIn("""Cannot find a rule that matches""", _textmode(stderr)) + status, out, stderr = yield self.execute([self.url('/invalid_url')]) + self.assertEqual(status, 0) + self.assertIn("""""", _textmode(stderr)) + @defer.inlineCallbacks def test_output_flag(self): """Checks if a file was created successfully having From 965fde24a4798ee51f05ce8669b7a28958ad3238 Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin Date: Fri, 8 Apr 2022 14:26:23 +0500 Subject: [PATCH 16/54] Pin mitmproxy to < 8 for now (#5459) --- tox.ini | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index db151f215..d13bb7b38 100644 --- a/tox.ini +++ b/tox.ini @@ -12,10 +12,11 @@ deps = -rtests/requirements.txt # mitmproxy does not support PyPy # mitmproxy does not support Windows when running Python < 3.7 - # Python 3.9+ requires https://github.com/mitmproxy/mitmproxy/commit/8e5e43de24c9bc93092b63efc67fbec029a9e7fe + # Python 3.9+ requires mitmproxy >= 5.3.0 # mitmproxy >= 5.3.0 requires h2 >= 4.0, Twisted 21.2 requires h2 < 4.0 #mitmproxy >= 5.3.0; python_version >= '3.9' and implementation_name != 'pypy' - mitmproxy >= 4.0.4; python_version >= '3.7' and python_version < '3.9' and implementation_name != 'pypy' + # The tests hang with mitmproxy 8.0.0: https://github.com/scrapy/scrapy/issues/5454 + mitmproxy >= 4.0.4, < 8; python_version >= '3.7' and python_version < '3.9' and implementation_name != 'pypy' mitmproxy >= 4.0.4, < 5; python_version >= '3.6' and python_version < '3.7' and platform_system != 'Windows' and implementation_name != 'pypy' # newer markupsafe is incompatible with deps of old mitmproxy (which we get on Python 3.7 and lower) markupsafe < 2.1.0; python_version >= '3.6' and python_version < '3.8' and implementation_name != 'pypy' From 078622cfb0ee364acba5d91a20244f9c1ee87d30 Mon Sep 17 00:00:00 2001 From: Maxime Nannan <28675918+mnannan@users.noreply.github.com> Date: Fri, 20 May 2022 08:30:06 +0200 Subject: [PATCH 17/54] Fix file expiration issue with GCS (#5318) --- scrapy/pipelines/files.py | 10 +++++++--- tests/test_pipeline_files.py | 23 +++++++++++++++++++++++ tox.ini | 7 ++++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 5c52c6c28..906e7eb24 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -222,8 +222,8 @@ class GCSFilesStore: return {'checksum': checksum, 'last_modified': last_modified} else: return {} - - return threads.deferToThread(self.bucket.get_blob, path).addCallback(_onsuccess) + blob_path = self._get_blob_path(path) + return threads.deferToThread(self.bucket.get_blob, blob_path).addCallback(_onsuccess) def _get_content_type(self, headers): if headers and 'Content-Type' in headers: @@ -231,8 +231,12 @@ class GCSFilesStore: else: return 'application/octet-stream' + def _get_blob_path(self, path): + return self.prefix + path + def persist_file(self, path, buf, info, meta=None, headers=None): - blob = self.bucket.blob(self.prefix + path) + blob_path = self._get_blob_path(path) + blob = self.bucket.blob(blob_path) blob.cache_control = self.CACHE_CONTROL blob.metadata = {k: str(v) for k, v in (meta or {}).items()} return threads.deferToThread( diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4e1b90787..0ff2045ed 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -525,6 +525,29 @@ class TestGCSFilesStore(unittest.TestCase): self.assertEqual(blob.content_type, 'application/octet-stream') self.assertIn(expected_policy, acl) + @defer.inlineCallbacks + def test_blob_path_consistency(self): + """Test to make sure that paths used to store files is the same as the one used to get + already uploaded files. + """ + assert_gcs_environ() + try: + import google.cloud.storage # noqa + except ModuleNotFoundError: + raise unittest.SkipTest("google-cloud-storage is not installed") + else: + with mock.patch('google.cloud.storage') as _: + with mock.patch('scrapy.pipelines.files.time') as _: + uri = 'gs://my_bucket/my_prefix/' + store = GCSFilesStore(uri) + store.bucket = mock.Mock() + path = 'full/my_data.txt' + yield store.persist_file(path, mock.Mock(), info=None, meta=None, headers=None) + yield store.stat_file(path, info=None) + expected_blob_path = store.prefix + path + store.bucket.blob.assert_called_with(expected_blob_path) + store.bucket.get_blob.assert_called_with(expected_blob_path) + class TestFTPFileStore(unittest.TestCase): @defer.inlineCallbacks diff --git a/tox.ini b/tox.ini index d13bb7b38..6951b6d16 100644 --- a/tox.ini +++ b/tox.ini @@ -126,13 +126,14 @@ setenv = deps = {[testenv]deps} boto + google-cloud-storage + # Twisted[http2] currently forces old mitmproxy because of h2 version + # restrictions in their deps, so we need to pin old markupsafe here too. + markupsafe < 2.1.0 reppy robotexclusionrulesparser Pillow>=4.0.0 Twisted[http2]>=17.9.0 - # Twisted[http2] currently forces old mitmproxy because of h2 version restrictions in their deps, - # so we need to pin old markupsafe here too - markupsafe < 2.1.0 [testenv:asyncio] commands = From b5c15d87ff5770220bca31792c89e58804b923bb Mon Sep 17 00:00:00 2001 From: Andreas Tziortziortziopoulos Date: Sun, 22 May 2022 12:19:20 +0300 Subject: [PATCH 18/54] [issue3264] Separate test for not matched spider to a url --- tests/test_command_parse.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index 0622074a3..0d992be56 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -223,9 +223,10 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}} self.assertRegex(_textmode(out), r"""# Scraped Items -+\n\[\]""") self.assertIn("""Cannot find a rule that matches""", _textmode(stderr)) + @defer.inlineCallbacks + def test_crawlspider_not_exists_with_not_matched_url(self): status, out, stderr = yield self.execute([self.url('/invalid_url')]) self.assertEqual(status, 0) - self.assertIn("""""", _textmode(stderr)) @defer.inlineCallbacks def test_output_flag(self): From 86331900125dc311223cbd1ebb0e10d09e7c592d Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Tue, 24 May 2022 14:47:00 +0430 Subject: [PATCH 19/54] pass on item to thumb_path function as additional argument resolves #5504 --- scrapy/pipelines/images.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 9c99dc69e..45ac03820 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -141,7 +141,7 @@ class ImagesPipeline(FilesPipeline): yield path, image, buf for thumb_id, size in self.thumbs.items(): - thumb_path = self.thumb_path(request, thumb_id, response=response, info=info) + thumb_path = self.thumb_path(request, thumb_id, response=response, info=info, item=item) thumb_image, thumb_buf = self.convert_image(image, size) yield thumb_path, thumb_image, thumb_buf @@ -179,6 +179,6 @@ class ImagesPipeline(FilesPipeline): image_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() return f'full/{image_guid}.jpg' - def thumb_path(self, request, thumb_id, response=None, info=None): + def thumb_path(self, request, thumb_id, response=None, info=None, item=None): thumb_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() return f'thumbs/{thumb_id}/{thumb_guid}.jpg' From f39def4492f838b2414324e22a12f3b355f5c062 Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Wed, 25 May 2022 23:57:38 +0430 Subject: [PATCH 20/54] add docs --- docs/topics/media-pipeline.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 7dff78390..2513faae2 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -656,6 +656,26 @@ See here the methods that you can override in your custom Images Pipeline: .. versionadded:: 2.4 The *item* parameter. + .. method:: ImagesPipeline.thumb_path(self, request, thumb_id, response=None, info=None, *, item=None) + + This method is called for every item of :setting:`IMAGES_THUMBS` per downloaded item. It returns the + thumbnail download path of the image originating from the specified + :class:`response `. + + In addition to ``response``, this method receives the original + :class:`request `, + ``thumb_id``, + :class:`info ` and + :class:`item `. + + You can override this method to customize the thumbnail download path of each image. + You can use the ``item`` to determine the file path based on some item + property. + + By default the :meth:`thumb_path` method returns + ``thumbs//.``. + + .. method:: ImagesPipeline.get_media_requests(item, info) Works the same way as :meth:`FilesPipeline.get_media_requests` method, From 5c586d78f0e1c5b66358ed644bb6e528ad4b062b Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Wed, 25 May 2022 23:58:09 +0430 Subject: [PATCH 21/54] add tests --- tests/test_pipeline_images.py | 16 ++++++++++++++++ tests/test_pipeline_media.py | 28 +++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index c69cd0e4a..dd94d296b 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -93,6 +93,22 @@ class ImagesPipelineTestCase(unittest.TestCase): info=object()), 'thumbs/50/850233df65a5b83361798f532f1fc549cd13cbe9.jpg') + def test_thumbnail_name_from_item(self): + """ + Custom thumbnail name based on item data, overriding default implementation + """ + + class CustomImagesPipeline(ImagesPipeline): + def thumb_path(self, request, thumb_id, response=None, info=None, item=None): + return f"thumb/{thumb_id}/{item.get('path')}" + + thumb_path = CustomImagesPipeline.from_settings(Settings( + {'IMAGES_STORE': self.tempdir} + )).thumb_path + item = dict(path='path-to-store-file') + request = Request("http://example.com") + self.assertEqual(thumb_path(request, 'small', item=item), 'thumb/small/path-to-store-file') + def test_convert_image(self): SIZE = (100, 100) # straigh forward case: RGB and JPEG diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 893d43052..a802c7cf1 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,4 +1,5 @@ from typing import Optional +import io from testfixtures import LogCapture from twisted.trial import unittest @@ -355,9 +356,12 @@ class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): def get_media_requests(self, item, info): item_url = item['image_urls'][0] + output_img = io.BytesIO() + img = Image.new('RGB', (60, 30), color='red') + img.save(output_img, format='JPEG') return Request( item_url, - meta={'response': Response(item_url, status=200, body=b'data')} + meta={'response': Response(item_url, status=200, body=output_img.getvalue())} ) def inc_stats(self, *args, **kwargs): @@ -379,9 +383,13 @@ class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): self._mockcalled.append('file_path') return super(MockedMediaPipelineDeprecatedMethods, self).file_path(request, response, info) + def thumb_path(self, request, thumb_id, response=None, info=None): + self._mockcalled.append('thumb_path') + return super(MockedMediaPipelineDeprecatedMethods, self).thumb_path(request, thumb_id, response, info) + def get_images(self, response, request, info): self._mockcalled.append('get_images') - return [] + return super(MockedMediaPipelineDeprecatedMethods, self).get_images(response, request, info) def image_downloaded(self, response, request, info): self._mockcalled.append('image_downloaded') @@ -392,7 +400,11 @@ class MediaPipelineDeprecatedMethodsTestCase(unittest.TestCase): skip = skip_pillow def setUp(self): - self.pipe = MockedMediaPipelineDeprecatedMethods(store_uri='store-uri', download_func=_mocked_download_func) + self.pipe = MockedMediaPipelineDeprecatedMethods( + store_uri='store-uri', + download_func=_mocked_download_func, + settings=Settings({"IMAGES_THUMBS": {'small': (50, 50)}}) + ) self.pipe.open_spider(None) self.item = dict(image_urls=['http://picsum.photos/id/1014/200/300'], images=[]) @@ -444,6 +456,16 @@ class MediaPipelineDeprecatedMethodsTestCase(unittest.TestCase): ) self._assert_method_called_with_warnings('file_path', message, warnings) + @inlineCallbacks + def test_thumb_path_called(self): + yield self.pipe.process_item(self.item, None) + warnings = self.flushWarnings([MediaPipeline._compatible]) + message = ( + 'thumb_path(self, request, thumb_id, response=None, info=None) is deprecated, ' + 'please use thumb_path(self, request, thumb_id, response=None, info=None, *, item=None)' + ) + self._assert_method_called_with_warnings('thumb_path', message, warnings) + @inlineCallbacks def test_get_images_called(self): yield self.pipe.process_item(self.item, None) From 896f16f2def7c276350421352dddf6e7b0145519 Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Wed, 25 May 2022 23:59:25 +0430 Subject: [PATCH 22/54] make thumb_path method backwards compatible --- scrapy/pipelines/images.py | 2 +- scrapy/pipelines/media.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 45ac03820..6b97190ee 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -179,6 +179,6 @@ class ImagesPipeline(FilesPipeline): image_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() return f'full/{image_guid}.jpg' - def thumb_path(self, request, thumb_id, response=None, info=None, item=None): + def thumb_path(self, request, thumb_id, response=None, info=None, *, item=None): thumb_guid = hashlib.sha1(to_bytes(request.url)).hexdigest() return f'thumbs/{thumb_id}/{thumb_guid}.jpg' diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index d1bccf323..430c37227 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -121,7 +121,7 @@ class MediaPipeline: def _make_compatible(self): """Make overridable methods of MediaPipeline and subclasses backwards compatible""" methods = [ - "file_path", "media_to_download", "media_downloaded", + "file_path", "thumb_path", "media_to_download", "media_downloaded", "file_downloaded", "image_downloaded", "get_images" ] From c5627af15bcf413c04539aeb47dd07cf8b3e4092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 7 Jun 2022 18:44:54 +0200 Subject: [PATCH 23/54] Centralize request fingerprints (#4524) Co-authored-by: Mikhail Korobov --- docs/conf.py | 2 + docs/news.rst | 2 +- docs/topics/api.rst | 7 + docs/topics/item-pipeline.rst | 4 +- docs/topics/request-response.rst | 268 +++++++ docs/topics/settings.rst | 8 +- scrapy/crawler.py | 7 + scrapy/dupefilters.py | 49 +- scrapy/extensions/httpcache.py | 14 +- scrapy/pipelines/media.py | 4 +- scrapy/settings/default_settings.py | 3 + .../templates/project/module/settings.py.tmpl | 3 + scrapy/utils/request.py | 235 +++++- scrapy/utils/test.py | 9 +- tests/test_crawler.py | 3 +- tests/test_dupefilters.py | 80 +- tests/test_pipeline_files.py | 5 +- tests/test_pipeline_media.py | 53 +- tests/test_utils_request.py | 699 ++++++++++++++++-- 19 files changed, 1304 insertions(+), 151 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 55aa72d5a..378b01804 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -294,7 +294,9 @@ intersphinx_mapping = { 'tox': ('https://tox.readthedocs.io/en/latest', None), 'twisted': ('https://twistedmatrix.com/documents/current', None), 'twistedapi': ('https://twistedmatrix.com/documents/current/api', None), + 'w3lib': ('https://w3lib.readthedocs.io/en/latest', None), } +intersphinx_disabled_reftypes = [] # Options for sphinx-hoverxref options diff --git a/docs/news.rst b/docs/news.rst index 5d92067b5..2d0ab485e 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -1643,7 +1643,7 @@ New features :issue:`4370`) * A new ``keep_fragments`` parameter of - :func:`scrapy.utils.request.request_fingerprint` allows to generate + ``scrapy.utils.request.request_fingerprint`` allows to generate different fingerprints for requests with different fragments in their URL (:issue:`4104`) diff --git a/docs/topics/api.rst b/docs/topics/api.rst index 900b19c7a..60b5acd10 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -32,6 +32,13 @@ how you :ref:`configure the downloader middlewares :class:`scrapy.Spider` subclass and a :class:`scrapy.settings.Settings` object. + .. attribute:: request_fingerprinter + + The request fingerprint builder of this crawler. + + This is used from extensions and middlewares to build short, unique + identifiers for requests. See :ref:`request-fingerprints`. + .. attribute:: settings The settings manager of this crawler. diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 391751364..882ff5661 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -60,9 +60,9 @@ Additionally, they may also implement the following methods: :param spider: the spider which was closed :type spider: :class:`~scrapy.Spider` object -.. method:: from_crawler(cls, crawler) +.. classmethod:: from_crawler(cls, crawler) - If present, this classmethod is called to create a pipeline instance + If present, this class method is called to create a pipeline instance from a :class:`~scrapy.crawler.Crawler`. It must return a new instance of the pipeline. Crawler object provides access to all Scrapy core components like settings and signals; it is a way for pipeline to diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 92a471faf..49cb69f67 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -339,6 +339,7 @@ errors if needed:: request = failure.request self.logger.error('TimeoutError on %s', request.url) + .. _errback-cb_kwargs: Accessing additional data in errback functions @@ -364,6 +365,273 @@ achieve this by using ``Failure.request.cb_kwargs``:: main_url=failure.request.cb_kwargs['main_url'], ) + +.. _request-fingerprints: + +Request fingerprints +-------------------- + +There are some aspects of scraping, such as filtering out duplicate requests +(see :setting:`DUPEFILTER_CLASS`) or caching responses (see +:setting:`HTTPCACHE_POLICY`), where you need the ability to generate a short, +unique identifier from a :class:`~scrapy.http.Request` object: a request +fingerprint. + +You often do not need to worry about request fingerprints, the default request +fingerprinter works for most projects. + +However, there is no universal way to generate a unique identifier from a +request, because different situations require comparing requests differently. +For example, sometimes you may need to compare URLs case-insensitively, include +URL fragments, exclude certain URL query parameters, include some or all +headers, etc. + +To change how request fingerprints are built for your requests, use the +:setting:`REQUEST_FINGERPRINTER_CLASS` setting. + +.. setting:: REQUEST_FINGERPRINTER_CLASS + +REQUEST_FINGERPRINTER_CLASS +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: VERSION + +Default: :class:`scrapy.utils.request.RequestFingerprinter` + +A :ref:`request fingerprinter class ` or its +import path. + +.. autoclass:: scrapy.utils.request.RequestFingerprinter + + +.. setting:: REQUEST_FINGERPRINTER_IMPLEMENTATION + +REQUEST_FINGERPRINTER_IMPLEMENTATION +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: VERSION + +Default: ``'PREVIOUS_VERSION'`` + +Determines which request fingerprinting algorithm is used by the default +request fingerprinter class (see :setting:`REQUEST_FINGERPRINTER_CLASS`). + +Possible values are: + +- ``'PREVIOUS_VERSION'`` (default) + + This implementation uses the same request fingerprinting algorithm as + Scrapy PREVIOUS_VERSION and earlier versions. + + Even though this is the default value for backward compatibility reasons, + it is a deprecated value. + +- ``'VERSION'`` + + This implementation was introduced in Scrapy VERSION to fix an issue of the + previous implementation. + + New projects should use this value. The :command:`startproject` command + sets this value in the generated ``settings.py`` file. + +If you are using the default value (``'PREVIOUS_VERSION'``) for this setting, and you are +using Scrapy components where changing the request fingerprinting algorithm +would cause undesired results, you need to carefully decide when to change the +value of this setting, or switch the :setting:`REQUEST_FINGERPRINTER_CLASS` +setting to a custom request fingerprinter class that implements the PREVIOUS_VERSION request +fingerprinting algorithm and does not log this warning ( +:ref:`PREVIOUS_VERSION-request-fingerprinter` includes an example implementation of such a +class). + +Scenarios where changing the request fingerprinting algorithm may cause +undesired results include, for example, using the HTTP cache middleware (see +:class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`). +Changing the request fingerprinting algorithm would invalidade the current +cache, requiring you to redownload all requests again. + +Otherwise, set :setting:`REQUEST_FINGERPRINTER_IMPLEMENTATION` to ``'VERSION'`` in +your settings to switch already to the request fingerprinting implementation +that will be the only request fingerprinting implementation available in a +future version of Scrapy, and remove the deprecation warning triggered by using +the default value (``'PREVIOUS_VERSION'``). + + +.. _PREVIOUS_VERSION-request-fingerprinter: +.. _custom-request-fingerprinter: + +Writing your own request fingerprinter +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A request fingerprinter is a class that must implement the following method: + +.. method:: fingerprint(self, request) + + Return a :class:`bytes` object that uniquely identifies *request*. + + See also :ref:`request-fingerprint-restrictions`. + + :param request: request to fingerprint + :type request: scrapy.http.Request + +Additionally, it may also implement the following methods: + +.. classmethod:: from_crawler(cls, crawler) + + If present, this class method is called to create a request fingerprinter + instance from a :class:`~scrapy.crawler.Crawler` object. It must return a + new instance of the request fingerprinter. + + *crawler* provides access to all Scrapy core components like settings and + signals; it is a way for the request fingerprinter to access them and hook + its functionality into Scrapy. + + :param crawler: crawler that uses this request fingerprinter + :type crawler: :class:`~scrapy.crawler.Crawler` object + +.. classmethod:: from_settings(cls, settings) + + If present, and ``from_crawler`` is not defined, this class method is called + to create a request fingerprinter instance from a + :class:`~scrapy.settings.Settings` object. It must return a new instance of + the request fingerprinter. + +The ``fingerprint`` method of the default request fingerprinter, +:class:`scrapy.utils.request.RequestFingerprinter`, uses +:func:`scrapy.utils.request.fingerprint` with its default parameters. For some +common use cases you can use :func:`~scrapy.utils.request.fingerprint` as well +in your ``fingerprint`` method implementation: + +.. autofunction:: scrapy.utils.request.fingerprint + +For example, to take the value of a request header named ``X-ID`` into +account:: + + # my_project/settings.py + REQUEST_FINGERPRINTER_CLASS = 'my_project.utils.RequestFingerprinter' + + # my_project/utils.py + from scrapy.utils.request import fingerprint + + class RequestFingerprinter: + + def fingerprint(self, request): + return fingerprint(request, include_headers=['X-ID']) + +You can also write your own fingerprinting logic from scratch. + +However, if you do not use :func:`~scrapy.utils.request.fingerprint`, make sure +you use :class:`~weakref.WeakKeyDictionary` to cache request fingerprints: + +- Caching saves CPU by ensuring that fingerprints are calculated only once + per request, and not once per Scrapy component that needs the fingerprint + of a request. + +- Using :class:`~weakref.WeakKeyDictionary` saves memory by ensuring that + request objects do not stay in memory forever just because you have + references to them in your cache dictionary. + +For example, to take into account only the URL of a request, without any prior +URL canonicalization or taking the request method or body into account:: + + from hashlib import sha1 + from weakref import WeakKeyDictionary + + from scrapy.utils.python import to_bytes + + class RequestFingerprinter: + + cache = WeakKeyDictionary() + + def fingerprint(self, request): + if request not in self.cache: + fp = sha1() + fp.update(to_bytes(request.url)) + self.cache[request] = fp.digest() + return self.cache[request] + +If you need to be able to override the request fingerprinting for arbitrary +requests from your spider callbacks, you may implement a request fingerprinter +that reads fingerprints from :attr:`request.meta ` +when available, and then falls back to +:func:`~scrapy.utils.request.fingerprint`. For example:: + + from scrapy.utils.request import fingerprint + + class RequestFingerprinter: + + def fingerprint(self, request): + if 'fingerprint' in request.meta: + return request.meta['fingerprint'] + return fingerprint(request) + +If you need to reproduce the same fingerprinting algorithm as Scrapy PREVIOUS_VERSION +without using the deprecated ``'PREVIOUS_VERSION'`` value of the +:setting:`REQUEST_FINGERPRINTER_IMPLEMENTATION` setting, use the following +request fingerprinter:: + + from hashlib import sha1 + from weakref import WeakKeyDictionary + + from scrapy.utils.python import to_bytes + from w3lib.url import canonicalize_url + + class RequestFingerprinter: + + cache = WeakKeyDictionary() + + def fingerprint(self, request): + if request not in self.cache: + fp = sha1() + fp.update(to_bytes(request.method)) + fp.update(to_bytes(canonicalize_url(request.url))) + fp.update(request.body or b'') + self.cache[request] = fp.digest() + return self.cache[request] + + +.. _request-fingerprint-restrictions: + +Request fingerprint restrictions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Scrapy components that use request fingerprints may impose additional +restrictions on the format of the fingerprints that your :ref:`request +fingerprinter ` generates. + +The following built-in Scrapy components have such restrictions: + +- :class:`scrapy.extensions.httpcache.FilesystemCacheStorage` (default + value of :setting:`HTTPCACHE_STORAGE`) + + Request fingerprints must be at least 1 byte long. + + Path and filename length limits of the file system of + :setting:`HTTPCACHE_DIR` also apply. Inside :setting:`HTTPCACHE_DIR`, + the following directory structure is created: + + - :attr:`Spider.name ` + + - first byte of a request fingerprint as hexadecimal + + - fingerprint as hexadecimal + + - filenames up to 16 characters long + + For example, if a request fingerprint is made of 20 bytes (default), + :setting:`HTTPCACHE_DIR` is ``'/home/user/project/.scrapy/httpcache'``, + and the name of your spider is ``'my_spider'`` your file system must + support a file path like:: + + /home/user/project/.scrapy/httpcache/my_spider/01/0123456789abcdef0123456789abcdef01234567/response_headers + +- :class:`scrapy.extensions.httpcache.DbmCacheStorage` + + The underlying DBM implementation must support keys as long as twice + the number of bytes of a request fingerprint, plus 5. For example, + if a request fingerprint is made of 20 bytes (default), + 45-character-long keys must be supported. + + .. _topics-request-meta: Request.meta special keys diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 4e105642d..2046c6446 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -825,12 +825,8 @@ Default: ``'scrapy.dupefilters.RFPDupeFilter'`` The class used to detect and filter duplicate requests. -The default (``RFPDupeFilter``) filters based on request fingerprint using -the ``scrapy.utils.request.request_fingerprint`` function. In order to change -the way duplicates are checked you could subclass ``RFPDupeFilter`` and -override its ``request_fingerprint`` method. This method should accept -scrapy :class:`~scrapy.Request` object and return its fingerprint -(a string). +The default (``RFPDupeFilter``) filters based on the +:setting:`REQUEST_FINGERPRINTER_CLASS` setting. You can disable filtering of duplicate requests by setting :setting:`DUPEFILTER_CLASS` to ``'scrapy.dupefilters.BaseDupeFilter'``. diff --git a/scrapy/crawler.py b/scrapy/crawler.py index a638254f1..fdca7b335 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -51,6 +51,7 @@ class Crawler: self.spidercls.update_settings(self.settings) self.signals = SignalManager(self) + self.stats = load_object(self.settings['STATS_CLASS'])(self) handler = LogCounterHandler(self, level=self.settings.get('LOG_LEVEL')) @@ -71,6 +72,12 @@ class Crawler: lf_cls = load_object(self.settings['LOG_FORMATTER']) self.logformatter = lf_cls.from_crawler(self) + self.request_fingerprinter = create_instance( + load_object(self.settings['REQUEST_FINGERPRINTER_CLASS']), + settings=self.settings, + crawler=self, + ) + reactor_class = self.settings.get("TWISTED_REACTOR") if init_reactor: # this needs to be done after the spider settings are merged, diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index 292c68099..d1b0559ef 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -1,14 +1,16 @@ import logging import os from typing import Optional, Set, Type, TypeVar +from warnings import warn from twisted.internet.defer import Deferred from scrapy.http.request import Request from scrapy.settings import BaseSettings from scrapy.spiders import Spider +from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.job import job_dir -from scrapy.utils.request import referer_str, request_fingerprint +from scrapy.utils.request import referer_str, RequestFingerprinter BaseDupeFilterTV = TypeVar("BaseDupeFilterTV", bound="BaseDupeFilter") @@ -39,8 +41,15 @@ RFPDupeFilterTV = TypeVar("RFPDupeFilterTV", bound="RFPDupeFilter") class RFPDupeFilter(BaseDupeFilter): """Request Fingerprint duplicates filter""" - def __init__(self, path: Optional[str] = None, debug: bool = False) -> None: + def __init__( + self, + path: Optional[str] = None, + debug: bool = False, + *, + fingerprinter=None, + ) -> None: self.file = None + self.fingerprinter = fingerprinter or RequestFingerprinter() self.fingerprints: Set[str] = set() self.logdupes = True self.debug = debug @@ -51,9 +60,39 @@ class RFPDupeFilter(BaseDupeFilter): self.fingerprints.update(x.rstrip() for x in self.file) @classmethod - def from_settings(cls: Type[RFPDupeFilterTV], settings: BaseSettings) -> RFPDupeFilterTV: + def from_settings(cls: Type[RFPDupeFilterTV], settings: BaseSettings, *, fingerprinter=None) -> RFPDupeFilterTV: debug = settings.getbool('DUPEFILTER_DEBUG') - return cls(job_dir(settings), debug) + try: + return cls(job_dir(settings), debug, fingerprinter=fingerprinter) + except TypeError: + warn( + "RFPDupeFilter subclasses must either modify their '__init__' " + "method to support a 'fingerprinter' parameter or reimplement " + "the 'from_settings' class method.", + ScrapyDeprecationWarning, + ) + result = cls(job_dir(settings), debug) + result.fingerprinter = fingerprinter + return result + + @classmethod + def from_crawler(cls, crawler): + try: + return cls.from_settings( + crawler.settings, + fingerprinter=crawler.request_fingerprinter, + ) + except TypeError: + warn( + "RFPDupeFilter subclasses must either modify their overridden " + "'__init__' method and 'from_settings' class method to " + "support a 'fingerprinter' parameter, or reimplement the " + "'from_crawler' class method.", + ScrapyDeprecationWarning, + ) + result = cls.from_settings(crawler.settings) + result.fingerprinter = crawler.request_fingerprinter + return result def request_seen(self, request: Request) -> bool: fp = self.request_fingerprint(request) @@ -65,7 +104,7 @@ class RFPDupeFilter(BaseDupeFilter): return False def request_fingerprint(self, request: Request) -> str: - return request_fingerprint(request) + return self.fingerprinter.fingerprint(request).hex() def close(self, reason: str) -> None: if self.file: diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index d0ae29b90..c71484cfa 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -14,7 +14,6 @@ from scrapy.responsetypes import responsetypes from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.project import data_path from scrapy.utils.python import to_bytes, to_unicode -from scrapy.utils.request import request_fingerprint logger = logging.getLogger(__name__) @@ -228,6 +227,8 @@ class DbmCacheStorage: logger.debug("Using DBM cache storage in %(cachepath)s", {'cachepath': dbpath}, extra={'spider': spider}) + self._fingerprinter = spider.crawler.request_fingerprinter + def close_spider(self, spider): self.db.close() @@ -244,7 +245,7 @@ class DbmCacheStorage: return response def store_response(self, spider, request, response): - key = self._request_key(request) + key = self._fingerprinter.fingerprint(request).hex() data = { 'status': response.status, 'url': response.url, @@ -255,7 +256,7 @@ class DbmCacheStorage: self.db[f'{key}_time'] = str(time()) def _read_data(self, spider, request): - key = self._request_key(request) + key = self._fingerprinter.fingerprint(request).hex() db = self.db tkey = f'{key}_time' if tkey not in db: @@ -267,9 +268,6 @@ class DbmCacheStorage: return pickle.loads(db[f'{key}_data']) - def _request_key(self, request): - return request_fingerprint(request) - class FilesystemCacheStorage: @@ -283,6 +281,8 @@ class FilesystemCacheStorage: logger.debug("Using filesystem cache storage in %(cachedir)s", {'cachedir': self.cachedir}, extra={'spider': spider}) + self._fingerprinter = spider.crawler.request_fingerprinter + def close_spider(self, spider): pass @@ -329,7 +329,7 @@ class FilesystemCacheStorage: f.write(request.body) def _get_request_path(self, spider, request): - key = request_fingerprint(request) + key = self._fingerprinter.fingerprint(request).hex() return os.path.join(self.cachedir, spider.name, key[0:2], key) def _read_meta(self, spider, request): diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 430c37227..5308a9793 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -11,7 +11,6 @@ from scrapy.settings import Settings from scrapy.utils.datatypes import SequenceExclude from scrapy.utils.defer import mustbe_deferred, defer_result from scrapy.utils.deprecate import ScrapyDeprecationWarning -from scrapy.utils.request import request_fingerprint from scrapy.utils.misc import arg_to_iter from scrapy.utils.log import failure_to_exc_info @@ -77,6 +76,7 @@ class MediaPipeline: except AttributeError: pipe = cls() pipe.crawler = crawler + pipe._fingerprinter = crawler.request_fingerprinter return pipe def open_spider(self, spider): @@ -90,7 +90,7 @@ class MediaPipeline: return dfd.addCallback(self.item_completed, item, info) def _process_request(self, request, info, item): - fp = request_fingerprint(request) + fp = self._fingerprinter.fingerprint(request) cb = request.callback or (lambda _: _) eb = request.errback request.callback = None diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 8389a70cb..f5a3efe69 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -246,6 +246,9 @@ REDIRECT_PRIORITY_ADJUST = +2 REFERER_ENABLED = True REFERRER_POLICY = 'scrapy.spidermiddlewares.referer.DefaultReferrerPolicy' +REQUEST_FINGERPRINTER_CLASS = 'scrapy.utils.request.RequestFingerprinter' +REQUEST_FINGERPRINTER_IMPLEMENTATION = 'PREVIOUS_VERSION' + RETRY_ENABLED = True RETRY_TIMES = 2 # initial response + 2 retries = 3 requests RETRY_HTTP_CODES = [500, 502, 503, 504, 522, 524, 408, 429] diff --git a/scrapy/templates/project/module/settings.py.tmpl b/scrapy/templates/project/module/settings.py.tmpl index a414b5fde..5e541e2c0 100644 --- a/scrapy/templates/project/module/settings.py.tmpl +++ b/scrapy/templates/project/module/settings.py.tmpl @@ -86,3 +86,6 @@ ROBOTSTXT_OBEY = True #HTTPCACHE_DIR = 'httpcache' #HTTPCACHE_IGNORE_HTTP_CODES = [] #HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage' + +# Set settings whose default value is deprecated to a future-proof value +REQUEST_FINGERPRINTER_IMPLEMENTATION = 'VERSION' diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py index 70ef3ba2b..cf33317ce 100644 --- a/scrapy/utils/request.py +++ b/scrapy/utils/request.py @@ -4,7 +4,9 @@ scrapy.http.Request objects """ import hashlib -from typing import Dict, Iterable, Optional, Tuple, Union +import json +import warnings +from typing import Dict, Iterable, List, Optional, Tuple, Union from urllib.parse import urlunparse from weakref import WeakKeyDictionary @@ -12,13 +14,22 @@ from w3lib.http import basic_auth_header from w3lib.url import canonicalize_url from scrapy import Request, Spider +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import load_object from scrapy.utils.python import to_bytes, to_unicode -_fingerprint_cache: "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], str]]" -_fingerprint_cache = WeakKeyDictionary() +_deprecated_fingerprint_cache: "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], str]]" +_deprecated_fingerprint_cache = WeakKeyDictionary() + + +def _serialize_headers(headers, request): + for header in headers: + if header in request.headers: + yield header + for value in request.headers.getlist(header): + yield value def request_fingerprint( @@ -26,6 +37,123 @@ def request_fingerprint( include_headers: Optional[Iterable[Union[bytes, str]]] = None, keep_fragments: bool = False, ) -> str: + """ + Return the request fingerprint as an hexadecimal string. + + The request fingerprint is a hash that uniquely identifies the resource the + request points to. For example, take the following two urls: + + http://www.example.com/query?id=111&cat=222 + http://www.example.com/query?cat=222&id=111 + + Even though those are two different URLs both point to the same resource + and are equivalent (i.e. they should return the same response). + + Another example are cookies used to store session ids. Suppose the + following page is only accessible to authenticated users: + + http://www.example.com/members/offers.html + + Lots of sites use a cookie to store the session id, which adds a random + component to the HTTP Request and thus should be ignored when calculating + the fingerprint. + + For this reason, request headers are ignored by default when calculating + the fingerprint. If you want to include specific headers use the + include_headers argument, which is a list of Request headers to include. + + Also, servers usually ignore fragments in urls when handling requests, + so they are also ignored by default when calculating the fingerprint. + If you want to include them, set the keep_fragments argument to True + (for instance when handling requests with a headless browser). + """ + if include_headers or keep_fragments: + message = ( + 'Call to deprecated function ' + 'scrapy.utils.request.request_fingerprint().\n' + '\n' + 'If you are using this function in a Scrapy component because you ' + 'need a non-default fingerprinting algorithm, and you are OK ' + 'with that non-default fingerprinting algorithm being used by ' + 'all Scrapy components and not just the one calling this ' + 'function, use crawler.request_fingerprinter.fingerprint() ' + 'instead in your Scrapy component (you can get the crawler ' + 'object from the \'from_crawler\' class method), and use the ' + '\'REQUEST_FINGERPRINTER_CLASS\' setting to configure your ' + 'non-default fingerprinting algorithm.\n' + '\n' + 'Otherwise, consider using the ' + 'scrapy.utils.request.fingerprint() function instead.\n' + '\n' + 'If you switch to \'fingerprint()\', or assign the ' + '\'REQUEST_FINGERPRINTER_CLASS\' setting a class that uses ' + '\'fingerprint()\', the generated fingerprints will not only be ' + 'bytes instead of a string, but they will also be different from ' + 'those generated by \'request_fingerprint()\'. Before you switch, ' + 'make sure that you understand the consequences of this (e.g. ' + 'cache invalidation) and are OK with them; otherwise, consider ' + 'implementing your own function which returns the same ' + 'fingerprints as the deprecated \'request_fingerprint()\' function.' + ) + else: + message = ( + 'Call to deprecated function ' + 'scrapy.utils.request.request_fingerprint().\n' + '\n' + 'If you are using this function in a Scrapy component, and you ' + 'are OK with users of your component changing the fingerprinting ' + 'algorithm through settings, use ' + 'crawler.request_fingerprinter.fingerprint() instead in your ' + 'Scrapy component (you can get the crawler object from the ' + '\'from_crawler\' class method).\n' + '\n' + 'Otherwise, consider using the ' + 'scrapy.utils.request.fingerprint() function instead.\n' + '\n' + 'Either way, the resulting fingerprints will be returned as ' + 'bytes, not as a string, and they will also be different from ' + 'those generated by \'request_fingerprint()\'. Before you switch, ' + 'make sure that you understand the consequences of this (e.g. ' + 'cache invalidation) and are OK with them; otherwise, consider ' + 'implementing your own function which returns the same ' + 'fingerprints as the deprecated \'request_fingerprint()\' function.' + ) + warnings.warn(message, category=ScrapyDeprecationWarning, stacklevel=2) + processed_include_headers: Optional[Tuple[bytes, ...]] = None + if include_headers: + processed_include_headers = tuple( + to_bytes(h.lower()) for h in sorted(include_headers) + ) + cache = _deprecated_fingerprint_cache.setdefault(request, {}) + cache_key = (processed_include_headers, keep_fragments) + if cache_key not in cache: + fp = hashlib.sha1() + fp.update(to_bytes(request.method)) + fp.update(to_bytes(canonicalize_url(request.url, keep_fragments=keep_fragments))) + fp.update(request.body or b'') + if processed_include_headers: + for part in _serialize_headers(processed_include_headers, request): + fp.update(part) + cache[cache_key] = fp.hexdigest() + return cache[cache_key] + + +def _request_fingerprint_as_bytes(*args, **kwargs): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + return bytes.fromhex(request_fingerprint(*args, **kwargs)) + + +_fingerprint_cache: "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], bytes]]" +_fingerprint_cache = WeakKeyDictionary() + + +def fingerprint( + request: Request, + *, + include_headers: Optional[Iterable[Union[bytes, str]]] = None, + keep_fragments: bool = False, +) -> bytes: """ Return the request fingerprint. @@ -43,7 +171,7 @@ def request_fingerprint( http://www.example.com/members/offers.html - Lot of sites use a cookie to store the session id, which adds a random + Lots of sites use a cookie to store the session id, which adds a random component to the HTTP Request and thus should be ignored when calculating the fingerprint. @@ -55,29 +183,96 @@ def request_fingerprint( so they are also ignored by default when calculating the fingerprint. If you want to include them, set the keep_fragments argument to True (for instance when handling requests with a headless browser). - """ - headers: Optional[Tuple[bytes, ...]] = None + processed_include_headers: Optional[Tuple[bytes, ...]] = None if include_headers: - headers = tuple(to_bytes(h.lower()) for h in sorted(include_headers)) + processed_include_headers = tuple( + to_bytes(h.lower()) for h in sorted(include_headers) + ) cache = _fingerprint_cache.setdefault(request, {}) - cache_key = (headers, keep_fragments) + cache_key = (processed_include_headers, keep_fragments) if cache_key not in cache: - fp = hashlib.sha1() - fp.update(to_bytes(request.method)) - fp.update(to_bytes(canonicalize_url(request.url, keep_fragments=keep_fragments))) - fp.update(request.body or b'') - if headers: - for hdr in headers: - if hdr in request.headers: - fp.update(hdr) - for v in request.headers.getlist(hdr): - fp.update(v) - cache[cache_key] = fp.hexdigest() + # To decode bytes reliably (JSON does not support bytes), regardless of + # character encoding, we use bytes.hex() + headers: Dict[str, List[str]] = {} + if processed_include_headers: + for header in processed_include_headers: + if header in request.headers: + headers[header.hex()] = [ + header_value.hex() + for header_value in request.headers.getlist(header) + ] + fingerprint_data = { + 'method': to_unicode(request.method), + 'url': canonicalize_url(request.url, keep_fragments=keep_fragments), + 'body': (request.body or b'').hex(), + 'headers': headers, + } + fingerprint_json = json.dumps(fingerprint_data, sort_keys=True) + cache[cache_key] = hashlib.sha1(fingerprint_json.encode()).digest() return cache[cache_key] -def request_authenticate(request: Request, username: str, password: str) -> None: +class RequestFingerprinter: + """Default fingerprinter. + + It takes into account a canonical version + (:func:`w3lib.url.canonicalize_url`) of :attr:`request.url + ` and the values of :attr:`request.method + ` and :attr:`request.body + `. It then generates an `SHA1 + `_ hash. + + .. seealso:: :setting:`REQUEST_FINGERPRINTER_IMPLEMENTATION`. + """ + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + + def __init__(self, crawler=None): + if crawler: + implementation = crawler.settings.get( + 'REQUEST_FINGERPRINTER_IMPLEMENTATION' + ) + else: + implementation = 'PREVIOUS_VERSION' + if implementation == 'PREVIOUS_VERSION': + message = ( + '\'PREVIOUS_VERSION\' is a deprecated value for the ' + '\'REQUEST_FINGERPRINTER_IMPLEMENTATION\' setting.\n' + '\n' + 'It is also the default value. In other words, it is normal ' + 'to get this warning if you have not defined a value for the ' + '\'REQUEST_FINGERPRINTER_IMPLEMENTATION\' setting. This is so ' + 'for backward compatibility reasons, but it will change in a ' + 'future version of Scrapy.\n' + '\n' + 'See the documentation of the ' + '\'REQUEST_FINGERPRINTER_IMPLEMENTATION\' setting for ' + 'information on how to handle this deprecation.' + ) + warnings.warn(message, category=ScrapyDeprecationWarning, stacklevel=2) + self._fingerprint = _request_fingerprint_as_bytes + elif implementation == 'VERSION': + self._fingerprint = fingerprint + else: + raise ValueError( + f'Got an invalid value on setting ' + f'\'REQUEST_FINGERPRINTER_IMPLEMENTATION\': ' + f'{implementation!r}. Valid values are \'PREVIOUS_VERSION\' (deprecated) ' + f'and \'VERSION\'.' + ) + + def fingerprint(self, request): + return self._fingerprint(request) + + +def request_authenticate( + request: Request, + username: str, + password: str, +) -> None: """Authenticate the given request (in place) using the HTTP basic access authentication mechanism (RFC 2617) and the given username and password """ diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 24c38283a..b90ea5009 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -54,7 +54,7 @@ def get_ftp_content_and_delete( return "".join(ftp_data) -def get_crawler(spidercls=None, settings_dict=None): +def get_crawler(spidercls=None, settings_dict=None, prevent_warnings=True): """Return an unconfigured Crawler object. If settings_dict is given, it will be used to populate the crawler settings with a project level priority. @@ -62,7 +62,12 @@ def get_crawler(spidercls=None, settings_dict=None): from scrapy.crawler import CrawlerRunner from scrapy.spiders import Spider - runner = CrawlerRunner(settings_dict) + # Set by default settings that prevent deprecation warnings. + settings = {} + if prevent_warnings: + settings['REQUEST_FINGERPRINTER_IMPLEMENTATION'] = 'VERSION' + settings.update(settings_dict or {}) + runner = CrawlerRunner(settings) return runner.create_crawler(spidercls or Spider) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 8f6227109..f7aa769e4 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -104,7 +104,8 @@ class CrawlerLoggingTestCase(unittest.TestCase): custom_settings = { 'LOG_LEVEL': 'INFO', 'LOG_FILE': log_file, - # disable telnet if not available to avoid an extra warning + # settings to avoid extra warnings + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION', 'TELNETCONSOLE_ENABLED': telnet.TWISTED_CONCH_AVAILABLE, } diff --git a/tests/test_dupefilters.py b/tests/test_dupefilters.py index 680bb6dc8..b7df2554a 100644 --- a/tests/test_dupefilters.py +++ b/tests/test_dupefilters.py @@ -15,6 +15,16 @@ from scrapy.utils.test import get_crawler from tests.spiders import SimpleSpider +def _get_dupefilter(*, crawler=None, settings=None, open=True): + if crawler is None: + crawler = get_crawler(settings_dict=settings) + scheduler = Scheduler.from_crawler(crawler) + dupefilter = scheduler.df + if open: + dupefilter.open() + return dupefilter + + class FromCrawlerRFPDupeFilter(RFPDupeFilter): @classmethod @@ -64,9 +74,7 @@ class RFPDupeFilterTest(unittest.TestCase): self.assertEqual(scheduler.df.method, 'n/a') def test_filter(self): - dupefilter = RFPDupeFilter() - dupefilter.open() - + dupefilter = _get_dupefilter() r1 = Request('http://scrapytest.org/1') r2 = Request('http://scrapytest.org/2') r3 = Request('http://scrapytest.org/2') @@ -85,7 +93,7 @@ class RFPDupeFilterTest(unittest.TestCase): path = tempfile.mkdtemp() try: - df = RFPDupeFilter(path) + df = _get_dupefilter(settings={'JOBDIR': path}, open=False) try: df.open() assert not df.request_seen(r1) @@ -93,7 +101,8 @@ class RFPDupeFilterTest(unittest.TestCase): finally: df.close('finished') - df2 = RFPDupeFilter(path) + df2 = _get_dupefilter(settings={'JOBDIR': path}, open=False) + assert df != df2 try: df2.open() assert df2.request_seen(r1) @@ -109,26 +118,24 @@ class RFPDupeFilterTest(unittest.TestCase): output of request_seen. """ + dupefilter = _get_dupefilter() r1 = Request('http://scrapytest.org/index.html') r2 = Request('http://scrapytest.org/INDEX.html') - dupefilter = RFPDupeFilter() - dupefilter.open() - assert not dupefilter.request_seen(r1) assert not dupefilter.request_seen(r2) dupefilter.close('finished') - class CaseInsensitiveRFPDupeFilter(RFPDupeFilter): + class RequestFingerprinter: - def request_fingerprint(self, request): + def fingerprint(self, request): fp = hashlib.sha1() fp.update(to_bytes(request.url.lower())) - return fp.hexdigest() + return fp.digest() - case_insensitive_dupefilter = CaseInsensitiveRFPDupeFilter() - case_insensitive_dupefilter.open() + settings = {'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter} + case_insensitive_dupefilter = _get_dupefilter(settings=settings) assert not case_insensitive_dupefilter.request_seen(r1) assert case_insensitive_dupefilter.request_seen(r2) @@ -142,8 +149,10 @@ class RFPDupeFilterTest(unittest.TestCase): r1 = Request('http://scrapytest.org/1') path = tempfile.mkdtemp() + crawler = get_crawler(settings_dict={'JOBDIR': path}) try: - df = RFPDupeFilter(path) + scheduler = Scheduler.from_crawler(crawler) + df = scheduler.df df.open() df.request_seen(r1) df.close('finished') @@ -164,11 +173,8 @@ class RFPDupeFilterTest(unittest.TestCase): settings = {'DUPEFILTER_DEBUG': False, 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} crawler = get_crawler(SimpleSpider, settings_dict=settings) - scheduler = Scheduler.from_crawler(crawler) spider = SimpleSpider.from_crawler(crawler) - - dupefilter = scheduler.df - dupefilter.open() + dupefilter = _get_dupefilter(crawler=crawler) r1 = Request('http://scrapytest.org/index.html') r2 = Request('http://scrapytest.org/index.html') @@ -193,11 +199,41 @@ class RFPDupeFilterTest(unittest.TestCase): settings = {'DUPEFILTER_DEBUG': True, 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} crawler = get_crawler(SimpleSpider, settings_dict=settings) - scheduler = Scheduler.from_crawler(crawler) spider = SimpleSpider.from_crawler(crawler) - - dupefilter = scheduler.df - dupefilter.open() + dupefilter = _get_dupefilter(crawler=crawler) + + r1 = Request('http://scrapytest.org/index.html') + r2 = Request('http://scrapytest.org/index.html', + headers={'Referer': 'http://scrapytest.org/INDEX.html'}) + + dupefilter.log(r1, spider) + dupefilter.log(r2, spider) + + assert crawler.stats.get_value('dupefilter/filtered') == 2 + log.check_present( + ( + 'scrapy.dupefilters', + 'DEBUG', + 'Filtered duplicate request: (referer: None)' + ) + ) + log.check_present( + ( + 'scrapy.dupefilters', + 'DEBUG', + 'Filtered duplicate request: ' + ' (referer: http://scrapytest.org/INDEX.html)' + ) + ) + + dupefilter.close('finished') + + def test_log_debug_default_dupefilter(self): + with LogCapture() as log: + settings = {'DUPEFILTER_DEBUG': True} + crawler = get_crawler(SimpleSpider, settings_dict=settings) + spider = SimpleSpider.from_crawler(crawler) + dupefilter = _get_dupefilter(crawler=crawler) r1 = Request('http://scrapytest.org/index.html') r2 = Request('http://scrapytest.org/index.html', diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 0ff2045ed..4228173ed 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -25,6 +25,7 @@ from scrapy.pipelines.files import ( from scrapy.settings import Settings from scrapy.utils.test import ( assert_gcs_environ, + get_crawler, get_ftp_content_and_delete, get_gcs_content_and_delete, skip_if_no_boto, @@ -47,7 +48,9 @@ class FilesPipelineTestCase(unittest.TestCase): def setUp(self): self.tempdir = mkdtemp() - self.pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir})) + settings_dict = {'FILES_STORE': self.tempdir} + crawler = get_crawler(spidercls=None, settings_dict=settings_dict) + self.pipeline = FilesPipeline.from_crawler(crawler) self.pipeline.download_func = _mocked_download_func self.pipeline.open_spider(None) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index a802c7cf1..84e867660 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -7,17 +7,17 @@ from twisted.python.failure import Failure from twisted.internet import reactor from twisted.internet.defer import Deferred, inlineCallbacks +from scrapy import signals from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.spiders import Spider -from scrapy.utils.deprecate import ScrapyDeprecationWarning -from scrapy.utils.request import request_fingerprint +from scrapy.pipelines.files import FileException from scrapy.pipelines.images import ImagesPipeline from scrapy.pipelines.media import MediaPipeline -from scrapy.pipelines.files import FileException +from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.log import failure_to_exc_info from scrapy.utils.signal import disconnect_all -from scrapy import signals +from scrapy.utils.test import get_crawler try: @@ -39,11 +39,14 @@ class BaseMediaPipelineTestCase(unittest.TestCase): settings = None def setUp(self): - self.spider = Spider('media.com') - self.pipe = self.pipeline_class(download_func=_mocked_download_func, - settings=Settings(self.settings)) + spider_cls = Spider + self.spider = spider_cls('media.com') + crawler = get_crawler(spider_cls, self.settings) + self.pipe = self.pipeline_class.from_crawler(crawler) + self.pipe.download_func = _mocked_download_func self.pipe.open_spider(self.spider) self.info = self.pipe.spiderinfo + self.fingerprint = crawler.request_fingerprinter.fingerprint def tearDown(self): for name, signal in vars(signals).items(): @@ -156,7 +159,7 @@ class BaseMediaPipelineTestCase(unittest.TestCase): self.assertEqual(failure.value.__context__, def_gen_return_exc) # Let's calculate the request fingerprint and fake some runtime data... - fp = request_fingerprint(request) + fp = self.fingerprint(request) info = self.pipe.spiderinfo info.downloading.add(fp) info.waiting[fp] = [] @@ -273,7 +276,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): item = dict(requests=req) # pass a single item new_item = yield self.pipe.process_item(item, self.spider) assert new_item is item - assert request_fingerprint(req) in self.info.downloaded + self.assertIn(self.fingerprint(req), self.info.downloaded) # returns iterable of Requests req1 = Request('http://url1') @@ -281,8 +284,8 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): item = dict(requests=iter([req1, req2])) new_item = yield self.pipe.process_item(item, self.spider) assert new_item is item - assert request_fingerprint(req1) in self.info.downloaded - assert request_fingerprint(req2) in self.info.downloaded + assert self.fingerprint(req1) in self.info.downloaded + assert self.fingerprint(req2) in self.info.downloaded @inlineCallbacks def test_results_are_cached_across_multiple_items(self): @@ -298,7 +301,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): item = dict(requests=req2) new_item = yield self.pipe.process_item(item, self.spider) self.assertTrue(new_item is item) - self.assertEqual(request_fingerprint(req1), request_fingerprint(req2)) + self.assertEqual(self.fingerprint(req1), self.fingerprint(req2)) self.assertEqual(new_item['results'], [(True, rsp1)]) @inlineCallbacks @@ -314,7 +317,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): @inlineCallbacks def test_wait_if_request_is_downloading(self): def _check_downloading(response): - fp = request_fingerprint(req1) + fp = self.fingerprint(req1) self.assertTrue(fp in self.info.downloading) self.assertTrue(fp in self.info.waiting) self.assertTrue(fp not in self.info.downloaded) @@ -351,7 +354,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): def __init__(self, *args, **kwargs): - super(MockedMediaPipelineDeprecatedMethods, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self._mockcalled = [] def get_media_requests(self, item, info): @@ -369,19 +372,19 @@ class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): def media_to_download(self, request, info): self._mockcalled.append('media_to_download') - return super(MockedMediaPipelineDeprecatedMethods, self).media_to_download(request, info) + return super().media_to_download(request, info) def media_downloaded(self, response, request, info): self._mockcalled.append('media_downloaded') - return super(MockedMediaPipelineDeprecatedMethods, self).media_downloaded(response, request, info) + return super().media_downloaded(response, request, info) def file_downloaded(self, response, request, info): self._mockcalled.append('file_downloaded') - return super(MockedMediaPipelineDeprecatedMethods, self).file_downloaded(response, request, info) + return super().file_downloaded(response, request, info) def file_path(self, request, response=None, info=None): self._mockcalled.append('file_path') - return super(MockedMediaPipelineDeprecatedMethods, self).file_path(request, response, info) + return super().file_path(request, response, info) def thumb_path(self, request, thumb_id, response=None, info=None): self._mockcalled.append('thumb_path') @@ -393,18 +396,20 @@ class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): def image_downloaded(self, response, request, info): self._mockcalled.append('image_downloaded') - return super(MockedMediaPipelineDeprecatedMethods, self).image_downloaded(response, request, info) + return super().image_downloaded(response, request, info) class MediaPipelineDeprecatedMethodsTestCase(unittest.TestCase): skip = skip_pillow def setUp(self): - self.pipe = MockedMediaPipelineDeprecatedMethods( - store_uri='store-uri', - download_func=_mocked_download_func, - settings=Settings({"IMAGES_THUMBS": {'small': (50, 50)}}) - ) + settings_dict = { + 'IMAGES_STORE': 'store-uri', + 'IMAGES_THUMBS': {'small': (50, 50)}, + } + crawler = get_crawler(spidercls=None, settings_dict=settings_dict) + self.pipe = MockedMediaPipelineDeprecatedMethods.from_crawler(crawler) + self.pipe.download_func = _mocked_download_func self.pipe.open_spider(None) self.item = dict(image_urls=['http://picsum.photos/id/1014/200/300'], images=[]) diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 7e0049b1d..e9edfee98 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -1,73 +1,29 @@ import unittest +import warnings +from hashlib import sha1 +from typing import Dict, Mapping, Optional, Tuple, Union +from weakref import WeakKeyDictionary + +import pytest +from w3lib.url import canonicalize_url + from scrapy.http import Request +from scrapy.utils.deprecate import ScrapyDeprecationWarning +from scrapy.utils.python import to_bytes from scrapy.utils.request import ( + _deprecated_fingerprint_cache, _fingerprint_cache, + _request_fingerprint_as_bytes, + fingerprint, request_authenticate, request_fingerprint, request_httprepr, ) +from scrapy.utils.test import get_crawler class UtilsRequestTest(unittest.TestCase): - def test_request_fingerprint(self): - r1 = Request("http://www.example.com/query?id=111&cat=222") - r2 = Request("http://www.example.com/query?cat=222&id=111") - self.assertEqual(request_fingerprint(r1), request_fingerprint(r1)) - self.assertEqual(request_fingerprint(r1), request_fingerprint(r2)) - - r1 = Request('http://www.example.com/hnnoticiaj1.aspx?78132,199') - r2 = Request('http://www.example.com/hnnoticiaj1.aspx?78160,199') - self.assertNotEqual(request_fingerprint(r1), request_fingerprint(r2)) - - # make sure caching is working - self.assertEqual(request_fingerprint(r1), _fingerprint_cache[r1][(None, False)]) - - r1 = Request("http://www.example.com/members/offers.html") - r2 = Request("http://www.example.com/members/offers.html") - r2.headers['SESSIONID'] = b"somehash" - self.assertEqual(request_fingerprint(r1), request_fingerprint(r2)) - - r1 = Request("http://www.example.com/") - r2 = Request("http://www.example.com/") - r2.headers['Accept-Language'] = b'en' - r3 = Request("http://www.example.com/") - r3.headers['Accept-Language'] = b'en' - r3.headers['SESSIONID'] = b"somehash" - - self.assertEqual(request_fingerprint(r1), request_fingerprint(r2), request_fingerprint(r3)) - - self.assertEqual(request_fingerprint(r1), - request_fingerprint(r1, include_headers=['Accept-Language'])) - - self.assertNotEqual( - request_fingerprint(r1), - request_fingerprint(r2, include_headers=['Accept-Language'])) - - self.assertEqual(request_fingerprint(r3, include_headers=['accept-language', 'sessionid']), - request_fingerprint(r3, include_headers=['SESSIONID', 'Accept-Language'])) - - r1 = Request("http://www.example.com/test.html") - r2 = Request("http://www.example.com/test.html#fragment") - self.assertEqual(request_fingerprint(r1), request_fingerprint(r2)) - self.assertEqual(request_fingerprint(r1), request_fingerprint(r1, keep_fragments=True)) - self.assertNotEqual(request_fingerprint(r2), request_fingerprint(r2, keep_fragments=True)) - self.assertNotEqual(request_fingerprint(r1), request_fingerprint(r2, keep_fragments=True)) - - r1 = Request("http://www.example.com") - r2 = Request("http://www.example.com", method='POST') - r3 = Request("http://www.example.com", method='POST', body=b'request body') - - self.assertNotEqual(request_fingerprint(r1), request_fingerprint(r2)) - self.assertNotEqual(request_fingerprint(r2), request_fingerprint(r3)) - - # cached fingerprint must be cleared on request copy - r1 = Request("http://www.example.com") - fp1 = request_fingerprint(r1) - r2 = r1.replace(url="http://www.example.com/other") - fp2 = request_fingerprint(r2) - self.assertNotEqual(fp1, fp2) - def test_request_authenticate(self): r = Request("http://www.example.com") request_authenticate(r, 'someuser', 'somepass') @@ -93,5 +49,632 @@ class UtilsRequestTest(unittest.TestCase): request_httprepr(Request("ftp://localhost/tmp/foo.txt")) +class FingerprintTest(unittest.TestCase): + maxDiff = None + + function = staticmethod(fingerprint) + cache: Union[ + "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], bytes]]", + "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], str]]", + ] = _fingerprint_cache + default_cache_key = (None, False) + known_hashes: Tuple[Tuple[Request, Union[bytes, str], Dict], ...] = ( + ( + Request("http://example.org"), + b'xs\xd7\x0c3uj\x15\xfe\xd7d\x9b\xa9\t\xe0d\xbf\x9cXD', + {}, + ), + ( + Request("https://example.org"), + b'\xc04\x85P,\xaa\x91\x06\xf8t\xb4\xbd*\xd9\xe9\x8a:m\xc3l', + {}, + ), + ( + Request("https://example.org?a"), + b'G\xad\xb8Ck\x19\x1c\xed\x838,\x01\xc4\xde;\xee\xa5\x94a\x0c', + {}, + ), + ( + Request("https://example.org?a=b"), + b'\x024MYb\x8a\xc2\x1e\xbc>\xd6\xac*\xda\x9cF\xc1r\x7f\x17', + {}, + ), + ( + Request("https://example.org?a=b&a"), + b't+\xe8*\xfb\x84\xe3v\x1a}\x88p\xc0\xccB\xd7\x9d\xfez\x96', + {}, + ), + ( + Request("https://example.org?a=b&a=c"), + b'\xda\x1ec\xd0\x9c\x08s`\xb4\x9b\xe2\xb6R\xf8k\xef\xeaQG\xef', + {}, + ), + ( + Request("https://example.org", method='POST'), + b'\x9d\xcdA\x0fT\x02:\xca\xa0}\x90\xda\x05B\xded\x8aN7\x1d', + {}, + ), + ( + Request("https://example.org", body=b'a'), + b'\xc34z>\xd8\x99\x8b\xda7\x05r\x99I\xa8\xa0x;\xa41_', + {}, + ), + ( + Request("https://example.org", method='POST', body=b'a'), + b'5`\xe2y4\xd0\x9d\xee\xe0\xbatw\x87Q\xe8O\xd78\xfc\xe7', + {}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + b'\xc04\x85P,\xaa\x91\x06\xf8t\xb4\xbd*\xd9\xe9\x8a:m\xc3l', + {}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + b']\xc7\x1f\xf2\xafG2\xbc\xa4\xfa\x99\n33\xda\x18\x94\x81U.', + {'include_headers': ['A']}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + b'<\x1a\xeb\x85y\xdeW\xfb\xdcq\x88\xee\xaf\x17\xdd\x0c\xbfH\x18\x1f', + {'keep_fragments': True}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + b'\xc1\xef~\x94\x9bS\xc1\x83\t\xdcz8\x9f\xdc{\x11\x16I.\x11', + {'include_headers': ['A'], 'keep_fragments': True}, + ), + ( + Request("https://example.org/ab"), + b'N\xe5l\xb8\x12@iw\xe2\xf3\x1bp\xea\xffp!u\xe2\x8a\xc6', + {}, + ), + ( + Request("https://example.org/a", body=b'b'), + b'_NOv\xbco$6\xfcW\x9f\xb24g\x9f\xbb\xdd\xa82\xc5', + {}, + ), + ) + + def test_query_string_key_order(self): + r1 = Request("http://www.example.com/query?id=111&cat=222") + r2 = Request("http://www.example.com/query?cat=222&id=111") + self.assertEqual(self.function(r1), self.function(r1)) + self.assertEqual(self.function(r1), self.function(r2)) + + def test_query_string_key_without_value(self): + r1 = Request('http://www.example.com/hnnoticiaj1.aspx?78132,199') + r2 = Request('http://www.example.com/hnnoticiaj1.aspx?78160,199') + self.assertNotEqual(self.function(r1), self.function(r2)) + + def test_caching(self): + r1 = Request('http://www.example.com/hnnoticiaj1.aspx?78160,199') + self.assertEqual( + self.function(r1), + self.cache[r1][self.default_cache_key] + ) + + def test_header(self): + r1 = Request("http://www.example.com/members/offers.html") + r2 = Request("http://www.example.com/members/offers.html") + r2.headers['SESSIONID'] = b"somehash" + self.assertEqual(self.function(r1), self.function(r2)) + + def test_headers(self): + r1 = Request("http://www.example.com/") + r2 = Request("http://www.example.com/") + r2.headers['Accept-Language'] = b'en' + r3 = Request("http://www.example.com/") + r3.headers['Accept-Language'] = b'en' + r3.headers['SESSIONID'] = b"somehash" + + self.assertEqual(self.function(r1), self.function(r2), self.function(r3)) + + self.assertEqual(self.function(r1), + self.function(r1, include_headers=['Accept-Language'])) + + self.assertNotEqual( + self.function(r1), + self.function(r2, include_headers=['Accept-Language'])) + + self.assertEqual(self.function(r3, include_headers=['accept-language', 'sessionid']), + self.function(r3, include_headers=['SESSIONID', 'Accept-Language'])) + + def test_fragment(self): + r1 = Request("http://www.example.com/test.html") + r2 = Request("http://www.example.com/test.html#fragment") + self.assertEqual(self.function(r1), self.function(r2)) + self.assertEqual(self.function(r1), self.function(r1, keep_fragments=True)) + self.assertNotEqual(self.function(r2), self.function(r2, keep_fragments=True)) + self.assertNotEqual(self.function(r1), self.function(r2, keep_fragments=True)) + + def test_method_and_body(self): + r1 = Request("http://www.example.com") + r2 = Request("http://www.example.com", method='POST') + r3 = Request("http://www.example.com", method='POST', body=b'request body') + + self.assertNotEqual(self.function(r1), self.function(r2)) + self.assertNotEqual(self.function(r2), self.function(r3)) + + def test_request_replace(self): + # cached fingerprint must be cleared on request copy + r1 = Request("http://www.example.com") + fp1 = self.function(r1) + r2 = r1.replace(url="http://www.example.com/other") + fp2 = self.function(r2) + self.assertNotEqual(fp1, fp2) + + def test_part_separation(self): + # An old implementation used to serialize request data in a way that + # would put the body right after the URL. + r1 = Request("http://www.example.com/foo") + fp1 = self.function(r1) + r2 = Request("http://www.example.com/f", body=b'oo') + fp2 = self.function(r2) + self.assertNotEqual(fp1, fp2) + + def test_hashes(self): + """Test hardcoded hashes, to make sure future changes to not introduce + backward incompatibilities.""" + actual = [ + self.function(request, **kwargs) + for request, _, kwargs in self.known_hashes + ] + expected = [ + _fingerprint + for _, _fingerprint, _ in self.known_hashes + ] + self.assertEqual(actual, expected) + + +class RequestFingerprintTest(FingerprintTest): + function = staticmethod(request_fingerprint) + cache = _deprecated_fingerprint_cache + known_hashes: Tuple[Tuple[Request, Union[bytes, str], Dict], ...] = ( + ( + Request("http://example.org"), + 'b2e5245ef826fd9576c93bd6e392fce3133fab62', + {}, + ), + ( + Request("https://example.org"), + 'bd10a0a89ea32cdee77917320f1309b0da87e892', + {}, + ), + ( + Request("https://example.org?a"), + '2fb7d48ae02f04b749f40caa969c0bc3c43204ce', + {}, + ), + ( + Request("https://example.org?a=b"), + '42e5fe149b147476e3f67ad0670c57b4cc57856a', + {}, + ), + ( + Request("https://example.org?a=b&a"), + 'd23a9787cb56c6375c2cae4453c5a8c634526942', + {}, + ), + ( + Request("https://example.org?a=b&a=c"), + '9a18a7a8552a9182b7f1e05d33876409e421e5c5', + {}, + ), + ( + Request("https://example.org", method='POST'), + 'ba20a80cb5c5ca460021ceefb3c2467b2bfd1bc6', + {}, + ), + ( + Request("https://example.org", body=b'a'), + '4bb136e54e715a4ea7a9dd1101831765d33f2d60', + {}, + ), + ( + Request("https://example.org", method='POST', body=b'a'), + '6c6595374a304b293be762f7b7be3f54e9947c65', + {}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + 'bd10a0a89ea32cdee77917320f1309b0da87e892', + {}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + '515b633cb3ca502a33a9d8c890e889ec1e425e65', + {'include_headers': ['A']}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + '505c96e7da675920dfef58725e8c957dfdb38f47', + {'keep_fragments': True}, + ), + ( + Request("https://example.org#a", headers={'A': b'B'}), + 'd6f673cdcb661b7970c2b9a00ee63e87d1e2e5da', + {'include_headers': ['A'], 'keep_fragments': True}, + ), + ( + Request("https://example.org/ab"), + '4e2870fee58582d6f81755e9b8fdefe3cba0c951', + {}, + ), + ( + Request("https://example.org/a", body=b'b'), + '4e2870fee58582d6f81755e9b8fdefe3cba0c951', + {}, + ), + ) + + @pytest.mark.xfail(reason='known bug kept for backward compatibility', strict=True) + def test_part_separation(self): + super().test_part_separation() + + def test_deprecation_default_parameters(self): + with pytest.warns(ScrapyDeprecationWarning) as warnings: + self.function(Request("http://www.example.com")) + messages = [str(warning.message) for warning in warnings] + self.assertTrue( + any( + 'Call to deprecated function' in message + for message in messages + ) + ) + self.assertFalse(any('non-default' in message for message in messages)) + + def test_deprecation_non_default_parameters(self): + with pytest.warns(ScrapyDeprecationWarning) as warnings: + self.function(Request("http://www.example.com"), keep_fragments=True) + messages = [str(warning.message) for warning in warnings] + self.assertTrue( + any( + 'Call to deprecated function' in message + for message in messages + ) + ) + self.assertTrue(any('non-default' in message for message in messages)) + + +class RequestFingerprintAsBytesTest(FingerprintTest): + function = staticmethod(_request_fingerprint_as_bytes) + cache = _deprecated_fingerprint_cache + known_hashes = RequestFingerprintTest.known_hashes + + def test_caching(self): + r1 = Request('http://www.example.com/hnnoticiaj1.aspx?78160,199') + self.assertEqual( + self.function(r1), + bytes.fromhex(self.cache[r1][self.default_cache_key]) + ) + + @pytest.mark.xfail(reason='known bug kept for backward compatibility', strict=True) + def test_part_separation(self): + super().test_part_separation() + + def test_hashes(self): + actual = [ + self.function(request, **kwargs) + for request, _, kwargs in self.known_hashes + ] + expected = [ + bytes.fromhex(_fingerprint) + for _, _fingerprint, _ in self.known_hashes + ] + self.assertEqual(actual, expected) + + +_fingerprint_cache_2_6: Mapping[Request, Tuple[None, bool]] = WeakKeyDictionary() + + +def request_fingerprint_2_6(request, include_headers=None, keep_fragments=False): + if include_headers: + include_headers = tuple(to_bytes(h.lower()) for h in sorted(include_headers)) + cache = _fingerprint_cache_2_6.setdefault(request, {}) + cache_key = (include_headers, keep_fragments) + if cache_key not in cache: + fp = sha1() + fp.update(to_bytes(request.method)) + fp.update(to_bytes(canonicalize_url(request.url, keep_fragments=keep_fragments))) + fp.update(request.body or b'') + if include_headers: + for hdr in include_headers: + if hdr in request.headers: + fp.update(hdr) + for v in request.headers.getlist(hdr): + fp.update(v) + cache[cache_key] = fp.hexdigest() + return cache[cache_key] + + +REQUEST_OBJECTS_TO_TEST = ( + Request("http://www.example.com/"), + Request("http://www.example.com/query?id=111&cat=222"), + Request("http://www.example.com/query?cat=222&id=111"), + Request('http://www.example.com/hnnoticiaj1.aspx?78132,199'), + Request('http://www.example.com/hnnoticiaj1.aspx?78160,199'), + Request("http://www.example.com/members/offers.html"), + Request( + "http://www.example.com/members/offers.html", + headers={'SESSIONID': b"somehash"}, + ), + Request( + "http://www.example.com/", + headers={'Accept-Language': b"en"}, + ), + Request( + "http://www.example.com/", + headers={ + 'Accept-Language': b"en", + 'SESSIONID': b"somehash", + }, + ), + Request("http://www.example.com/test.html"), + Request("http://www.example.com/test.html#fragment"), + Request("http://www.example.com", method='POST'), + Request("http://www.example.com", method='POST', body=b'request body'), +) + + +class BackwardCompatibilityTestCase(unittest.TestCase): + + def test_function_backward_compatibility(self): + include_headers_to_test = ( + None, + ['Accept-Language'], + ['accept-language', 'sessionid'], + ['SESSIONID', 'Accept-Language'], + ) + for request_object in REQUEST_OBJECTS_TO_TEST: + for include_headers in include_headers_to_test: + for keep_fragments in (False, True): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + fp = request_fingerprint( + request_object, + include_headers=include_headers, + keep_fragments=keep_fragments, + ) + old_fp = request_fingerprint_2_6( + request_object, + include_headers=include_headers, + keep_fragments=keep_fragments, + ) + self.assertEqual(fp, old_fp) + + def test_component_backward_compatibility(self): + for request_object in REQUEST_OBJECTS_TO_TEST: + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + crawler = get_crawler(prevent_warnings=False) + fp = crawler.request_fingerprinter.fingerprint(request_object) + old_fp = request_fingerprint_2_6(request_object) + self.assertEqual(fp.hex(), old_fp) + + def test_custom_component_backward_compatibility(self): + """Tests that the backward-compatible request fingerprinting class featured + in the documentation is indeed backward compatible and does not cause a + warning to be logged.""" + + class RequestFingerprinter: + + cache = WeakKeyDictionary() + + def fingerprint(self, request): + if request not in self.cache: + fp = sha1() + fp.update(to_bytes(request.method)) + fp.update(to_bytes(canonicalize_url(request.url))) + fp.update(request.body or b'') + self.cache[request] = fp.digest() + return self.cache[request] + + for request_object in REQUEST_OBJECTS_TO_TEST: + with warnings.catch_warnings() as logged_warnings: + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + } + crawler = get_crawler(settings_dict=settings) + fp = crawler.request_fingerprinter.fingerprint(request_object) + old_fp = request_fingerprint_2_6(request_object) + self.assertEqual(fp.hex(), old_fp) + self.assertFalse(logged_warnings) + + +class RequestFingerprinterTestCase(unittest.TestCase): + + def test_default_implementation(self): + with warnings.catch_warnings(record=True) as logged_warnings: + crawler = get_crawler(prevent_warnings=False) + request = Request('https://example.com') + self.assertEqual( + crawler.request_fingerprinter.fingerprint(request), + _request_fingerprint_as_bytes(request), + ) + self.assertTrue(logged_warnings) + + def test_deprecated_implementation(self): + settings = { + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'PREVIOUS_VERSION', + } + with warnings.catch_warnings(record=True) as logged_warnings: + crawler = get_crawler(settings_dict=settings) + request = Request('https://example.com') + self.assertEqual( + crawler.request_fingerprinter.fingerprint(request), + _request_fingerprint_as_bytes(request), + ) + self.assertTrue(logged_warnings) + + def test_recommended_implementation(self): + settings = { + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION', + } + with warnings.catch_warnings(record=True) as logged_warnings: + crawler = get_crawler(settings_dict=settings) + request = Request('https://example.com') + self.assertEqual( + crawler.request_fingerprinter.fingerprint(request), + fingerprint(request), + ) + self.assertFalse(logged_warnings) + + def test_unknown_implementation(self): + settings = { + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': '2.5', + } + with self.assertRaises(ValueError): + get_crawler(settings_dict=settings) + + +class CustomRequestFingerprinterTestCase(unittest.TestCase): + + def test_include_headers(self): + + class RequestFingerprinter: + + def fingerprint(self, request): + return fingerprint(request, include_headers=['X-ID']) + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + } + crawler = get_crawler(settings_dict=settings) + + r1 = Request("http://www.example.com", headers={'X-ID': '1'}) + fp1 = crawler.request_fingerprinter.fingerprint(r1) + r2 = Request("http://www.example.com", headers={'X-ID': '2'}) + fp2 = crawler.request_fingerprinter.fingerprint(r2) + self.assertNotEqual(fp1, fp2) + + def test_dont_canonicalize(self): + + class RequestFingerprinter: + cache = WeakKeyDictionary() + + def fingerprint(self, request): + if request not in self.cache: + fp = sha1() + fp.update(to_bytes(request.url)) + self.cache[request] = fp.digest() + return self.cache[request] + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + } + crawler = get_crawler(settings_dict=settings) + + r1 = Request("http://www.example.com?a=1&a=2") + fp1 = crawler.request_fingerprinter.fingerprint(r1) + r2 = Request("http://www.example.com?a=2&a=1") + fp2 = crawler.request_fingerprinter.fingerprint(r2) + self.assertNotEqual(fp1, fp2) + + def test_meta(self): + + class RequestFingerprinter: + + def fingerprint(self, request): + if 'fingerprint' in request.meta: + return request.meta['fingerprint'] + return fingerprint(request) + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + } + crawler = get_crawler(settings_dict=settings) + + r1 = Request("http://www.example.com") + fp1 = crawler.request_fingerprinter.fingerprint(r1) + r2 = Request("http://www.example.com", meta={'fingerprint': 'a'}) + fp2 = crawler.request_fingerprinter.fingerprint(r2) + r3 = Request("http://www.example.com", meta={'fingerprint': 'a'}) + fp3 = crawler.request_fingerprinter.fingerprint(r3) + r4 = Request("http://www.example.com", meta={'fingerprint': 'b'}) + fp4 = crawler.request_fingerprinter.fingerprint(r4) + self.assertNotEqual(fp1, fp2) + self.assertNotEqual(fp1, fp4) + self.assertNotEqual(fp2, fp4) + self.assertEqual(fp2, fp3) + + def test_from_crawler(self): + + class RequestFingerprinter: + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + + def __init__(self, crawler): + self._fingerprint = crawler.settings['FINGERPRINT'] + + def fingerprint(self, request): + return self._fingerprint + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + 'FINGERPRINT': b'fingerprint', + } + crawler = get_crawler(settings_dict=settings) + + request = Request("http://www.example.com") + fingerprint = crawler.request_fingerprinter.fingerprint(request) + self.assertEqual(fingerprint, settings['FINGERPRINT']) + + def test_from_settings(self): + + class RequestFingerprinter: + + @classmethod + def from_settings(cls, settings): + return cls(settings) + + def __init__(self, settings): + self._fingerprint = settings['FINGERPRINT'] + + def fingerprint(self, request): + return self._fingerprint + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + 'FINGERPRINT': b'fingerprint', + } + crawler = get_crawler(settings_dict=settings) + + request = Request("http://www.example.com") + fingerprint = crawler.request_fingerprinter.fingerprint(request) + self.assertEqual(fingerprint, settings['FINGERPRINT']) + + def test_from_crawler_and_settings(self): + + class RequestFingerprinter: + + # This method is ignored due to the presence of from_crawler + @classmethod + def from_settings(cls, settings): + return cls(settings) + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + + def __init__(self, crawler): + self._fingerprint = crawler.settings['FINGERPRINT'] + + def fingerprint(self, request): + return self._fingerprint + + settings = { + 'REQUEST_FINGERPRINTER_CLASS': RequestFingerprinter, + 'FINGERPRINT': b'fingerprint', + } + crawler = get_crawler(settings_dict=settings) + + request = Request("http://www.example.com") + fingerprint = crawler.request_fingerprinter.fingerprint(request) + self.assertEqual(fingerprint, settings['FINGERPRINT']) + + if __name__ == "__main__": unittest.main() From 407562b38b6ab375ae650c8799bdd511025527f4 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 9 Jun 2022 00:25:03 -0300 Subject: [PATCH 24/54] Drop Python 3.6 support (#5514) * chore: Drop Python 3.6 support * Attend PR comments * Tweak versions * Update dependencies version * fix: Ubuntu workflow * fix windows workflow * chore: Remove comment * update `install_requires` dependencies versions * move lxml to main pinned requirements * Attend code-review comments * remove non-pinned 3.7 from windows workflow * simplify condition * lint * remove paragraph * refactor * remove leftover --- .github/workflows/checks.yml | 2 +- .github/workflows/tests-macos.yml | 2 +- .github/workflows/tests-ubuntu.yml | 11 ++++------- .github/workflows/tests-windows.yml | 5 +---- README.rst | 2 +- docs/contributing.rst | 10 +++++----- docs/intro/install.rst | 4 ++-- docs/topics/items.rst | 5 ----- docs/topics/media-pipeline.rst | 2 +- scrapy/__init__.py | 4 ++-- scrapy/utils/py36.py | 11 ----------- setup.py | 19 ++++++------------- tests/requirements.txt | 4 +--- tests/test_utils_python.py | 8 +------- tox.ini | 23 ++++++++--------------- 15 files changed, 34 insertions(+), 78 deletions(-) delete mode 100644 scrapy/utils/py36.py diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 98fa44c7f..b26f344ff 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -19,7 +19,7 @@ jobs: - python-version: 3.8 env: TOXENV: pylint - - python-version: 3.6 + - python-version: 3.7 env: TOXENV: typing - python-version: "3.10" # Keep in sync with .readthedocs.yml diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 3aaf688c7..7819a4e12 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -7,7 +7,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"] + python-version: ["3.7", "3.8", "3.9", "3.10"] steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 1fc8d914b..be40c7c71 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -8,9 +8,6 @@ jobs: fail-fast: false matrix: include: - - python-version: 3.7 - env: - TOXENV: py - python-version: 3.8 env: TOXENV: py @@ -26,19 +23,19 @@ jobs: - python-version: pypy3 env: TOXENV: pypy3 - PYPY_VERSION: 3.6-v7.3.3 + PYPY_VERSION: 3.9-v7.3.9 # pinned deps - - python-version: 3.6.12 + - python-version: 3.7.13 env: TOXENV: pinned - - python-version: 3.6.12 + - python-version: 3.7.13 env: TOXENV: asyncio-pinned - python-version: pypy3 env: TOXENV: pypy3-pinned - PYPY_VERSION: 3.6-v7.2.0 + PYPY_VERSION: 3.7-v7.3.5 # extras # extra-deps includes reppy, which does not support Python 3.9 diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml index ab7385118..955b9b449 100644 --- a/.github/workflows/tests-windows.yml +++ b/.github/workflows/tests-windows.yml @@ -8,12 +8,9 @@ jobs: fail-fast: false matrix: include: - - python-version: 3.6 - env: - TOXENV: windows-pinned - python-version: 3.7 env: - TOXENV: py + TOXENV: windows-pinned - python-version: 3.8 env: TOXENV: py diff --git a/README.rst b/README.rst index 6b563d638..b543a30f4 100644 --- a/README.rst +++ b/README.rst @@ -57,7 +57,7 @@ including a list of features. Requirements ============ -* Python 3.6+ +* Python 3.7+ * Works on Linux, Windows, macOS, BSD Install diff --git a/docs/contributing.rst b/docs/contributing.rst index 4d2580a6c..946bdc23e 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -232,15 +232,15 @@ To run a specific test (say ``tests/test_loader.py``) use: To run the tests on a specific :doc:`tox ` environment, use ``-e `` with an environment name from ``tox.ini``. For example, to run -the tests with Python 3.6 use:: +the tests with Python 3.7 use:: - tox -e py36 + tox -e py37 You can also specify a comma-separated list of environments, and use :ref:`tox’s parallel mode ` to run the tests on multiple environments in parallel:: - tox -e py36,py38 -p auto + tox -e py37,py38 -p auto To pass command-line options to :doc:`pytest `, add them after ``--`` in your call to :doc:`tox `. Using ``--`` overrides the @@ -250,9 +250,9 @@ default positional arguments (``scrapy tests``) after ``--`` as well:: tox -- scrapy tests -x # stop after first failure You can also use the `pytest-xdist`_ plugin. For example, to run all tests on -the Python 3.6 :doc:`tox ` environment using all your CPU cores:: +the Python 3.7 :doc:`tox ` environment using all your CPU cores:: - tox -e py36 -- scrapy tests -n auto + tox -e py37 -- scrapy tests -n auto To see coverage report install :doc:`coverage ` (``pip install coverage``) and run: diff --git a/docs/intro/install.rst b/docs/intro/install.rst index b8d3a16bc..1f01c068d 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -9,8 +9,8 @@ Installation guide Supported Python versions ========================= -Scrapy requires Python 3.6+, either the CPython implementation (default) or -the PyPy 7.2.0+ implementation (see :ref:`python:implementations`). +Scrapy requires Python 3.7+, either the CPython implementation (default) or +the PyPy 7.3.5+ implementation (see :ref:`python:implementations`). .. _intro-install-scrapy: diff --git a/docs/topics/items.rst b/docs/topics/items.rst index 7cd482d07..167014381 100644 --- a/docs/topics/items.rst +++ b/docs/topics/items.rst @@ -102,11 +102,6 @@ Additionally, ``dataclass`` items also allow to: * define custom field metadata through :func:`dataclasses.field`, which can be used to :ref:`customize serialization `. -They work natively in Python 3.7 or later, or using the `dataclasses -backport`_ in Python 3.6. - -.. _dataclasses backport: https://pypi.org/project/dataclasses/ - Example:: from dataclasses import dataclass diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 2513faae2..0925e6bb5 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -70,7 +70,7 @@ The advantage of using the :class:`ImagesPipeline` for image files is that you can configure some extra functions like generating thumbnails and filtering the images based on their size. -The Images Pipeline requires Pillow_ 4.0.0 or greater. It is used for +The Images Pipeline requires Pillow_ 7.1.0 or greater. It is used for thumbnailing and normalizing images to JPEG/RGB format. .. _Pillow: https://github.com/python-pillow/Pillow diff --git a/scrapy/__init__.py b/scrapy/__init__.py index 396f98219..86e584396 100644 --- a/scrapy/__init__.py +++ b/scrapy/__init__.py @@ -28,8 +28,8 @@ twisted_version = (_txv.major, _txv.minor, _txv.micro) # Check minimum required Python version -if sys.version_info < (3, 6): - print(f"Scrapy {__version__} requires Python 3.6+") +if sys.version_info < (3, 7): + print(f"Scrapy {__version__} requires Python 3.7+") sys.exit(1) diff --git a/scrapy/utils/py36.py b/scrapy/utils/py36.py deleted file mode 100644 index 653e2bbbb..000000000 --- a/scrapy/utils/py36.py +++ /dev/null @@ -1,11 +0,0 @@ -import warnings - -from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.asyncgen import collect_asyncgen # noqa: F401 - - -warnings.warn( - "Module `scrapy.utils.py36` is deprecated, please import from `scrapy.utils.asyncgen` instead.", - category=ScrapyDeprecationWarning, - stacklevel=2, -) diff --git a/setup.py b/setup.py index d86c0f285..ed197273f 100644 --- a/setup.py +++ b/setup.py @@ -19,35 +19,29 @@ def has_environment_marker_platform_impl_support(): install_requires = [ - 'Twisted>=17.9.0', - 'cryptography>=2.0', + 'Twisted>=18.9.0', + 'cryptography>=2.8', 'cssselect>=0.9.1', 'itemloaders>=1.0.1', 'parsel>=1.5.0', - 'pyOpenSSL>=16.2.0', + 'pyOpenSSL>=19.1.0', 'queuelib>=1.4.2', 'service_identity>=16.0.0', 'w3lib>=1.17.0', - 'zope.interface>=4.1.3', + 'zope.interface>=5.1.0', 'protego>=0.1.15', 'itemadapter>=0.1.0', 'setuptools', 'tldextract', + 'lxml>=4.3.0', ] extras_require = {} cpython_dependencies = [ - 'lxml>=3.5.0', 'PyDispatcher>=2.0.5', ] if has_environment_marker_platform_impl_support(): extras_require[':platform_python_implementation == "CPython"'] = cpython_dependencies extras_require[':platform_python_implementation == "PyPy"'] = [ - # Earlier lxml versions are affected by - # https://foss.heptapod.net/pypy/pypy/-/issues/2498, - # which was fixed in Cython 0.26, released on 2017-06-19, and used to - # generate the C headers of lxml release tarballs published since then, the - # first of which was: - 'lxml>=4.0.0', 'PyPyDispatcher>=2.1.0', ] else: @@ -84,7 +78,6 @@ setup( 'Operating System :: OS Independent', 'Programming Language :: Python', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', @@ -95,7 +88,7 @@ setup( 'Topic :: Software Development :: Libraries :: Application Frameworks', 'Topic :: Software Development :: Libraries :: Python Modules', ], - python_requires='>=3.6', + python_requires='>=3.7', install_requires=install_requires, extras_require=extras_require, ) diff --git a/tests/requirements.txt b/tests/requirements.txt index d2a8aae1b..d9373dfa8 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,14 +1,12 @@ # Tests requirements attrs -dataclasses; python_version == '3.6' pyftpdlib pytest pytest-cov==3.0.0 pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures -uvloop < 0.15.0; platform_system != "Windows" and python_version == '3.6' -uvloop; platform_system != "Windows" and python_version > '3.6' +uvloop; platform_system != "Windows" # optional for shell wrapper tests bpython diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 4b3964154..7dec5624a 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -3,7 +3,6 @@ import gc import operator import platform import unittest -from datetime import datetime from itertools import count from warnings import catch_warnings, filterwarnings @@ -224,12 +223,7 @@ class UtilsPythonTestCase(unittest.TestCase): elif platform.python_implementation() == 'PyPy': self.assertEqual(get_func_args(str.split, stripself=True), ['sep', 'maxsplit']) self.assertEqual(get_func_args(operator.itemgetter(2), stripself=True), ['obj']) - - build_date = datetime.strptime(platform.python_build()[1], '%b %d %Y') - if build_date >= datetime(2020, 4, 7): # PyPy 3.6-v7.3.1 - self.assertEqual(get_func_args(" ".join, stripself=True), ['iterable']) - else: - self.assertEqual(get_func_args(" ".join, stripself=True), ['list']) + self.assertEqual(get_func_args(" ".join, stripself=True), ['iterable']) def test_without_none_values(self): self.assertEqual(without_none_values([1, None, 3, 4]), [1, 3, 4]) diff --git a/tox.ini b/tox.ini index 6951b6d16..ab8a715c2 100644 --- a/tox.ini +++ b/tox.ini @@ -11,15 +11,13 @@ minversion = 1.7.0 deps = -rtests/requirements.txt # mitmproxy does not support PyPy - # mitmproxy does not support Windows when running Python < 3.7 # Python 3.9+ requires mitmproxy >= 5.3.0 # mitmproxy >= 5.3.0 requires h2 >= 4.0, Twisted 21.2 requires h2 < 4.0 #mitmproxy >= 5.3.0; python_version >= '3.9' and implementation_name != 'pypy' # The tests hang with mitmproxy 8.0.0: https://github.com/scrapy/scrapy/issues/5454 - mitmproxy >= 4.0.4, < 8; python_version >= '3.7' and python_version < '3.9' and implementation_name != 'pypy' - mitmproxy >= 4.0.4, < 5; python_version >= '3.6' and python_version < '3.7' and platform_system != 'Windows' and implementation_name != 'pypy' + mitmproxy >= 4.0.4, < 8; python_version < '3.9' and implementation_name != 'pypy' # newer markupsafe is incompatible with deps of old mitmproxy (which we get on Python 3.7 and lower) - markupsafe < 2.1.0; python_version >= '3.6' and python_version < '3.8' and implementation_name != 'pypy' + markupsafe < 2.1.0; python_version < '3.8' and implementation_name != 'pypy' # Extras botocore>=1.4.87 passenv = @@ -44,7 +42,6 @@ deps = types-pyOpenSSL==20.0.3 types-setuptools==57.0.0 commands = - pip install types-dataclasses # remove once py36 support is dropped mypy --show-error-codes {posargs: scrapy tests} [testenv:security] @@ -75,18 +72,19 @@ commands = [pinned] deps = - cryptography==2.0 + cryptography==2.8 cssselect==0.9.1 h2==3.0 itemadapter==0.1.0 parsel==1.5.0 Protego==0.1.15 - pyOpenSSL==16.2.0 + pyOpenSSL==19.1.0 queuelib==1.4.2 service_identity==16.0.0 - Twisted[http2]==17.9.0 + Twisted[http2]==18.9.0 w3lib==1.17.0 - zope.interface==4.1.3 + zope.interface==5.1.0 + lxml==4.3.0 -rtests/requirements.txt # mitmproxy 4.0.4+ requires upgrading some of the pinned dependencies @@ -95,7 +93,7 @@ deps = # Extras botocore==1.4.87 google-cloud-storage==1.29.0 - Pillow==4.0.0 + Pillow==7.1.0 setenv = _SCRAPY_PINNED=true install_command = @@ -104,7 +102,6 @@ install_command = [testenv:pinned] deps = {[pinned]deps} - lxml==3.5.0 PyDispatcher==2.0.5 install_command = {[pinned]install_command} setenv = @@ -114,9 +111,6 @@ setenv = basepython = python3 deps = {[pinned]deps} - # First lxml version that includes a Windows wheel for Python 3.6, so we do - # not need to build lxml from sources in a CI Windows job: - lxml==3.8.0 PyDispatcher==2.0.5 install_command = {[pinned]install_command} setenv = @@ -155,7 +149,6 @@ commands = basepython = {[testenv:pypy3]basepython} deps = {[pinned]deps} - lxml==4.0.0 PyPyDispatcher==2.1.0 commands = {[testenv:pypy3]commands} install_command = {[pinned]install_command} From 2e6721fd86e3bd00301f8cd3ceb4175b2f395017 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 9 Jun 2022 08:37:01 -0300 Subject: [PATCH 25/54] docs: Update minimal versions that Scrapy is tested against --- docs/intro/install.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/intro/install.rst b/docs/intro/install.rst index 1f01c068d..23c3af74b 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -54,9 +54,9 @@ Scrapy is written in pure Python and depends on a few key Python packages (among The minimal versions which Scrapy is tested against are: -* Twisted 14.0 -* lxml 3.4 -* pyOpenSSL 0.14 +* Twisted 18.9.0 +* lxml 4.3.0 +* pyOpenSSL 19.1.0 Scrapy may work with older versions of these packages but it is not guaranteed it will continue working From 6770d1ec62012fcfe8a36fdebeeb89cb5157c2df Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Thu, 9 Jun 2022 09:08:09 -0300 Subject: [PATCH 26/54] chore(tests): Remove validations for unsupported modules versions --- tests/test_downloadermiddleware.py | 20 +------------------- tests/test_utils_signal.py | 20 -------------------- tests/test_webclient.py | 5 ----- 3 files changed, 1 insertion(+), 44 deletions(-) diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index b538a0ed3..38be915f2 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -1,13 +1,11 @@ import asyncio -from unittest import mock, SkipTest +from unittest import mock from pytest import mark -from twisted import version as twisted_version from twisted.internet import defer from twisted.internet.defer import Deferred from twisted.trial.unittest import TestCase from twisted.python.failure import Failure -from twisted.python.versions import Version from scrapy.http import Request, Response from scrapy.spiders import Spider @@ -218,16 +216,6 @@ class MiddlewareUsingCoro(ManagerTestCase): """Middlewares using asyncio coroutines should work""" def test_asyncdef(self): - if ( - self.reactor_pytest == 'asyncio' - and twisted_version < Version('twisted', 18, 4, 0) - ): - raise SkipTest( - 'Due to https://twistedmatrix.com/trac/ticket/9390, this test ' - 'hangs when using AsyncIO and Twisted versions lower than ' - '18.4.0' - ) - resp = Response('http://example.com/index.html') class CoroMiddleware: @@ -248,12 +236,6 @@ class MiddlewareUsingCoro(ManagerTestCase): @mark.only_asyncio() def test_asyncdef_asyncio(self): - if twisted_version < Version('twisted', 18, 4, 0): - raise SkipTest( - 'Due to https://twistedmatrix.com/trac/ticket/9390, this test ' - 'hangs when using Twisted versions lower than 18.4.0' - ) - resp = Response('http://example.com/index.html') class CoroMiddleware: diff --git a/tests/test_utils_signal.py b/tests/test_utils_signal.py index ad7394232..a36e7bc97 100644 --- a/tests/test_utils_signal.py +++ b/tests/test_utils_signal.py @@ -1,13 +1,10 @@ import asyncio -from unittest import SkipTest from pydispatch import dispatcher from pytest import mark from testfixtures import LogCapture -from twisted import version as twisted_version from twisted.internet import defer, reactor from twisted.python.failure import Failure -from twisted.python.versions import Version from twisted.trial import unittest from scrapy.utils.signal import send_catch_log, send_catch_log_deferred @@ -81,16 +78,6 @@ class SendCatchLogDeferredAsyncDefTest(SendCatchLogDeferredTest): return "OK" def test_send_catch_log(self): - if ( - self.reactor_pytest == 'asyncio' - and twisted_version < Version('twisted', 18, 4, 0) - ): - raise SkipTest( - 'Due to https://twistedmatrix.com/trac/ticket/9390, this test ' - 'fails due to a timeout when using AsyncIO and Twisted ' - 'versions lower than 18.4.0' - ) - return super().test_send_catch_log() @@ -104,13 +91,6 @@ class SendCatchLogDeferredAsyncioTest(SendCatchLogDeferredTest): return await get_from_asyncio_queue("OK") def test_send_catch_log(self): - if twisted_version < Version('twisted', 18, 4, 0): - raise SkipTest( - 'Due to https://twistedmatrix.com/trac/ticket/9390, this test ' - 'fails due to a timeout when using Twisted versions lower ' - 'than 18.4.0' - ) - return super().test_send_catch_log() diff --git a/tests/test_webclient.py b/tests/test_webclient.py index a6d55cb38..0d5827339 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -4,10 +4,7 @@ Tests borrowed from the twisted.web.client tests. """ import os import shutil -import sys -from pkg_resources import parse_version -import cryptography import OpenSSL.SSL from twisted.trial import unittest from twisted.web import server, static, util, resource @@ -417,8 +414,6 @@ class WebClientCustomCiphersSSLTestCase(WebClientSSLTestCase): ).addCallback(self.assertEqual, to_bytes(s)) def testPayloadDisabledCipher(self): - if sys.implementation.name == "pypy" and parse_version(cryptography.__version__) <= parse_version("2.3.1"): - self.skipTest("This test expects a failure, but the code does work in PyPy with cryptography<=2.3.1") s = "0123456789" * 10 settings = Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': 'ECDHE-RSA-AES256-GCM-SHA384'}) client_context_factory = create_instance(ScrapyClientContextFactory, settings=settings, crawler=None) From c4c5c9f25841a783aab2c682125f3200d6c6e446 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Thu, 9 Jun 2022 10:00:44 -0300 Subject: [PATCH 27/54] docs: Remove minimal versions paragraphs --- docs/intro/install.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/intro/install.rst b/docs/intro/install.rst index 23c3af74b..c1fd6d522 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -52,12 +52,6 @@ Scrapy is written in pure Python and depends on a few key Python packages (among * `twisted`_, an asynchronous networking framework * `cryptography`_ and `pyOpenSSL`_, to deal with various network-level security needs -The minimal versions which Scrapy is tested against are: - -* Twisted 18.9.0 -* lxml 4.3.0 -* pyOpenSSL 19.1.0 - Scrapy may work with older versions of these packages but it is not guaranteed it will continue working because it’s not being tested against them. From 197aca2c94201f9944404f30fc4a002309cad99b Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Thu, 9 Jun 2022 10:11:49 -0300 Subject: [PATCH 28/54] docs: Remove leftover --- docs/intro/install.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/intro/install.rst b/docs/intro/install.rst index c1fd6d522..80a9c16d6 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -52,10 +52,6 @@ Scrapy is written in pure Python and depends on a few key Python packages (among * `twisted`_, an asynchronous networking framework * `cryptography`_ and `pyOpenSSL`_, to deal with various network-level security needs -Scrapy may work with older versions of these packages -but it is not guaranteed it will continue working -because it’s not being tested against them. - Some of these packages themselves depends on non-Python packages that might require additional installation steps depending on your platform. Please check :ref:`platform-specific guides below `. From ddfd192b704dddfefe2dd78345de239995a40159 Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Sat, 11 Jun 2022 23:51:34 +0430 Subject: [PATCH 29/54] add tests for multiple headers with same name --- tests/test_http_headers.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_http_headers.py b/tests/test_http_headers.py index 64ff7a73d..0c51fd701 100644 --- a/tests/test_http_headers.py +++ b/tests/test_http_headers.py @@ -38,6 +38,13 @@ class HeadersTest(unittest.TestCase): self.assertEqual(h.getlist('X-Forwarded-For'), [b'ip1', b'ip2']) assert h.getlist('X-Forwarded-For') is not hlist + def test_multivalue_for_one_header(self): + h = Headers((("a", "b"), ("a", "c"))) + self.assertEqual(h["a"], b"c") + self.assertEqual(h.get("a"), b"c") + self.assertEqual(h.getlist("a"), [b"b", b"c"]) + assert h.getlist("a") is not ["b", "c"] + def test_encode_utf8(self): h = Headers({'key': '\xa3'}, encoding='utf-8') key, val = dict(h).popitem() From 6a0bcf97cc6016cb966b92170709bc7518cf62c2 Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Sat, 11 Jun 2022 23:52:21 +0430 Subject: [PATCH 30/54] Merge values of multiple headers with same name (#5515) --- scrapy/http/headers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scrapy/http/headers.py b/scrapy/http/headers.py index 1a2b99b0a..a9471d721 100644 --- a/scrapy/http/headers.py +++ b/scrapy/http/headers.py @@ -1,6 +1,7 @@ from w3lib.http import headers_dict_to_raw from scrapy.utils.datatypes import CaselessDict from scrapy.utils.python import to_unicode +from collections.abc import Mapping class Headers(CaselessDict): @@ -10,6 +11,13 @@ class Headers(CaselessDict): self.encoding = encoding super().__init__(seq) + def update(self, seq): + seq = seq.items() if isinstance(seq, Mapping) else seq + iseq = {} + for k, v in seq: + iseq.setdefault(self.normkey(k), []).extend(self.normvalue(v)) + super().update(iseq) + def normkey(self, key): """Normalize key to bytes""" return self._tobytes(key.title()) @@ -86,4 +94,5 @@ class Headers(CaselessDict): def __copy__(self): return self.__class__(self) + copy = __copy__ From a135d6caf050f4b7b5af28d4cccc5d5ef51dbaf6 Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Mon, 13 Jun 2022 15:24:30 +0430 Subject: [PATCH 31/54] Move Mapping import line up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrián Chaves --- scrapy/http/headers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/http/headers.py b/scrapy/http/headers.py index a9471d721..9c03fe54f 100644 --- a/scrapy/http/headers.py +++ b/scrapy/http/headers.py @@ -1,7 +1,8 @@ +from collections.abc import Mapping + from w3lib.http import headers_dict_to_raw from scrapy.utils.datatypes import CaselessDict from scrapy.utils.python import to_unicode -from collections.abc import Mapping class Headers(CaselessDict): From 892c2a46554bdf80d49d3f28cc012c49cd1e19ca Mon Sep 17 00:00:00 2001 From: Mohammadtaher Abbasi Date: Mon, 13 Jun 2022 23:46:42 +0430 Subject: [PATCH 32/54] delete unnecessary test --- tests/test_http_headers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_http_headers.py b/tests/test_http_headers.py index 0c51fd701..1ca936247 100644 --- a/tests/test_http_headers.py +++ b/tests/test_http_headers.py @@ -43,7 +43,6 @@ class HeadersTest(unittest.TestCase): self.assertEqual(h["a"], b"c") self.assertEqual(h.get("a"), b"c") self.assertEqual(h.getlist("a"), [b"b", b"c"]) - assert h.getlist("a") is not ["b", "c"] def test_encode_utf8(self): h = Headers({'key': '\xa3'}, encoding='utf-8') From 9e265a2c1f6bccb551e8292785e09462594b8402 Mon Sep 17 00:00:00 2001 From: Kromitvs <74136201+Kromitvs@users.noreply.github.com> Date: Thu, 16 Jun 2022 19:52:19 +0100 Subject: [PATCH 33/54] Mind body to choose response class in cache, FTP and HTTP/1.0 (#4873) --- scrapy/core/downloader/handlers/ftp.py | 6 +-- scrapy/core/downloader/webclient.py | 2 +- scrapy/extensions/httpcache.py | 4 +- scrapy/responsetypes.py | 10 ++-- tests/test_downloader_handlers.py | 54 ++++++++++++++++++-- tests/test_downloadermiddleware_httpcache.py | 15 ++++++ tests/test_responsetypes.py | 2 + 7 files changed, 78 insertions(+), 15 deletions(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 3ef129587..a495874bd 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -102,11 +102,11 @@ class FTPDownloadHandler: def _build_response(self, result, request, protocol): self.result = result - respcls = responsetypes.from_args(url=request.url) protocol.close() - body = protocol.filename or protocol.body.read() headers = {"local filename": protocol.filename or '', "size": protocol.size} - return respcls(url=request.url, status=200, body=to_bytes(body), headers=headers) + body = to_bytes(protocol.filename or protocol.body.read()) + respcls = responsetypes.from_args(url=request.url, body=body) + return respcls(url=request.url, status=200, body=body, headers=headers) def _failed(self, result, request): message = result.getErrorMessage() diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index 06cb96489..7d048c1e4 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -112,7 +112,7 @@ class ScrapyHTTPClientFactory(ClientFactory): request.meta['download_latency'] = self.headers_time - self.start_time status = int(self.status) headers = Headers(self.response_headers) - respcls = responsetypes.from_args(headers=headers, url=self._url) + respcls = responsetypes.from_args(headers=headers, url=self._url, body=body) return respcls(url=self._url, status=status, headers=headers, body=body, protocol=to_unicode(self.version)) def _set_connection_attributes(self, request): diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index c71484cfa..843e14812 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -240,7 +240,7 @@ class DbmCacheStorage: status = data['status'] headers = Headers(data['headers']) body = data['body'] - respcls = responsetypes.from_args(headers=headers, url=url) + respcls = responsetypes.from_args(headers=headers, url=url, body=body) response = respcls(url=url, headers=headers, status=status, body=body) return response @@ -299,7 +299,7 @@ class FilesystemCacheStorage: url = metadata.get('response_url') status = metadata['status'] headers = Headers(headers_raw_to_dict(rawheaders)) - respcls = responsetypes.from_args(headers=headers, url=url) + respcls = responsetypes.from_args(headers=headers, url=url, body=body) response = respcls(url=url, headers=headers, status=status, body=body) return response diff --git a/scrapy/responsetypes.py b/scrapy/responsetypes.py index 6ed9f8b8f..3efd4d2fd 100644 --- a/scrapy/responsetypes.py +++ b/scrapy/responsetypes.py @@ -95,12 +95,14 @@ class ResponseTypes: chunk = to_bytes(chunk) if not binary_is_text(chunk): return self.from_mimetype('application/octet-stream') - elif b"" in chunk.lower(): + lowercase_chunk = chunk.lower() + if b"" in lowercase_chunk: return self.from_mimetype('text/html') - elif b"' in lowercase_chunk: + return self.from_mimetype('text/html') + return self.from_mimetype('text') def from_args(self, headers=None, url=None, filename=None, body=None): """Guess the most appropriate Response class based on diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 2bb53950d..72f52121e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -25,7 +25,7 @@ from scrapy.core.downloader.handlers.http10 import HTTP10DownloadHandler from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.core.downloader.handlers.s3 import S3DownloadHandler from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning -from scrapy.http import Headers, Request +from scrapy.http import Headers, HtmlResponse, Request from scrapy.http.response.text import TextResponse from scrapy.responsetypes import responsetypes from scrapy.spiders import Spider @@ -389,6 +389,23 @@ class HttpTestCase(unittest.TestCase): d.addCallback(self.assertEqual, b'159') return d + def _test_response_class(self, filename, body, response_class): + def _test(response): + self.assertEqual(type(response), response_class) + + request = Request(self.getURL(filename), body=body) + return self.download_request(request, Spider('foo')).addCallback(_test) + + def test_response_class_from_url(self): + return self._test_response_class('foo.html', b'', HtmlResponse) + + def test_response_class_from_body(self): + return self._test_response_class( + 'foo', + b"\n.", + HtmlResponse, + ) + class Http10TestCase(HttpTestCase): """HTTP 1.0 test case""" @@ -971,6 +988,12 @@ class BaseFTPTestCase(unittest.TestCase): password = "passwd" req_meta = {"ftp_user": username, "ftp_password": password} + test_files = ( + ('file.txt', b"I have the power!"), + ('file with spaces.txt', b"Moooooooooo power!"), + ('html-file-without-extension', b"\n."), + ) + def setUp(self): from twisted.protocols.ftp import FTPRealm, FTPFactory from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler @@ -981,8 +1004,8 @@ class BaseFTPTestCase(unittest.TestCase): userdir = os.path.join(self.directory, self.username) os.mkdir(userdir) fp = FilePath(userdir) - fp.child('file.txt').setContent(b"I have the power!") - fp.child('file with spaces.txt').setContent(b"Moooooooooo power!") + for filename, content in self.test_files: + fp.child(filename).setContent(content) # setup server realm = FTPRealm(anonymousRoot=self.directory, userHome=self.directory) @@ -1069,6 +1092,27 @@ class BaseFTPTestCase(unittest.TestCase): return self._add_test_callbacks(d, _test) + def _test_response_class(self, filename, response_class): + f, local_fname = tempfile.mkstemp() + local_fname = to_bytes(local_fname) + os.close(f) + meta = {} + meta.update(self.req_meta) + request = Request(url=f"ftp://127.0.0.1:{self.portNum}/{filename}", + meta=meta) + d = self.download_handler.download_request(request, None) + + def _test(r): + self.assertEqual(type(r), response_class) + os.remove(local_fname) + return self._add_test_callbacks(d, _test) + + def test_response_class_from_url(self): + return self._test_response_class('file.txt', TextResponse) + + def test_response_class_from_body(self): + return self._test_response_class('html-file-without-extension', HtmlResponse) + class FTPTestCase(BaseFTPTestCase): @@ -1104,8 +1148,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase): os.mkdir(self.directory) fp = FilePath(self.directory) - fp.child('file.txt').setContent(b"I have the power!") - fp.child('file with spaces.txt').setContent(b"Moooooooooo power!") + for filename, content in self.test_files: + fp.child(filename).setContent(content) # setup server for anonymous access realm = FTPRealm(anonymousRoot=self.directory) diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index 0c6dcf2aa..928c007f5 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -122,6 +122,21 @@ class DefaultStorageTest(_BaseTest): time.sleep(0.5) # give the chance to expire assert storage.retrieve_response(self.spider, self.request) + def test_storage_no_content_type_header(self): + """Test that the response body is used to get the right response class + even if there is no Content-Type header""" + with self._storage() as storage: + assert storage.retrieve_response(self.spider, self.request) is None + response = Response( + 'http://www.example.com', + body=b'\n.', + status=202, + ) + storage.store_response(self.spider, self.request, response) + cached_response = storage.retrieve_response(self.spider, self.request) + self.assertIsInstance(cached_response, HtmlResponse) + self.assertEqualResponse(response, cached_response) + class DbmStorageTest(DefaultStorageTest): diff --git a/tests/test_responsetypes.py b/tests/test_responsetypes.py index c07d3a99c..4b4095fb0 100644 --- a/tests/test_responsetypes.py +++ b/tests/test_responsetypes.py @@ -54,6 +54,8 @@ class ResponseTypesTest(unittest.TestCase): (b'\x03\x02\xdf\xdd\x23', Response), (b'Some plain text\ndata with tabs\t and null bytes\0', TextResponse), (b'Hello', HtmlResponse), + # https://codersblock.com/blog/the-smallest-valid-html5-page/ + (b'\n.', HtmlResponse), (b' Date: Thu, 16 Jun 2022 20:53:14 +0200 Subject: [PATCH 34/54] Update for Python 3.7+ --- docs/topics/exporters.rst | 10 +--------- scrapy/exporters.py | 2 +- scrapy/settings/__init__.py | 9 ++++----- tests/test_exporters.py | 5 ++--- tests/test_feedexport.py | 10 +++------- 5 files changed, 11 insertions(+), 25 deletions(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index 7580011ac..3c36ef002 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -205,7 +205,7 @@ BaseItemExporter ['field1', 'field2'] - - A dict [3]_ where keys are fields and values are output names:: + - A dict where keys are fields and values are output names:: {'field1': 'Field 1', 'field2': 'Field 2'} @@ -214,14 +214,6 @@ BaseItemExporter all their possible fields, exporters that do not support exporting a different subset of fields per item will only export the fields found in the first item exported. - .. [3] Dicts preserve insertion order since `Python 3.7`_ - (`CPython 3.6`_, `PyPy 2.5`_). If you are using an older version - of Python, use an OrderedDict_ to enforce a specific field order. - - .. _Python 3.7: https://docs.python.org/whatsnew/3.7.html - .. _CPython 3.6: https://docs.python.org/whatsnew/3.6.html#new-dict-implementation - .. _PyPy 2.5: https://morepypy.blogspot.com/2015/02/pypy-250-released.html - .. _OrderedDict: https://docs.python.org/library/collections.html#collections.OrderedDict .. attribute:: export_empty_fields diff --git a/scrapy/exporters.py b/scrapy/exporters.py index ad12f26d6..76cbe4d4b 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -2,13 +2,13 @@ Item Exporters are used to export/serialize items into different formats. """ -from collections import Mapping import csv import io import marshal import pickle import pprint import warnings +from collections.abc import Mapping from xml.sax.saxutils import XMLGenerator from itemadapter import is_item, ItemAdapter diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index 2bbe38481..6cacc63e1 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -1,6 +1,5 @@ import json import copy -from collections import OrderedDict from collections.abc import MutableMapping from importlib import import_module from pprint import pformat @@ -199,7 +198,7 @@ class BaseSettings(MutableMapping): return dict(value) def getdictorlist(self, name, default=None): - """Get a setting value as either an ``OrderedDict`` or a list. + """Get a setting value as either a :class:`dict` or a :class:`list`. If the setting is already a dict or a list, a copy of it will be returned. @@ -209,7 +208,7 @@ class BaseSettings(MutableMapping): For example, settings populated from the command line will return: - - ``OrdetedDict([('key1', 'value1'), ('key2', 'value2')])`` if set to + - ``{'key1': 'value1', 'key2': 'value2'}`` if set to ``'{"key1": "value1", "key2": "value2"}'`` - ``['one', 'two']`` if set to ``'["one", "two"]'`` or ``'one,two'`` @@ -222,10 +221,10 @@ class BaseSettings(MutableMapping): """ value = self.get(name, default) if value is None: - return OrderedDict() + return {} if isinstance(value, str): try: - return json.loads(value, object_pairs_hook=OrderedDict) + return json.loads(value) except ValueError: return value.split(',') return copy.deepcopy(value) diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 6ba7428f6..096cd3116 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -4,7 +4,6 @@ import marshal import pickle import tempfile import unittest -from collections import OrderedDict from io import BytesIO from datetime import datetime from warnings import catch_warnings, filterwarnings @@ -114,11 +113,11 @@ class BaseItemExporterTest(unittest.TestCase): self.assertEqual(name, 'John\xa3') ie = self._get_exporter( - fields_to_export=OrderedDict([('name', u'名稱')]) + fields_to_export={'name': '名稱'} ) self.assertEqual( list(ie._get_serialized_fields(self.i)), - [(u'名稱', u'John\xa3')] + [('名稱', 'John\xa3')] ) def test_field_custom_serializer(self): diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 83aabbdc7..9098e035d 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -11,7 +11,7 @@ import sys import tempfile import warnings from abc import ABC, abstractmethod -from collections import defaultdict, OrderedDict +from collections import defaultdict from contextlib import ExitStack from io import BytesIO from logging import getLogger @@ -998,9 +998,7 @@ class FeedExportTest(FeedExportTestBase): @defer.inlineCallbacks def test_export_items_field_names(self): items = [{'foo': 'bar'}] - header = OrderedDict(( - ("foo", "Foo"), - )) + header = {'foo': 'Foo'} rows = [{'Foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': header} yield self.assertExported(items, list(header.values()), rows, @@ -1023,9 +1021,7 @@ class FeedExportTest(FeedExportTestBase): @defer.inlineCallbacks def test_export_items_json_field_names(self): items = [{'foo': 'bar'}] - header = OrderedDict(( - ("foo", "Foo"), - )) + header = {'foo': 'Foo'} rows = [{'Foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': json.dumps(header)} yield self.assertExported(items, list(header.values()), rows, From 1b9ed22becf03311ec014dc9b7e0c09ce87b612c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 08:27:17 +0200 Subject: [PATCH 35/54] Remove Python < 3.7 leftover --- tests/test_feedexport.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 9098e035d..946c94bd4 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1004,8 +1004,6 @@ class FeedExportTest(FeedExportTestBase): yield self.assertExported(items, list(header.values()), rows, settings=settings) - @pytest.mark.skipif(sys.version_info < (3, 7), - reason='Only official in Python 3.7+') @defer.inlineCallbacks def test_export_items_dict_field_names(self): items = [{'foo': 'bar'}] From 24f382fa459434cccfa4c0a8884a48d09d75e243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 08:31:45 +0200 Subject: [PATCH 36/54] test_feedexport: remove ordered=False --- tests/test_feedexport.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 946c94bd4..4006b5957 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -657,8 +657,8 @@ class FeedExportTestBase(ABC, unittest.TestCase): return data @defer.inlineCallbacks - def assertExported(self, items, header, rows, settings=None, ordered=True): - yield self.assertExportedCsv(items, header, rows, settings, ordered) + def assertExported(self, items, header, rows, settings=None): + yield self.assertExportedCsv(items, header, rows, settings) yield self.assertExportedJsonLines(items, rows, settings) yield self.assertExportedXml(items, rows, settings) yield self.assertExportedPickle(items, rows, settings) @@ -719,7 +719,7 @@ class FeedExportTest(FeedExportTestBase): return content @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): + def assertExportedCsv(self, items, header, rows, settings=None): settings = settings or {} settings.update({ 'FEEDS': { @@ -730,10 +730,7 @@ class FeedExportTest(FeedExportTestBase): reader = csv.DictReader(to_unicode(data['csv']).splitlines()) got_rows = list(reader) - if ordered: - self.assertEqual(reader.fieldnames, header) - else: - self.assertEqual(set(reader.fieldnames), set(header)) + self.assertEqual(reader.fieldnames, header) self.assertEqual(rows, got_rows) @@ -886,7 +883,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam2', 'foo': 'bar2', 'baz': 'quux2'} ] header = self.MyItem.fields.keys() - yield self.assertExported(items, header, rows, ordered=False) + yield self.assertExported(items, header, rows) @defer.inlineCallbacks def test_export_no_items_not_store_empty(self): @@ -958,7 +955,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam4', 'foo': '', 'baz': ''}, ] rows_jl = [dict(row) for row in items] - yield self.assertExportedCsv(items, header, rows_csv, ordered=False) + yield self.assertExportedCsv(items, header, rows_csv) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -968,7 +965,7 @@ class FeedExportTest(FeedExportTestBase): header = ["foo"] rows = [{'foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': []} - yield self.assertExportedCsv(items, header, rows, ordered=False) + yield self.assertExportedCsv(items, header, rows) yield self.assertExportedJsonLines(items, rows, settings) @defer.inlineCallbacks @@ -1146,7 +1143,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam', 'foo': 'bar'} ] rows_jl = items - yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv, ordered=False) + yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -2065,7 +2062,7 @@ class BatchDeliveriesTest(FeedExportTestBase): self.assertEqual(expected_batch, got_batch) @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): + def assertExportedCsv(self, items, header, rows, settings=None): settings = settings or {} settings.update({ 'FEEDS': { From 3729c6d26698ae6b8a7ef297606a1c7630d82619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 08:33:34 +0200 Subject: [PATCH 37/54] Remove unused import and redundant import --- tests/test_feedexport.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 4006b5957..8ef221b70 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -21,7 +21,6 @@ from unittest import mock from urllib.parse import urljoin, quote from urllib.request import pathname2url -import pytest import lxml.etree from testfixtures import LogCapture from twisted.internet import defer @@ -731,7 +730,6 @@ class FeedExportTest(FeedExportTestBase): reader = csv.DictReader(to_unicode(data['csv']).splitlines()) got_rows = list(reader) self.assertEqual(reader.fieldnames, header) - self.assertEqual(rows, got_rows) @defer.inlineCallbacks @@ -1815,7 +1813,6 @@ class FeedPostProcessedExportsTest(FeedExportTestBase): @defer.inlineCallbacks def test_lzma_plugin_filters(self): - import sys if "PyPy" in sys.version: # https://foss.heptapod.net/pypy/pypy/-/issues/3527 raise unittest.SkipTest("lzma filters doesn't work in PyPy") From 6e878490e823a8105276b71ca5f6dc789465d330 Mon Sep 17 00:00:00 2001 From: Michel Ace Date: Fri, 17 Jun 2022 08:37:14 +0200 Subject: [PATCH 38/54] Support and prefer the .jsonl file extension (#4848) --- docs/intro/overview.rst | 4 ++-- docs/intro/tutorial.rst | 2 +- docs/topics/feed-exports.rst | 3 ++- docs/topics/item-pipeline.rst | 8 ++++---- scrapy/settings/default_settings.py | 1 + 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/intro/overview.rst b/docs/intro/overview.rst index f3d652621..cfa6bfa83 100644 --- a/docs/intro/overview.rst +++ b/docs/intro/overview.rst @@ -45,9 +45,9 @@ https://quotes.toscrape.com, following the pagination:: Put this in a text file, name it to something like ``quotes_spider.py`` and run the spider using the :command:`runspider` command:: - scrapy runspider quotes_spider.py -o quotes.jl + scrapy runspider quotes_spider.py -o quotes.jsonl -When this finishes you will have in the ``quotes.jl`` file a list of the +When this finishes you will have in the ``quotes.jsonl`` file a list of the quotes in JSON Lines format, containing text and author, looking like this:: {"author": "Jane Austen", "text": "\u201cThe person, be it gentleman or lady, who has not pleasure in a good novel, must be intolerably stupid.\u201d"} diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index cde1b1ef4..75928077e 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -482,7 +482,7 @@ to append new content to any existing file. However, appending to a JSON file makes the file contents invalid JSON. When appending to a file, consider using a different serialization format, such as `JSON Lines`_:: - scrapy crawl quotes -o quotes.jl + scrapy crawl quotes -o quotes.jsonl The `JSON Lines`_ format is useful because it's stream-like, you can easily append new records to it. It doesn't have the same problem of JSON when you run diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 9a13eb82f..398f80633 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -638,6 +638,7 @@ Default:: { 'json': 'scrapy.exporters.JsonItemExporter', 'jsonlines': 'scrapy.exporters.JsonLinesItemExporter', + 'jsonl': 'scrapy.exporters.JsonLinesItemExporter', 'jl': 'scrapy.exporters.JsonLinesItemExporter', 'csv': 'scrapy.exporters.CsvItemExporter', 'xml': 'scrapy.exporters.XmlItemExporter', @@ -763,7 +764,7 @@ source spider in the feed URI: #. Use ``%(spider_name)s`` in your feed URI:: - scrapy crawl -o "%(spider_name)s.jl" + scrapy crawl -o "%(spider_name)s.jsonl" .. _URIs: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 882ff5661..af294f52c 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -99,11 +99,11 @@ contain a price:: raise DropItem(f"Missing price in {item}") -Write items to a JSON file --------------------------- +Write items to a JSON lines file +-------------------------------- The following pipeline stores all scraped items (from all spiders) into a -single ``items.jl`` file, containing one item per line serialized in JSON +single ``items.jsonl`` file, containing one item per line serialized in JSON format:: import json @@ -113,7 +113,7 @@ format:: class JsonWriterPipeline: def open_spider(self, spider): - self.file = open('items.jl', 'w') + self.file = open('items.jsonl', 'w') def close_spider(self, spider): self.file.close() diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index f5a3efe69..ff86af125 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -154,6 +154,7 @@ FEED_EXPORTERS = {} FEED_EXPORTERS_BASE = { 'json': 'scrapy.exporters.JsonItemExporter', 'jsonlines': 'scrapy.exporters.JsonLinesItemExporter', + 'jsonl': 'scrapy.exporters.JsonLinesItemExporter', 'jl': 'scrapy.exporters.JsonLinesItemExporter', 'csv': 'scrapy.exporters.CsvItemExporter', 'xml': 'scrapy.exporters.XmlItemExporter', From 516e2d6ec0da77b8e0c01eb5188311b5fbeaa22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 08:55:45 +0200 Subject: [PATCH 39/54] Revert "test_feedexport: remove ordered=False" This reverts commit 24f382fa459434cccfa4c0a8884a48d09d75e243. --- tests/test_feedexport.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 8ef221b70..fe90501fb 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -656,8 +656,8 @@ class FeedExportTestBase(ABC, unittest.TestCase): return data @defer.inlineCallbacks - def assertExported(self, items, header, rows, settings=None): - yield self.assertExportedCsv(items, header, rows, settings) + def assertExported(self, items, header, rows, settings=None, ordered=True): + yield self.assertExportedCsv(items, header, rows, settings, ordered) yield self.assertExportedJsonLines(items, rows, settings) yield self.assertExportedXml(items, rows, settings) yield self.assertExportedPickle(items, rows, settings) @@ -718,7 +718,7 @@ class FeedExportTest(FeedExportTestBase): return content @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None): + def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): settings = settings or {} settings.update({ 'FEEDS': { @@ -729,7 +729,11 @@ class FeedExportTest(FeedExportTestBase): reader = csv.DictReader(to_unicode(data['csv']).splitlines()) got_rows = list(reader) - self.assertEqual(reader.fieldnames, header) + if ordered: + self.assertEqual(reader.fieldnames, header) + else: + self.assertEqual(set(reader.fieldnames), set(header)) + self.assertEqual(rows, got_rows) @defer.inlineCallbacks @@ -881,7 +885,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam2', 'foo': 'bar2', 'baz': 'quux2'} ] header = self.MyItem.fields.keys() - yield self.assertExported(items, header, rows) + yield self.assertExported(items, header, rows, ordered=False) @defer.inlineCallbacks def test_export_no_items_not_store_empty(self): @@ -953,7 +957,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam4', 'foo': '', 'baz': ''}, ] rows_jl = [dict(row) for row in items] - yield self.assertExportedCsv(items, header, rows_csv) + yield self.assertExportedCsv(items, header, rows_csv, ordered=False) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -963,7 +967,7 @@ class FeedExportTest(FeedExportTestBase): header = ["foo"] rows = [{'foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': []} - yield self.assertExportedCsv(items, header, rows) + yield self.assertExportedCsv(items, header, rows, ordered=False) yield self.assertExportedJsonLines(items, rows, settings) @defer.inlineCallbacks @@ -1141,7 +1145,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam', 'foo': 'bar'} ] rows_jl = items - yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv) + yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv, ordered=False) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -2059,7 +2063,7 @@ class BatchDeliveriesTest(FeedExportTestBase): self.assertEqual(expected_batch, got_batch) @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None): + def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): settings = settings or {} settings.update({ 'FEEDS': { From bc285f393ca8ff33ef715f98ef3367c973d23ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 09:00:39 +0200 Subject: [PATCH 40/54] Revert "Revert "test_feedexport: remove ordered=False"" This reverts commit 516e2d6ec0da77b8e0c01eb5188311b5fbeaa22e. --- tests/test_feedexport.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index fe90501fb..8ef221b70 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -656,8 +656,8 @@ class FeedExportTestBase(ABC, unittest.TestCase): return data @defer.inlineCallbacks - def assertExported(self, items, header, rows, settings=None, ordered=True): - yield self.assertExportedCsv(items, header, rows, settings, ordered) + def assertExported(self, items, header, rows, settings=None): + yield self.assertExportedCsv(items, header, rows, settings) yield self.assertExportedJsonLines(items, rows, settings) yield self.assertExportedXml(items, rows, settings) yield self.assertExportedPickle(items, rows, settings) @@ -718,7 +718,7 @@ class FeedExportTest(FeedExportTestBase): return content @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): + def assertExportedCsv(self, items, header, rows, settings=None): settings = settings or {} settings.update({ 'FEEDS': { @@ -729,11 +729,7 @@ class FeedExportTest(FeedExportTestBase): reader = csv.DictReader(to_unicode(data['csv']).splitlines()) got_rows = list(reader) - if ordered: - self.assertEqual(reader.fieldnames, header) - else: - self.assertEqual(set(reader.fieldnames), set(header)) - + self.assertEqual(reader.fieldnames, header) self.assertEqual(rows, got_rows) @defer.inlineCallbacks @@ -885,7 +881,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam2', 'foo': 'bar2', 'baz': 'quux2'} ] header = self.MyItem.fields.keys() - yield self.assertExported(items, header, rows, ordered=False) + yield self.assertExported(items, header, rows) @defer.inlineCallbacks def test_export_no_items_not_store_empty(self): @@ -957,7 +953,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam4', 'foo': '', 'baz': ''}, ] rows_jl = [dict(row) for row in items] - yield self.assertExportedCsv(items, header, rows_csv, ordered=False) + yield self.assertExportedCsv(items, header, rows_csv) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -967,7 +963,7 @@ class FeedExportTest(FeedExportTestBase): header = ["foo"] rows = [{'foo': 'bar'}] settings = {'FEED_EXPORT_FIELDS': []} - yield self.assertExportedCsv(items, header, rows, ordered=False) + yield self.assertExportedCsv(items, header, rows) yield self.assertExportedJsonLines(items, rows, settings) @defer.inlineCallbacks @@ -1145,7 +1141,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam', 'foo': 'bar'} ] rows_jl = items - yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv, ordered=False) + yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks @@ -2063,7 +2059,7 @@ class BatchDeliveriesTest(FeedExportTestBase): self.assertEqual(expected_batch, got_batch) @defer.inlineCallbacks - def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): + def assertExportedCsv(self, items, header, rows, settings=None): settings = settings or {} settings.update({ 'FEEDS': { From ec5cf3e9cea3c66aca4cf1aad576f33edca3ad1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 17 Jun 2022 09:10:18 +0200 Subject: [PATCH 41/54] test_feedexport: solve ordered comparison issues --- tests/test_feedexport.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 8ef221b70..ec48f8d4a 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -726,11 +726,9 @@ class FeedExportTest(FeedExportTestBase): }, }) data = yield self.exported_data(items, settings) - reader = csv.DictReader(to_unicode(data['csv']).splitlines()) - got_rows = list(reader) - self.assertEqual(reader.fieldnames, header) - self.assertEqual(rows, got_rows) + self.assertEqual(reader.fieldnames, list(header)) + self.assertEqual(rows, list(reader)) @defer.inlineCallbacks def assertExportedJsonLines(self, items, rows, settings=None): @@ -1141,7 +1139,7 @@ class FeedExportTest(FeedExportTestBase): {'egg': 'spam', 'foo': 'bar'} ] rows_jl = items - yield self.assertExportedCsv(items, ['egg', 'foo'], rows_csv) + yield self.assertExportedCsv(items, ['foo', 'egg'], rows_csv) yield self.assertExportedJsonLines(items, rows_jl) @defer.inlineCallbacks From d8223adfacc7e0ae684e5f9463474707bfcb008d Mon Sep 17 00:00:00 2001 From: Emanuele Date: Mon, 20 Jun 2022 11:54:05 +0200 Subject: [PATCH 42/54] =?UTF-8?q?Typo:=20cleanup=20(verb)=20=E2=86=92=20cl?= =?UTF-8?q?ean=20up=20(#5538)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.rst b/docs/README.rst index 0b7afa548..36dd5aea4 100644 --- a/docs/README.rst +++ b/docs/README.rst @@ -43,7 +43,7 @@ This command will fire up your default browser and open the main page of your Start over ---------- -To cleanup all generated documentation files and start from scratch run:: +To clean up all generated documentation files and start from scratch run:: make clean From 387326fad42c4709933851108f2370d3105b8dd1 Mon Sep 17 00:00:00 2001 From: Vardhaman <83634399+cyai@users.noreply.github.com> Date: Thu, 23 Jun 2022 14:40:49 +0530 Subject: [PATCH 43/54] MAINT: Updated f-string format Updated the code with the f-string method for better and cleaner understanding. --- scrapy/downloadermiddlewares/cookies.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index 3afa06077..c592acb57 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -104,8 +104,8 @@ class CookiesMiddleware: for key in ("name", "value", "path", "domain"): if cookie.get(key) is None: if key in ("name", "value"): - msg = "Invalid cookie found in request {}: {} ('{}' is missing)" - logger.warning(msg.format(request, cookie, key)) + msg = f"Invalid cookie found in request {request}: {cookie} ('{key}' is missing)" + logger.warning(msg) return continue if isinstance(cookie[key], (bool, float, int, str)): From c4c816624fc4fd5fbc1866c507b661e92704136a Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Thu, 30 Jun 2022 10:42:01 -0300 Subject: [PATCH 44/54] chore: Deprecate the `scrapy.downloadermiddlewares.decompression` module --- scrapy/downloadermiddlewares/decompression.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scrapy/downloadermiddlewares/decompression.py b/scrapy/downloadermiddlewares/decompression.py index 0fcf8fb8c..98f18a836 100644 --- a/scrapy/downloadermiddlewares/decompression.py +++ b/scrapy/downloadermiddlewares/decompression.py @@ -9,10 +9,19 @@ import tarfile import zipfile from io import BytesIO from tempfile import mktemp +import warnings +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.responsetypes import responsetypes +warnings.warn( + 'scrapy.downloadermiddlewares.decompression is deprecated', + ScrapyDeprecationWarning, + stacklevel=2, +) + + logger = logging.getLogger(__name__) From fe08a119d965b2291e44801901e15b58a1f959ad Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Thu, 30 Jun 2022 10:46:00 -0300 Subject: [PATCH 45/54] chore: import only used function --- scrapy/downloadermiddlewares/decompression.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/decompression.py b/scrapy/downloadermiddlewares/decompression.py index 98f18a836..e01e9cc76 100644 --- a/scrapy/downloadermiddlewares/decompression.py +++ b/scrapy/downloadermiddlewares/decompression.py @@ -9,13 +9,13 @@ import tarfile import zipfile from io import BytesIO from tempfile import mktemp -import warnings +from warnings import warn from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.responsetypes import responsetypes -warnings.warn( +warn( 'scrapy.downloadermiddlewares.decompression is deprecated', ScrapyDeprecationWarning, stacklevel=2, From 09c3a4ad082dd6fc431be65975292d2eba369ad6 Mon Sep 17 00:00:00 2001 From: Rotzbua Date: Tue, 12 Jul 2022 12:41:46 +0200 Subject: [PATCH 46/54] Fix doc: `scrapy.exporter` to `scrapy.exporters` --- docs/topics/exporters.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index 3c36ef002..9360ecf37 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -117,7 +117,7 @@ after your custom code. Example:: - from scrapy.exporter import XmlItemExporter + from scrapy.exporters import XmlItemExporter class ProductXmlExporter(XmlItemExporter): From 1c7ed4f2e59a94651d6d9d136cab78821cd3e80e Mon Sep 17 00:00:00 2001 From: Rotzbua Date: Thu, 6 Jan 2022 22:21:56 +0100 Subject: [PATCH 47/54] [doc] Remove incompatible web service project * Abandoned since 2017 * Not compatible with Python3 --- docs/index.rst | 4 ---- docs/topics/webservice.rst | 11 ----------- 2 files changed, 15 deletions(-) delete mode 100644 docs/topics/webservice.rst diff --git a/docs/index.rst b/docs/index.rst index 75e08f537..40c6cb485 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -130,7 +130,6 @@ Built-in services topics/stats topics/email topics/telnetconsole - topics/webservice :doc:`topics/logging` Learn how to use Python's builtin logging on Scrapy. @@ -144,9 +143,6 @@ Built-in services :doc:`topics/telnetconsole` Inspect a running crawler using a built-in Python console. -:doc:`topics/webservice` - Monitor and control a crawler using a web service. - Solving specific problems ========================= diff --git a/docs/topics/webservice.rst b/docs/topics/webservice.rst deleted file mode 100644 index 2c4052c04..000000000 --- a/docs/topics/webservice.rst +++ /dev/null @@ -1,11 +0,0 @@ -.. _topics-webservice: - -=========== -Web Service -=========== - -webservice has been moved into a separate project. - -It is hosted at: - - https://github.com/scrapy-plugins/scrapy-jsonrpc From 2f13f23d927900de0a89197ded0ee7aed387e351 Mon Sep 17 00:00:00 2001 From: Ikko Ashimine Date: Fri, 15 Jul 2022 18:16:23 +0900 Subject: [PATCH 48/54] Fix typo in sep-014.rst requets -> requests --- sep/sep-014.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sep/sep-014.rst b/sep/sep-014.rst index 0859e3f7c..2521aa0e5 100644 --- a/sep/sep-014.rst +++ b/sep/sep-014.rst @@ -590,11 +590,11 @@ Request Generator def generate_requests(self, response): """ - Extract and process new requets from response + Extract and process new requests from response """ requests = [] for ext in self._request_extractors: - requets.extend(ext.extract_requests(response)) + requests.extend(ext.extract_requests(response)) for proc in self._request_processors: requests = proc(requests) From 9b33b82a8b802c3906c2f1eaf1b88efee9b2fb09 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Sun, 17 Jul 2022 15:50:40 +0500 Subject: [PATCH 49/54] Fixed intersphinx references --- docs/conf.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 378b01804..3241295af 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -291,9 +291,9 @@ intersphinx_mapping = { 'pytest': ('https://docs.pytest.org/en/latest', None), '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), - 'twistedapi': ('https://twistedmatrix.com/documents/current/api', None), + 'tox': ('https://tox.wiki/en/latest/', None), + 'twisted': ('https://docs.twisted.org/en/stable/', None), + 'twistedapi': ('https://docs.twisted.org/en/stable/api/', None), 'w3lib': ('https://w3lib.readthedocs.io/en/latest', None), } intersphinx_disabled_reftypes = [] From 26c70318cb14806a07ee09d0283e9d5d306490e9 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Sun, 17 Jul 2022 16:47:20 +0500 Subject: [PATCH 50/54] make Scrapy testing suite more robust in environments where non-existing hosts are resolvable --- tests/__init__.py | 10 ++++++++++ tests/test_command_shell.py | 4 +++- tests/test_crawl.py | 4 ++++ tests/test_downloader_handlers.py | 5 ++++- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 12ce79fa9..bb62851dc 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -5,6 +5,7 @@ see https://docs.scrapy.org/en/latest/contributing.html#running-tests """ import os +import socket # ignore system-wide proxies for tests # which would send requests to a totally unsuspecting server @@ -25,6 +26,15 @@ tests_datadir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'sample_data') +# In some environments accessing a non-existing host doesn't raise an +# error. In such cases we're going to skip tests which rely on it. +try: + socket.getaddrinfo('non-existing-host', 80) + NON_EXISTING_RESOLVABLE = True +except socket.gaierror: + NON_EXISTING_RESOLVABLE = False + + def get_testdata(*paths): """Return test data""" path = os.path.join(tests_datadir, *paths) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 16c9559b5..33189e9be 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -6,7 +6,7 @@ from twisted.internet import defer from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest -from tests import tests_datadir +from tests import tests_datadir, NON_EXISTING_RESOLVABLE class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -109,6 +109,8 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @defer.inlineCallbacks def test_dns_failures(self): + if NON_EXISTING_RESOLVABLE: + raise unittest.SkipTest("Non-existing hosts are resolvable") url = 'www.somedomainthatdoesntexi.st' errcode, out, err = yield self.execute([url, '-c', 'item'], check_code=False) self.assertEqual(errcode, 1, out or err) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 7bda3bef2..f9ffcd6bb 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -3,6 +3,7 @@ import logging from ipaddress import IPv4Address from socket import gethostbyname from urllib.parse import urlparse +import unittest from pytest import mark from testfixtures import LogCapture @@ -17,6 +18,7 @@ from scrapy.exceptions import StopDownload from scrapy.http import Request from scrapy.http.response import Response from scrapy.utils.python import to_unicode +from tests import NON_EXISTING_RESOLVABLE from tests.mockserver import MockServer from tests.spiders import ( AsyncDefAsyncioGenComplexSpider, @@ -137,6 +139,8 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_retry_dns_error(self): + if NON_EXISTING_RESOLVABLE: + raise unittest.SkipTest("Non-existing hosts are resolvable") crawler = self.runner.create_crawler(SimpleSpider) with LogCapture() as log: # try to fetch the homepage of a non-existent domain diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 72f52121e..883960084 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -4,7 +4,7 @@ import shutil import sys import tempfile from typing import Optional, Type -from unittest import mock +from unittest import mock, SkipTest from testfixtures import LogCapture from twisted.cred import checkers, credentials, portal @@ -32,6 +32,7 @@ from scrapy.spiders import Spider from scrapy.utils.misc import create_instance from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler, skip_if_no_boto +from tests import NON_EXISTING_RESOLVABLE from tests.mockserver import ( Echo, ForeverTakingResource, @@ -791,6 +792,8 @@ class Http11ProxyTestCase(HttpProxyTestCase): @defer.inlineCallbacks def test_download_with_proxy_https_timeout(self): """ Test TunnelingTCP4ClientEndpoint """ + if NON_EXISTING_RESOLVABLE: + raise SkipTest("Non-existing hosts are resolvable") http_proxy = self.getURL('') domain = 'https://no-such-domain.nosuch' request = Request( From e248360e6e3dbb36fab185caf131707195fa6a26 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Mon, 18 Jul 2022 23:49:08 +0500 Subject: [PATCH 51/54] remove compatibility code from tests for the case dataclasses module is not available It was Python 3.6 compat code, and Python 3.6 support is dropped. --- tests/test_exporters.py | 32 ++++++++++++++--------------- tests/test_loader.py | 23 +++++++-------------- tests/test_pipeline_files.py | 37 ++++++++++++---------------------- tests/test_pipeline_images.py | 38 ++++++++++++----------------------- tests/test_utils_serialize.py | 18 +++++++---------- 5 files changed, 55 insertions(+), 93 deletions(-) diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 096cd3116..69ac928c3 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -4,6 +4,7 @@ import marshal import pickle import tempfile import unittest +import dataclasses from io import BytesIO from datetime import datetime from warnings import catch_warnings, filterwarnings @@ -21,31 +22,30 @@ from scrapy.exporters import ( ) +def custom_serializer(value): + return str(int(value) + 2) + + class TestItem(Item): name = Field() age = Field() -def custom_serializer(value): - return str(int(value) + 2) - - class CustomFieldItem(Item): name = Field() age = Field(serializer=custom_serializer) -try: - from dataclasses import make_dataclass, field -except ImportError: - TestDataClass = None - CustomFieldDataclass = None -else: - TestDataClass = make_dataclass("TestDataClass", [("name", str), ("age", int)]) - CustomFieldDataclass = make_dataclass( - "CustomFieldDataclass", - [("name", str), ("age", int, field(metadata={"serializer": custom_serializer}))] - ) +@dataclasses.dataclass +class TestDataClass: + name: str + age: int + + +@dataclasses.dataclass +class CustomFieldDataclass: + name: str + age: int = dataclasses.field(metadata={"serializer": custom_serializer}) class BaseItemExporterTest(unittest.TestCase): @@ -54,8 +54,6 @@ class BaseItemExporterTest(unittest.TestCase): custom_field_item_class = CustomFieldItem def setUp(self): - if self.item_class is None: - raise unittest.SkipTest("item class is None") self.i = self.item_class(name='John\xa3', age='22') self.output = BytesIO() self.ie = self._get_exporter() diff --git a/tests/test_loader.py b/tests/test_loader.py index f7ab1f236..c0937b349 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -1,4 +1,5 @@ import unittest +import dataclasses import attr from itemadapter import ItemAdapter @@ -10,13 +11,6 @@ from scrapy.loader import ItemLoader from scrapy.selector import Selector -try: - from dataclasses import make_dataclass, field as dataclass_field -except ImportError: - make_dataclass = None - dataclass_field = None - - # test items class NameItem(Item): name = Field() @@ -41,6 +35,11 @@ class AttrsNameItem: name = attr.ib(default="") +@dataclasses.dataclass +class TestDataClass: + name: list = dataclasses.field(default_factory=list) + + # test item loaders class NameItemLoader(ItemLoader): default_item_class = TestItem @@ -187,16 +186,8 @@ class InitializationFromAttrsItemTest(InitializationTestMixin, unittest.TestCase item_class = AttrsNameItem -@unittest.skipIf(not make_dataclass, "dataclasses module is not available") class InitializationFromDataClassTest(InitializationTestMixin, unittest.TestCase): - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if make_dataclass: - self.item_class = make_dataclass( - "TestDataClass", - [("name", list, dataclass_field(default_factory=list))], - ) + item_class = TestDataClass class BaseNoInputReprocessingLoader(ItemLoader): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4228173ed..5d381c018 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -7,6 +7,7 @@ from shutil import rmtree from tempfile import mkdtemp from unittest import mock, skipIf from urllib.parse import urlparse +import dataclasses import attr from itemadapter import ItemAdapter @@ -32,13 +33,6 @@ from scrapy.utils.test import ( ) -try: - from dataclasses import make_dataclass, field as dataclass_field -except ImportError: - make_dataclass = None - dataclass_field = None - - def _mocked_download_func(request, info): response = request.meta.get('response') return response() if callable(response) else response @@ -226,24 +220,19 @@ class FilesPipelineTestCaseFieldsItem(FilesPipelineTestCaseFieldsMixin, unittest item_class = FilesPipelineTestItem -@skipIf(not make_dataclass, "dataclasses module is not available") -class FilesPipelineTestCaseFieldsDataClass(FilesPipelineTestCaseFieldsMixin, unittest.TestCase): +@dataclasses.dataclass +class FilesPipelineTestDataClass: + name: str + # default fields + file_urls: list = dataclasses.field(default_factory=list) + files: list = dataclasses.field(default_factory=list) + # overridden fields + custom_file_urls: list = dataclasses.field(default_factory=list) + custom_files: list = dataclasses.field(default_factory=list) - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if make_dataclass: - self.item_class = make_dataclass( - "FilesPipelineTestDataClass", - [ - ("name", str), - # default fields - ("file_urls", list, dataclass_field(default_factory=list)), - ("files", list, dataclass_field(default_factory=list)), - # overridden fields - ("custom_file_urls", list, dataclass_field(default_factory=list)), - ("custom_files", list, dataclass_field(default_factory=list)), - ], - ) + +class FilesPipelineTestCaseFieldsDataClass(FilesPipelineTestCaseFieldsMixin, unittest.TestCase): + item_class = FilesPipelineTestDataClass @attr.s diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index dd94d296b..e6f5bea21 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -4,6 +4,7 @@ import random from shutil import rmtree from tempfile import mkdtemp from unittest import skipIf +import dataclasses import attr from itemadapter import ItemAdapter @@ -16,13 +17,6 @@ from scrapy.settings import Settings from scrapy.utils.python import to_bytes -try: - from dataclasses import make_dataclass, field as dataclass_field -except ImportError: - make_dataclass = None - dataclass_field = None - - try: from PIL import Image except ImportError: @@ -203,25 +197,19 @@ class ImagesPipelineTestCaseFieldsItem(ImagesPipelineTestCaseFieldsMixin, unitte item_class = ImagesPipelineTestItem -@skipIf(not make_dataclass, "dataclasses module is not available") -class ImagesPipelineTestCaseFieldsDataClass(ImagesPipelineTestCaseFieldsMixin, unittest.TestCase): - item_class = None +@dataclasses.dataclass +class ImagesPipelineTestDataClass: + name: str + # default fields + image_urls: list = dataclasses.field(default_factory=list) + images: list = dataclasses.field(default_factory=list) + # overridden fields + custom_image_urls: list = dataclasses.field(default_factory=list) + custom_images: list = dataclasses.field(default_factory=list) - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if make_dataclass: - self.item_class = make_dataclass( - "FilesPipelineTestDataClass", - [ - ("name", str), - # default fields - ("image_urls", list, dataclass_field(default_factory=list)), - ("images", list, dataclass_field(default_factory=list)), - # overridden fields - ("custom_image_urls", list, dataclass_field(default_factory=list)), - ("custom_images", list, dataclass_field(default_factory=list)), - ], - ) + +class ImagesPipelineTestCaseFieldsDataClass(ImagesPipelineTestCaseFieldsMixin, unittest.TestCase): + item_class = ImagesPipelineTestDataClass @attr.s diff --git a/tests/test_utils_serialize.py b/tests/test_utils_serialize.py index daf022aee..a51de1877 100644 --- a/tests/test_utils_serialize.py +++ b/tests/test_utils_serialize.py @@ -1,6 +1,7 @@ import datetime import json import unittest +import dataclasses from decimal import Decimal import attr @@ -10,12 +11,6 @@ from scrapy.http import Request, Response from scrapy.utils.serialize import ScrapyJSONEncoder -try: - from dataclasses import make_dataclass -except ImportError: - make_dataclass = None - - class JsonEncoderTestCase(unittest.TestCase): def setUp(self): @@ -56,12 +51,13 @@ class JsonEncoderTestCase(unittest.TestCase): self.assertIn(r.url, rs) self.assertIn(str(r.status), rs) - @unittest.skipIf(not make_dataclass, "No dataclass support") def test_encode_dataclass_item(self): - TestDataClass = make_dataclass( - "TestDataClass", - [("name", str), ("url", str), ("price", int)], - ) + @dataclasses.dataclass + class TestDataClass: + name: str + url: str + price: int + item = TestDataClass(name="Product", url="http://product.org", price=1) encoded = self.encoder.encode(item) self.assertEqual( From 105468959363ee50b597038ac30fd32d3ea1b1f2 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Mon, 18 Jul 2022 23:53:30 +0500 Subject: [PATCH 52/54] remove unused imports thanks flake8! --- tests/test_pipeline_files.py | 2 +- tests/test_pipeline_images.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 5d381c018..d641e7a43 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -5,7 +5,7 @@ from datetime import datetime from io import BytesIO from shutil import rmtree from tempfile import mkdtemp -from unittest import mock, skipIf +from unittest import mock from urllib.parse import urlparse import dataclasses diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index e6f5bea21..0082e7a4e 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -3,7 +3,6 @@ import io import random from shutil import rmtree from tempfile import mkdtemp -from unittest import skipIf import dataclasses import attr From b103664bf45b079e5488b13a0737866de1b7dc50 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Tue, 19 Jul 2022 20:39:26 +0500 Subject: [PATCH 53/54] Address 2/3 of warnings from tests (#5561) --- pytest.ini | 3 + tests/test_contracts.py | 4 +- tests/test_crawl.py | 100 +++---- tests/test_crawler.py | 36 ++- tests/test_downloadermiddleware_httpauth.py | 14 +- tests/test_downloadermiddleware_httpproxy.py | 12 +- tests/test_downloadermiddleware_stats.py | 7 +- tests/test_dupefilters.py | 27 +- tests/test_engine.py | 228 ++++++++-------- tests/test_engine_stop_download_bytes.py | 32 ++- tests/test_engine_stop_download_headers.py | 32 ++- tests/test_feedexport.py | 269 +++++++------------ tests/test_logformatter.py | 6 +- tests/test_pipeline_crawl.py | 12 +- tests/test_request_attribute_binding.py | 21 +- tests/test_request_cb_kwargs.py | 5 +- tests/test_scheduler.py | 3 +- tests/test_scheduler_base.py | 15 +- tests/test_spiderloader/__init__.py | 5 +- tests/test_utils_project.py | 27 +- tests/test_utils_request.py | 13 +- tests/test_utils_response.py | 21 +- 22 files changed, 424 insertions(+), 468 deletions(-) diff --git a/pytest.ini b/pytest.ini index ae2ed2029..af0f2fb6e 100644 --- a/pytest.ini +++ b/pytest.ini @@ -21,3 +21,6 @@ addopts = markers = only_asyncio: marks tests as only enabled when --reactor=asyncio is passed only_not_asyncio: marks tests as only enabled when --reactor=asyncio is not passed +filterwarnings = + ignore:scrapy.downloadermiddlewares.decompression is deprecated + ignore:Module scrapy.utils.reqser is deprecated diff --git a/tests/test_contracts.py b/tests/test_contracts.py index d0f4a68c2..136056f50 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -5,11 +5,11 @@ from twisted.python import failure from twisted.trial import unittest from scrapy import FormRequest -from scrapy.crawler import CrawlerRunner from scrapy.spidermiddlewares.httperror import HttpError from scrapy.spiders import Spider from scrapy.http import Request from scrapy.item import Item, Field +from scrapy.utils.test import get_crawler from scrapy.contracts import ContractsManager, Contract from scrapy.contracts.default import ( UrlContract, @@ -398,7 +398,7 @@ class ContractsManagerTest(unittest.TestCase): TestSameUrlSpider.parse_first.__doc__ = contract_doc TestSameUrlSpider.parse_second.__doc__ = contract_doc - crawler = CrawlerRunner().create_crawler(TestSameUrlSpider) + crawler = get_crawler(TestSameUrlSpider) yield crawler.crawl() self.assertEqual(crawler.spider.visited, 2) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index f9ffcd6bb..59c271868 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -18,6 +18,7 @@ from scrapy.exceptions import StopDownload from scrapy.http import Request from scrapy.http.response import Response from scrapy.utils.python import to_unicode +from scrapy.utils.test import get_crawler from tests import NON_EXISTING_RESOLVABLE from tests.mockserver import MockServer from tests.spiders import ( @@ -49,14 +50,13 @@ class CrawlTestCase(TestCase): def setUp(self): self.mockserver = MockServer() self.mockserver.__enter__() - self.runner = CrawlerRunner() def tearDown(self): self.mockserver.__exit__(None, None, None) @defer.inlineCallbacks def test_follow_all(self): - crawler = self.runner.create_crawler(FollowAllSpider) + crawler = get_crawler(FollowAllSpider) yield crawler.crawl(mockserver=self.mockserver) self.assertEqual(len(crawler.spider.urls_visited), 11) # 10 + start_url @@ -79,7 +79,7 @@ class CrawlTestCase(TestCase): settings = {"DOWNLOAD_DELAY": delay, 'RANDOMIZE_DOWNLOAD_DELAY': randomize} - crawler = CrawlerRunner(settings).create_crawler(FollowAllSpider) + crawler = get_crawler(FollowAllSpider, settings) yield crawler.crawl(**crawl_kwargs) times = crawler.spider.times total_time = times[-1] - times[0] @@ -92,7 +92,7 @@ class CrawlTestCase(TestCase): # of ``total`` and ``delay`` values that are too small for the test # code above to have any meaning. settings["DOWNLOAD_DELAY"] = 0 - crawler = CrawlerRunner(settings).create_crawler(FollowAllSpider) + crawler = get_crawler(FollowAllSpider, settings) yield crawler.crawl(**crawl_kwargs) times = crawler.spider.times total_time = times[-1] - times[0] @@ -102,7 +102,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_timeout_success(self): - crawler = self.runner.create_crawler(DelaySpider) + crawler = get_crawler(DelaySpider) yield crawler.crawl(n=0.5, mockserver=self.mockserver) self.assertTrue(crawler.spider.t1 > 0) self.assertTrue(crawler.spider.t2 > 0) @@ -110,7 +110,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_timeout_failure(self): - crawler = CrawlerRunner({"DOWNLOAD_TIMEOUT": 0.35}).create_crawler(DelaySpider) + crawler = get_crawler(DelaySpider, {"DOWNLOAD_TIMEOUT": 0.35}) yield crawler.crawl(n=0.5, mockserver=self.mockserver) self.assertTrue(crawler.spider.t1 > 0) self.assertTrue(crawler.spider.t2 == 0) @@ -125,14 +125,14 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_retry_503(self): - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: yield crawler.crawl(self.mockserver.url("/status?n=503"), mockserver=self.mockserver) self._assert_retried(log) @defer.inlineCallbacks def test_retry_conn_failed(self): - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: yield crawler.crawl("http://localhost:65432/status?n=503", mockserver=self.mockserver) self._assert_retried(log) @@ -141,7 +141,7 @@ class CrawlTestCase(TestCase): def test_retry_dns_error(self): if NON_EXISTING_RESOLVABLE: raise unittest.SkipTest("Non-existing hosts are resolvable") - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: # try to fetch the homepage of a non-existent domain yield crawler.crawl("http://dns.resolution.invalid./", mockserver=self.mockserver) @@ -150,7 +150,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_start_requests_bug_before_yield(self): with LogCapture('scrapy', level=logging.ERROR) as log: - crawler = self.runner.create_crawler(BrokenStartRequestsSpider) + crawler = get_crawler(BrokenStartRequestsSpider) yield crawler.crawl(fail_before_yield=1, mockserver=self.mockserver) self.assertEqual(len(log.records), 1) @@ -161,7 +161,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_start_requests_bug_yielding(self): with LogCapture('scrapy', level=logging.ERROR) as log: - crawler = self.runner.create_crawler(BrokenStartRequestsSpider) + crawler = get_crawler(BrokenStartRequestsSpider) yield crawler.crawl(fail_yielding=1, mockserver=self.mockserver) self.assertEqual(len(log.records), 1) @@ -172,7 +172,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_start_requests_lazyness(self): settings = {"CONCURRENT_REQUESTS": 1} - crawler = CrawlerRunner(settings).create_crawler(BrokenStartRequestsSpider) + crawler = get_crawler(BrokenStartRequestsSpider, settings) yield crawler.crawl(mockserver=self.mockserver) self.assertTrue( crawler.spider.seedsseen.index(None) < crawler.spider.seedsseen.index(99), @@ -181,7 +181,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_start_requests_dupes(self): settings = {"CONCURRENT_REQUESTS": 1} - crawler = CrawlerRunner(settings).create_crawler(DuplicateStartRequestsSpider) + crawler = get_crawler(DuplicateStartRequestsSpider, settings) yield crawler.crawl(dont_filter=True, distinct_urls=2, dupe_factor=3, mockserver=self.mockserver) self.assertEqual(crawler.spider.visited, 6) @@ -210,7 +210,7 @@ Connection: close foo body with multiples lines '''}) - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: yield crawler.crawl(self.mockserver.url(f"/raw?{query}"), mockserver=self.mockserver) self.assertEqual(str(log).count("Got response 200"), 1) @@ -218,7 +218,7 @@ with multiples lines @defer.inlineCallbacks def test_retry_conn_lost(self): # connection lost after receiving data - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: yield crawler.crawl(self.mockserver.url("/drop?abort=0"), mockserver=self.mockserver) self._assert_retried(log) @@ -226,7 +226,7 @@ with multiples lines @defer.inlineCallbacks def test_retry_conn_aborted(self): # connection lost before receiving data - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) with LogCapture() as log: yield crawler.crawl(self.mockserver.url("/drop?abort=1"), mockserver=self.mockserver) self._assert_retried(log) @@ -245,7 +245,7 @@ with multiples lines req0.meta['next'] = req1 req1.meta['next'] = req2 req2.meta['next'] = req3 - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) yield crawler.crawl(seed=req0, mockserver=self.mockserver) # basic asserts in case of weird communication errors self.assertIn('responses', crawler.spider.meta) @@ -271,7 +271,7 @@ with multiples lines def cb(response): est.append(get_engine_status(crawler.engine)) - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) yield crawler.crawl(seed=self.mockserver.url('/'), callback_func=cb, mockserver=self.mockserver) self.assertEqual(len(est), 1, est) s = dict(est[0]) @@ -286,7 +286,7 @@ with multiples lines def cb(response): est.append(format_engine_status(crawler.engine)) - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) yield crawler.crawl(seed=self.mockserver.url('/'), callback_func=cb, mockserver=self.mockserver) self.assertEqual(len(est), 1, est) est = est[0].split("\n")[2:-2] # remove header & footer @@ -317,7 +317,7 @@ with multiples lines def start_requests(self): raise TestError - crawler = self.runner.create_crawler(FaultySpider) + crawler = get_crawler(FaultySpider) yield self.assertFailure(crawler.crawl(mockserver=self.mockserver), TestError) self.assertFalse(crawler.crawling) @@ -328,26 +328,28 @@ with multiples lines "tests.pipelines.ZeroDivisionErrorPipeline": 300, } } - crawler = CrawlerRunner(settings).create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider, settings) yield self.assertFailure( - self.runner.crawl(crawler, self.mockserver.url("/status?n=200"), mockserver=self.mockserver), + crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver), ZeroDivisionError) self.assertFalse(crawler.crawling) @defer.inlineCallbacks def test_crawlerrunner_accepts_crawler(self): - crawler = self.runner.create_crawler(SimpleSpider) + crawler = get_crawler(SimpleSpider) + runner = CrawlerRunner() with LogCapture() as log: - yield self.runner.crawl(crawler, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) + yield runner.crawl(crawler, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) self.assertIn("Got response 200", str(log)) @defer.inlineCallbacks def test_crawl_multiple(self): - self.runner.crawl(SimpleSpider, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) - self.runner.crawl(SimpleSpider, self.mockserver.url("/status?n=503"), mockserver=self.mockserver) + runner = CrawlerRunner({'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'}) + runner.crawl(SimpleSpider, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) + runner.crawl(SimpleSpider, self.mockserver.url("/status?n=503"), mockserver=self.mockserver) with LogCapture() as log: - yield self.runner.join() + yield runner.join() self._assert_retried(log) self.assertIn("Got response 200", str(log)) @@ -358,7 +360,6 @@ class CrawlSpiderTestCase(TestCase): def setUp(self): self.mockserver = MockServer() self.mockserver.__enter__() - self.runner = CrawlerRunner() def tearDown(self): self.mockserver.__exit__(None, None, None) @@ -370,7 +371,7 @@ class CrawlSpiderTestCase(TestCase): def _on_item_scraped(item): items.append(item) - crawler = self.runner.create_crawler(spider_cls) + crawler = get_crawler(spider_cls) crawler.signals.connect(_on_item_scraped, signals.item_scraped) with LogCapture() as log: yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver) @@ -378,10 +379,9 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_crawlspider_with_parse(self): - self.runner.crawl(CrawlSpiderWithParseMethod, mockserver=self.mockserver) - + crawler = get_crawler(CrawlSpiderWithParseMethod) with LogCapture() as log: - yield self.runner.join() + yield crawler.crawl(mockserver=self.mockserver) self.assertIn("[parse] status 200 (foo: None)", str(log)) self.assertIn("[parse] status 201 (foo: None)", str(log)) @@ -389,10 +389,9 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_crawlspider_with_errback(self): - self.runner.crawl(CrawlSpiderWithErrback, mockserver=self.mockserver) - + crawler = get_crawler(CrawlSpiderWithErrback) with LogCapture() as log: - yield self.runner.join() + yield crawler.crawl(mockserver=self.mockserver) self.assertIn("[parse] status 200 (foo: None)", str(log)) self.assertIn("[parse] status 201 (foo: None)", str(log)) @@ -403,18 +402,19 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_async_def_parse(self): - self.runner.crawl(AsyncDefSpider, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) + crawler = get_crawler(AsyncDefSpider) with LogCapture() as log: - yield self.runner.join() + yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver) self.assertIn("Got response 200", str(log)) @mark.only_asyncio() @defer.inlineCallbacks def test_async_def_asyncio_parse(self): - runner = CrawlerRunner({"TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor"}) - runner.crawl(AsyncDefAsyncioSpider, self.mockserver.url("/status?n=200"), mockserver=self.mockserver) + crawler = get_crawler(AsyncDefAsyncioSpider, { + "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor" + }) with LogCapture() as log: - yield runner.join() + yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver) self.assertIn("Got response 200", str(log)) @mark.only_asyncio() @@ -433,7 +433,7 @@ class CrawlSpiderTestCase(TestCase): def _on_item_scraped(item): items.append(item) - crawler = self.runner.create_crawler(AsyncDefAsyncioReturnSingleElementSpider) + crawler = get_crawler(AsyncDefAsyncioReturnSingleElementSpider) crawler.signals.connect(_on_item_scraped, signals.item_scraped) with LogCapture() as log: yield crawler.crawl(self.mockserver.url("/status?n=200"), mockserver=self.mockserver) @@ -479,14 +479,14 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_response_ssl_certificate_none(self): - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) url = self.mockserver.url("/echo?body=test", is_secure=False) yield crawler.crawl(seed=url, mockserver=self.mockserver) self.assertIsNone(crawler.spider.meta['responses'][0].certificate) @defer.inlineCallbacks def test_response_ssl_certificate(self): - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) url = self.mockserver.url("/echo?body=test", is_secure=True) yield crawler.crawl(seed=url, mockserver=self.mockserver) cert = crawler.spider.meta['responses'][0].certificate @@ -497,7 +497,7 @@ class CrawlSpiderTestCase(TestCase): @mark.xfail(reason="Responses with no body return early and contain no certificate") @defer.inlineCallbacks def test_response_ssl_certificate_empty_response(self): - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) url = self.mockserver.url("/status?n=200", is_secure=True) yield crawler.crawl(seed=url, mockserver=self.mockserver) cert = crawler.spider.meta['responses'][0].certificate @@ -507,7 +507,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_dns_server_ip_address_none(self): - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) url = self.mockserver.url('/status?n=200') yield crawler.crawl(seed=url, mockserver=self.mockserver) ip_address = crawler.spider.meta['responses'][0].ip_address @@ -515,7 +515,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_dns_server_ip_address(self): - crawler = self.runner.create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) url = self.mockserver.url('/echo?body=test') expected_netloc, _ = urlparse(url).netloc.split(':') yield crawler.crawl(seed=url, mockserver=self.mockserver) @@ -525,7 +525,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_bytes_received_stop_download_callback(self): - crawler = self.runner.create_crawler(BytesReceivedCallbackSpider) + crawler = get_crawler(BytesReceivedCallbackSpider) yield crawler.crawl(mockserver=self.mockserver) self.assertIsNone(crawler.spider.meta.get("failure")) self.assertIsInstance(crawler.spider.meta["response"], Response) @@ -534,7 +534,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_bytes_received_stop_download_errback(self): - crawler = self.runner.create_crawler(BytesReceivedErrbackSpider) + crawler = get_crawler(BytesReceivedErrbackSpider) yield crawler.crawl(mockserver=self.mockserver) self.assertIsNone(crawler.spider.meta.get("response")) self.assertIsInstance(crawler.spider.meta["failure"], Failure) @@ -549,7 +549,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_headers_received_stop_download_callback(self): - crawler = self.runner.create_crawler(HeadersReceivedCallbackSpider) + crawler = get_crawler(HeadersReceivedCallbackSpider) yield crawler.crawl(mockserver=self.mockserver) self.assertIsNone(crawler.spider.meta.get("failure")) self.assertIsInstance(crawler.spider.meta["response"], Response) @@ -557,7 +557,7 @@ class CrawlSpiderTestCase(TestCase): @defer.inlineCallbacks def test_headers_received_stop_download_errback(self): - crawler = self.runner.create_crawler(HeadersReceivedErrbackSpider) + crawler = get_crawler(HeadersReceivedErrbackSpider) yield crawler.crawl(mockserver=self.mockserver) self.assertIsNone(crawler.spider.meta.get("response")) self.assertIsInstance(crawler.spider.meta["failure"], Failure) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index f7aa769e4..d67abed7c 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -13,11 +13,13 @@ from twisted.trial import unittest import scrapy from scrapy.crawler import Crawler, CrawlerRunner, CrawlerProcess +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.settings import Settings, default_settings from scrapy.spiderloader import SpiderLoader from scrapy.utils.log import configure_logging, get_scrapy_root_handler from scrapy.utils.spider import DefaultSpider from scrapy.utils.misc import load_object +from scrapy.utils.test import get_crawler from scrapy.extensions.throttle import AutoThrottle from scrapy.extensions import telnet from scrapy.utils.test import get_testenv @@ -34,9 +36,6 @@ class BaseCrawlerTest(unittest.TestCase): class CrawlerTestCase(BaseCrawlerTest): - def setUp(self): - self.crawler = Crawler(DefaultSpider, Settings()) - def test_populate_spidercls_settings(self): spider_settings = {'TEST1': 'spider', 'TEST2': 'spider'} project_settings = {'TEST1': 'project', 'TEST3': 'project'} @@ -46,7 +45,9 @@ class CrawlerTestCase(BaseCrawlerTest): settings = Settings() settings.setdict(project_settings, priority='project') - crawler = Crawler(CustomSettingsSpider, settings) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ScrapyDeprecationWarning) + crawler = Crawler(CustomSettingsSpider, settings) self.assertEqual(crawler.settings.get('TEST1'), 'spider') self.assertEqual(crawler.settings.get('TEST2'), 'spider') @@ -56,12 +57,14 @@ class CrawlerTestCase(BaseCrawlerTest): self.assertTrue(crawler.settings.frozen) def test_crawler_accepts_dict(self): - crawler = Crawler(DefaultSpider, {'foo': 'bar'}) + crawler = get_crawler(DefaultSpider, {'foo': 'bar'}) self.assertEqual(crawler.settings['foo'], 'bar') self.assertOptionIsDefault(crawler.settings, 'RETRY_ENABLED') def test_crawler_accepts_None(self): - crawler = Crawler(DefaultSpider) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ScrapyDeprecationWarning) + crawler = Crawler(DefaultSpider) self.assertOptionIsDefault(crawler.settings, 'RETRY_ENABLED') def test_crawler_rejects_spider_objects(self): @@ -77,7 +80,7 @@ class SpiderSettingsTestCase(unittest.TestCase): 'AUTOTHROTTLE_ENABLED': True } - crawler = Crawler(MySpider, {}) + crawler = get_crawler(MySpider) enabled_exts = [e.__class__ for e in crawler.extensions.middlewares] self.assertIn(AutoThrottle, enabled_exts) @@ -91,7 +94,7 @@ class CrawlerLoggingTestCase(unittest.TestCase): class MySpider(scrapy.Spider): name = 'spider' - Crawler(MySpider, {}) + get_crawler(MySpider) assert get_scrapy_root_handler() is None def test_spider_custom_settings_log_level(self): @@ -111,7 +114,7 @@ class CrawlerLoggingTestCase(unittest.TestCase): configure_logging() self.assertEqual(get_scrapy_root_handler().level, logging.DEBUG) - crawler = Crawler(MySpider, {}) + crawler = get_crawler(MySpider) self.assertEqual(get_scrapy_root_handler().level, logging.INFO) info_count = crawler.stats.get_value('log_count/INFO') logging.debug('debug message') @@ -148,7 +151,7 @@ class CrawlerLoggingTestCase(unittest.TestCase): } configure_logging() - Crawler(MySpider, {}) + get_crawler(MySpider) logging.debug('debug message') with open(log_file, 'rb') as fo: @@ -229,22 +232,25 @@ class NoRequestsSpider(scrapy.Spider): @mark.usefixtures('reactor_pytest') class CrawlerRunnerHasSpider(unittest.TestCase): + def _runner(self): + return CrawlerRunner({'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'}) + @defer.inlineCallbacks def test_crawler_runner_bootstrap_successful(self): - runner = CrawlerRunner() + runner = self._runner() yield runner.crawl(NoRequestsSpider) self.assertEqual(runner.bootstrap_failed, False) @defer.inlineCallbacks def test_crawler_runner_bootstrap_successful_for_several(self): - runner = CrawlerRunner() + runner = self._runner() yield runner.crawl(NoRequestsSpider) yield runner.crawl(NoRequestsSpider) self.assertEqual(runner.bootstrap_failed, False) @defer.inlineCallbacks def test_crawler_runner_bootstrap_failed(self): - runner = CrawlerRunner() + runner = self._runner() try: yield runner.crawl(ExceptionSpider) @@ -257,7 +263,7 @@ class CrawlerRunnerHasSpider(unittest.TestCase): @defer.inlineCallbacks def test_crawler_runner_bootstrap_failed_for_several(self): - runner = CrawlerRunner() + runner = self._runner() try: yield runner.crawl(ExceptionSpider) @@ -275,12 +281,14 @@ class CrawlerRunnerHasSpider(unittest.TestCase): if self.reactor_pytest == 'asyncio': CrawlerRunner(settings={ "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", + "REQUEST_FINGERPRINTER_IMPLEMENTATION": "VERSION", }) else: msg = r"The installed reactor \(.*?\) does not match the requested one \(.*?\)" with self.assertRaisesRegex(Exception, msg): runner = CrawlerRunner(settings={ "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", + "REQUEST_FINGERPRINTER_IMPLEMENTATION": "VERSION", }) yield runner.crawl(NoRequestsSpider) diff --git a/tests/test_downloadermiddleware_httpauth.py b/tests/test_downloadermiddleware_httpauth.py index 0362e2018..b9f3e24a4 100644 --- a/tests/test_downloadermiddleware_httpauth.py +++ b/tests/test_downloadermiddleware_httpauth.py @@ -1,7 +1,9 @@ import unittest +import pytest from w3lib.http import basic_auth_header +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request from scrapy.downloadermiddlewares.httpauth import HttpAuthMiddleware from scrapy.spiders import Spider @@ -30,8 +32,10 @@ class HttpAuthMiddlewareLegacyTest(unittest.TestCase): self.spider = TestSpiderLegacy('foo') def test_auth(self): - mw = HttpAuthMiddleware() - mw.spider_opened(self.spider) + with pytest.warns(ScrapyDeprecationWarning, + match="Using HttpAuthMiddleware without http_auth_domain is deprecated"): + mw = HttpAuthMiddleware() + mw.spider_opened(self.spider) # initial request, sets the domain and sends the header req = Request('http://example.com/') @@ -49,8 +53,10 @@ class HttpAuthMiddlewareLegacyTest(unittest.TestCase): self.assertNotIn('Authorization', req.headers) def test_auth_already_set(self): - mw = HttpAuthMiddleware() - mw.spider_opened(self.spider) + with pytest.warns(ScrapyDeprecationWarning, + match="Using HttpAuthMiddleware without http_auth_domain is deprecated"): + mw = HttpAuthMiddleware() + mw.spider_opened(self.spider) req = Request('http://example.com/', headers=dict(Authorization='Digest 123')) assert mw.process_request(req, self.spider) is None diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py index 7c97bf32a..4ac85c1ec 100644 --- a/tests/test_downloadermiddleware_httpproxy.py +++ b/tests/test_downloadermiddleware_httpproxy.py @@ -1,13 +1,13 @@ import os -from functools import partial + +import pytest from twisted.trial.unittest import TestCase from scrapy.downloadermiddlewares.httpproxy import HttpProxyMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request from scrapy.spiders import Spider -from scrapy.crawler import Crawler -from scrapy.settings import Settings +from scrapy.utils.test import get_crawler spider = Spider('foo') @@ -23,9 +23,9 @@ class TestHttpProxyMiddleware(TestCase): os.environ = self._oldenv def test_not_enabled(self): - settings = Settings({'HTTPPROXY_ENABLED': False}) - crawler = Crawler(Spider, settings) - self.assertRaises(NotConfigured, partial(HttpProxyMiddleware.from_crawler, crawler)) + crawler = get_crawler(Spider, {'HTTPPROXY_ENABLED': False}) + with pytest.raises(NotConfigured): + HttpProxyMiddleware.from_crawler(crawler) def test_no_environment_proxies(self): os.environ = {'dummy_proxy': 'reset_env_and_do_not_raise'} diff --git a/tests/test_downloadermiddleware_stats.py b/tests/test_downloadermiddleware_stats.py index 9e75f0a50..7d88ba4d2 100644 --- a/tests/test_downloadermiddleware_stats.py +++ b/tests/test_downloadermiddleware_stats.py @@ -1,7 +1,9 @@ +import warnings from itertools import product from unittest import TestCase from scrapy.downloadermiddlewares.stats import DownloaderStats +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.spiders import Spider from scrapy.utils.response import response_httprepr @@ -54,7 +56,10 @@ class TestDownloaderStats(TestCase): for test_response in test_responses: self.crawler.stats.set_value('downloader/response_bytes', 0) self.mw.process_response(self.req, test_response, self.spider) - self.assertStatsEqual('downloader/response_bytes', len(response_httprepr(test_response))) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ScrapyDeprecationWarning) + resp_size = len(response_httprepr(test_response)) + self.assertStatsEqual('downloader/response_bytes', resp_size) def test_process_exception(self): self.mw.process_exception(self.req, MyException(), self.spider) diff --git a/tests/test_dupefilters.py b/tests/test_dupefilters.py index b7df2554a..8a37a8ebe 100644 --- a/tests/test_dupefilters.py +++ b/tests/test_dupefilters.py @@ -10,7 +10,6 @@ from scrapy.dupefilters import RFPDupeFilter from scrapy.http import Request from scrapy.core.scheduler import Scheduler from scrapy.utils.python import to_bytes -from scrapy.utils.job import job_dir from scrapy.utils.test import get_crawler from tests.spiders import SimpleSpider @@ -29,8 +28,7 @@ class FromCrawlerRFPDupeFilter(RFPDupeFilter): @classmethod def from_crawler(cls, crawler): - debug = crawler.settings.getbool('DUPEFILTER_DEBUG') - df = cls(job_dir(crawler.settings), debug) + df = super().from_crawler(crawler) df.method = 'from_crawler' return df @@ -38,9 +36,8 @@ class FromCrawlerRFPDupeFilter(RFPDupeFilter): class FromSettingsRFPDupeFilter(RFPDupeFilter): @classmethod - def from_settings(cls, settings): - debug = settings.getbool('DUPEFILTER_DEBUG') - df = cls(job_dir(settings), debug) + def from_settings(cls, settings, *, fingerprinter=None): + df = super().from_settings(settings, fingerprinter=fingerprinter) df.method = 'from_settings' return df @@ -53,7 +50,8 @@ class RFPDupeFilterTest(unittest.TestCase): def test_df_from_crawler_scheduler(self): settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertTrue(scheduler.df.debug) @@ -61,14 +59,16 @@ class RFPDupeFilterTest(unittest.TestCase): def test_df_from_settings_scheduler(self): settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': FromSettingsRFPDupeFilter} + 'DUPEFILTER_CLASS': FromSettingsRFPDupeFilter, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertTrue(scheduler.df.debug) self.assertEqual(scheduler.df.method, 'from_settings') def test_df_direct_scheduler(self): - settings = {'DUPEFILTER_CLASS': DirectDupeFilter} + settings = {'DUPEFILTER_CLASS': DirectDupeFilter, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(settings_dict=settings) scheduler = Scheduler.from_crawler(crawler) self.assertEqual(scheduler.df.method, 'n/a') @@ -171,7 +171,8 @@ class RFPDupeFilterTest(unittest.TestCase): def test_log(self): with LogCapture() as log: settings = {'DUPEFILTER_DEBUG': False, - 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(SimpleSpider, settings_dict=settings) spider = SimpleSpider.from_crawler(crawler) dupefilter = _get_dupefilter(crawler=crawler) @@ -197,7 +198,8 @@ class RFPDupeFilterTest(unittest.TestCase): def test_log_debug(self): with LogCapture() as log: settings = {'DUPEFILTER_DEBUG': True, - 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter} + 'DUPEFILTER_CLASS': FromCrawlerRFPDupeFilter, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(SimpleSpider, settings_dict=settings) spider = SimpleSpider.from_crawler(crawler) dupefilter = _get_dupefilter(crawler=crawler) @@ -230,7 +232,8 @@ class RFPDupeFilterTest(unittest.TestCase): def test_log_debug_default_dupefilter(self): with LogCapture() as log: - settings = {'DUPEFILTER_DEBUG': True} + settings = {'DUPEFILTER_DEBUG': True, + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION'} crawler = get_crawler(SimpleSpider, settings_dict=settings) spider = SimpleSpider.from_crawler(crawler) dupefilter = _get_dupefilter(crawler=crawler) diff --git a/tests/test_engine.py b/tests/test_engine.py index fa7d0c8d4..1bd802bcf 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -13,10 +13,11 @@ module with the ``runserver`` argument:: import os import re import sys -import warnings from collections import defaultdict from urllib.parse import urlparse +from dataclasses import dataclass +import pytest import attr from itemadapter import ItemAdapter from pydispatch import dispatcher @@ -50,6 +51,13 @@ class AttrsItem: price = attr.ib(default=0) +@dataclass +class DataClassItem: + name: str = "" + url: str = "" + price: int = 0 + + class TestSpider(Spider): name = "scrapytest.org" allowed_domains = ["scrapytest.org", "localhost"] @@ -92,17 +100,8 @@ class AttrsItemsSpider(TestSpider): item_cls = AttrsItem -try: - from dataclasses import make_dataclass -except ImportError: - DataClassItemsSpider = None -else: - TestDataClass = make_dataclass("TestDataClass", [("name", str), ("url", str), ("price", int)]) - - class DataClassItemsSpider(DictItemsSpider): # type: ignore[no-redef] - def parse_item(self, response): - item = super().parse_item(response) - return TestDataClass(**item) +class DataClassItemsSpider(TestSpider): + item_cls = DataClassItem class ItemZeroDivisionErrorSpider(TestSpider): @@ -188,7 +187,7 @@ class CrawlerRun: return self.deferred def stop(self): - self.port.stopListening() + self.port.stopListening() # FIXME: wait for this Deferred for name, signal in vars(signals).items(): if not name.startswith('_'): disconnect_all(signal) @@ -239,79 +238,77 @@ class EngineTest(unittest.TestCase): def test_crawler(self): for spider in (TestSpider, DictItemsSpider, AttrsItemsSpider, DataClassItemsSpider): - if spider is None: - continue - self.run = CrawlerRun(spider) - yield self.run.run() - self._assert_visited_urls() - self._assert_scheduled_requests(count=9) - self._assert_downloaded_responses(count=9) - self._assert_scraped_items() - self._assert_signals_caught() - self._assert_bytes_received() + run = CrawlerRun(spider) + yield run.run() + self._assert_visited_urls(run) + self._assert_scheduled_requests(run, count=9) + self._assert_downloaded_responses(run, count=9) + self._assert_scraped_items(run) + self._assert_signals_caught(run) + self._assert_bytes_received(run) @defer.inlineCallbacks def test_crawler_dupefilter(self): - self.run = CrawlerRun(TestDupeFilterSpider) - yield self.run.run() - self._assert_scheduled_requests(count=8) - self._assert_dropped_requests() + run = CrawlerRun(TestDupeFilterSpider) + yield run.run() + self._assert_scheduled_requests(run, count=8) + self._assert_dropped_requests(run) @defer.inlineCallbacks def test_crawler_itemerror(self): - self.run = CrawlerRun(ItemZeroDivisionErrorSpider) - yield self.run.run() - self._assert_items_error() + run = CrawlerRun(ItemZeroDivisionErrorSpider) + yield run.run() + self._assert_items_error(run) @defer.inlineCallbacks def test_crawler_change_close_reason_on_idle(self): - self.run = CrawlerRun(ChangeCloseReasonSpider) - yield self.run.run() - self.assertEqual({'spider': self.run.spider, 'reason': 'custom_reason'}, - self.run.signals_caught[signals.spider_closed]) + run = CrawlerRun(ChangeCloseReasonSpider) + yield run.run() + self.assertEqual({'spider': run.spider, 'reason': 'custom_reason'}, + run.signals_caught[signals.spider_closed]) - def _assert_visited_urls(self): + def _assert_visited_urls(self, run: CrawlerRun): must_be_visited = ["/", "/redirect", "/redirected", "/item1.html", "/item2.html", "/item999.html"] - urls_visited = {rp[0].url for rp in self.run.respplug} - urls_expected = {self.run.geturl(p) for p in must_be_visited} + urls_visited = {rp[0].url for rp in run.respplug} + urls_expected = {run.geturl(p) for p in must_be_visited} assert urls_expected <= urls_visited, f"URLs not visited: {list(urls_expected - urls_visited)}" - def _assert_scheduled_requests(self, count=None): - self.assertEqual(count, len(self.run.reqplug)) + def _assert_scheduled_requests(self, run: CrawlerRun, count=None): + self.assertEqual(count, len(run.reqplug)) paths_expected = ['/item999.html', '/item2.html', '/item1.html'] - urls_requested = {rq[0].url for rq in self.run.reqplug} - urls_expected = {self.run.geturl(p) for p in paths_expected} + urls_requested = {rq[0].url for rq in run.reqplug} + urls_expected = {run.geturl(p) for p in paths_expected} assert urls_expected <= urls_requested - scheduled_requests_count = len(self.run.reqplug) - dropped_requests_count = len(self.run.reqdropped) - responses_count = len(self.run.respplug) + scheduled_requests_count = len(run.reqplug) + dropped_requests_count = len(run.reqdropped) + responses_count = len(run.respplug) self.assertEqual(scheduled_requests_count, dropped_requests_count + responses_count) - self.assertEqual(len(self.run.reqreached), + self.assertEqual(len(run.reqreached), responses_count) - def _assert_dropped_requests(self): - self.assertEqual(len(self.run.reqdropped), 1) + def _assert_dropped_requests(self, run: CrawlerRun): + self.assertEqual(len(run.reqdropped), 1) - def _assert_downloaded_responses(self, count): + def _assert_downloaded_responses(self, run: CrawlerRun, count): # response tests - self.assertEqual(count, len(self.run.respplug)) - self.assertEqual(count, len(self.run.reqreached)) + self.assertEqual(count, len(run.respplug)) + self.assertEqual(count, len(run.reqreached)) - for response, _ in self.run.respplug: - if self.run.getpath(response.url) == '/item999.html': + for response, _ in run.respplug: + if run.getpath(response.url) == '/item999.html': self.assertEqual(404, response.status) - if self.run.getpath(response.url) == '/redirect': + if run.getpath(response.url) == '/redirect': self.assertEqual(302, response.status) - def _assert_items_error(self): - self.assertEqual(2, len(self.run.itemerror)) - for item, response, spider, failure in self.run.itemerror: + def _assert_items_error(self, run: CrawlerRun): + self.assertEqual(2, len(run.itemerror)) + for item, response, spider, failure in run.itemerror: self.assertEqual(failure.value.__class__, ZeroDivisionError) - self.assertEqual(spider, self.run.spider) + self.assertEqual(spider, run.spider) self.assertEqual(item['url'], response.url) if 'item1.html' in item['url']: @@ -321,9 +318,9 @@ class EngineTest(unittest.TestCase): self.assertEqual('Item 2 name', item['name']) self.assertEqual('200', item['price']) - def _assert_scraped_items(self): - self.assertEqual(2, len(self.run.itemresp)) - for item, response in self.run.itemresp: + def _assert_scraped_items(self, run: CrawlerRun): + self.assertEqual(2, len(run.itemresp)) + for item, response in run.itemresp: item = ItemAdapter(item) self.assertEqual(item['url'], response.url) if 'item1.html' in item['url']: @@ -333,26 +330,26 @@ class EngineTest(unittest.TestCase): self.assertEqual('Item 2 name', item['name']) self.assertEqual('200', item['price']) - def _assert_headers_received(self): - for headers in self.run.headers.values(): + def _assert_headers_received(self, run: CrawlerRun): + for headers in run.headers.values(): self.assertIn(b"Server", headers) self.assertIn(b"TwistedWeb", headers[b"Server"]) self.assertIn(b"Date", headers) self.assertIn(b"Content-Type", headers) - def _assert_bytes_received(self): - self.assertEqual(9, len(self.run.bytes)) - for request, data in self.run.bytes.items(): + def _assert_bytes_received(self, run: CrawlerRun): + self.assertEqual(9, len(run.bytes)) + for request, data in run.bytes.items(): joined_data = b"".join(data) - if self.run.getpath(request.url) == "/": + if run.getpath(request.url) == "/": self.assertEqual(joined_data, get_testdata("test_site", "index.html")) - elif self.run.getpath(request.url) == "/item1.html": + elif run.getpath(request.url) == "/item1.html": self.assertEqual(joined_data, get_testdata("test_site", "item1.html")) - elif self.run.getpath(request.url) == "/item2.html": + elif run.getpath(request.url) == "/item2.html": self.assertEqual(joined_data, get_testdata("test_site", "item2.html")) - elif self.run.getpath(request.url) == "/redirected": + elif run.getpath(request.url) == "/redirected": self.assertEqual(joined_data, b"Redirected here") - elif self.run.getpath(request.url) == '/redirect': + elif run.getpath(request.url) == '/redirect': self.assertEqual( joined_data, b"\n\n" @@ -364,7 +361,7 @@ class EngineTest(unittest.TestCase): b" \n" b"\n" ) - elif self.run.getpath(request.url) == "/tem999.html": + elif run.getpath(request.url) == "/tem999.html": self.assertEqual( joined_data, b"\n\n" @@ -375,27 +372,27 @@ class EngineTest(unittest.TestCase): b" \n" b"\n" ) - elif self.run.getpath(request.url) == "/numbers": + elif 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**18)] self.assertEqual(joined_data, b"".join(numbers)) - 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 - assert signals.headers_received in self.run.signals_caught + def _assert_signals_caught(self, run: CrawlerRun): + assert signals.engine_started in run.signals_caught + assert signals.engine_stopped in run.signals_caught + assert signals.spider_opened in run.signals_caught + assert signals.spider_idle in run.signals_caught + assert signals.spider_closed in run.signals_caught + assert signals.headers_received in run.signals_caught - self.assertEqual({'spider': self.run.spider}, - self.run.signals_caught[signals.spider_opened]) - self.assertEqual({'spider': self.run.spider}, - self.run.signals_caught[signals.spider_idle]) - self.assertEqual({'spider': self.run.spider, 'reason': 'finished'}, - self.run.signals_caught[signals.spider_closed]) + self.assertEqual({'spider': run.spider}, + run.signals_caught[signals.spider_opened]) + self.assertEqual({'spider': run.spider}, + run.signals_caught[signals.spider_idle]) + self.assertEqual({'spider': run.spider, 'reason': 'finished'}, + run.signals_caught[signals.spider_closed]) @defer.inlineCallbacks def test_close_downloader(self): @@ -407,28 +404,29 @@ class EngineTest(unittest.TestCase): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) yield e.open_spider(TestSpider(), []) e.start() - yield self.assertFailure(e.start(), RuntimeError).addBoth( - lambda exc: self.assertEqual(str(exc), "Engine already running") - ) - yield e.stop() + try: + yield self.assertFailure(e.start(), RuntimeError).addBoth( + lambda exc: self.assertEqual(str(exc), "Engine already running") + ) + finally: + yield e.stop() @defer.inlineCallbacks def test_close_spiders_downloader(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="ExecutionEngine.open_spiders is deprecated, " + "please use ExecutionEngine.spider instead"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) yield e.open_spider(TestSpider(), []) self.assertEqual(len(e.open_spiders), 1) yield e.close() self.assertEqual(len(e.open_spiders), 0) - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual( - str(warning_list[0].message), - "ExecutionEngine.open_spiders is deprecated, please use ExecutionEngine.spider instead", - ) @defer.inlineCallbacks def test_close_engine_spiders_downloader(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="ExecutionEngine.open_spiders is deprecated, " + "please use ExecutionEngine.spider instead"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) yield e.open_spider(TestSpider(), []) e.start() @@ -436,61 +434,47 @@ class EngineTest(unittest.TestCase): yield e.close() self.assertFalse(e.running) self.assertEqual(len(e.open_spiders), 0) - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual( - str(warning_list[0].message), - "ExecutionEngine.open_spiders is deprecated, please use ExecutionEngine.spider instead", - ) @defer.inlineCallbacks def test_crawl_deprecated_spider_arg(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="Passing a 'spider' argument to " + "ExecutionEngine.crawl is deprecated"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) spider = TestSpider() yield e.open_spider(spider, []) e.start() e.crawl(Request("data:,"), spider) yield e.close() - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual( - str(warning_list[0].message), - "Passing a 'spider' argument to ExecutionEngine.crawl is deprecated", - ) @defer.inlineCallbacks def test_download_deprecated_spider_arg(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="Passing a 'spider' argument to " + "ExecutionEngine.download is deprecated"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) spider = TestSpider() yield e.open_spider(spider, []) e.start() e.download(Request("data:,"), spider) yield e.close() - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual( - str(warning_list[0].message), - "Passing a 'spider' argument to ExecutionEngine.download is deprecated", - ) @defer.inlineCallbacks def test_deprecated_schedule(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="ExecutionEngine.schedule is deprecated, please use " + "ExecutionEngine.crawl or ExecutionEngine.download instead"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) spider = TestSpider() yield e.open_spider(spider, []) e.start() e.schedule(Request("data:,"), spider) yield e.close() - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual( - str(warning_list[0].message), - "ExecutionEngine.schedule is deprecated, please use " - "ExecutionEngine.crawl or ExecutionEngine.download instead", - ) @defer.inlineCallbacks def test_deprecated_has_capacity(self): - with warnings.catch_warnings(record=True) as warning_list: + with pytest.warns(ScrapyDeprecationWarning, + match="ExecutionEngine.has_capacity is deprecated"): e = ExecutionEngine(get_crawler(TestSpider), lambda _: None) self.assertTrue(e.has_capacity()) spider = TestSpider() @@ -499,8 +483,6 @@ class EngineTest(unittest.TestCase): e.start() yield e.close() self.assertTrue(e.has_capacity()) - self.assertEqual(warning_list[0].category, ScrapyDeprecationWarning) - self.assertEqual(str(warning_list[0].message), "ExecutionEngine.has_capacity is deprecated") if __name__ == "__main__": diff --git a/tests/test_engine_stop_download_bytes.py b/tests/test_engine_stop_download_bytes.py index 0ba69e096..933e4067d 100644 --- a/tests/test_engine_stop_download_bytes.py +++ b/tests/test_engine_stop_download_bytes.py @@ -23,36 +23,34 @@ class BytesReceivedEngineTest(EngineTest): @defer.inlineCallbacks def test_crawler(self): for spider in (TestSpider, DictItemsSpider, AttrsItemsSpider, DataClassItemsSpider): - if spider is None: - continue - self.run = BytesReceivedCrawlerRun(spider) + run = BytesReceivedCrawlerRun(spider) with LogCapture() as log: - yield self.run.run() + yield run.run() log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for " + f"Download stopped for " "from signal handler BytesReceivedCrawlerRun.bytes_received")) log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for " + f"Download stopped for " "from signal handler BytesReceivedCrawlerRun.bytes_received")) log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for " + f"Download stopped for " "from signal handler BytesReceivedCrawlerRun.bytes_received")) - self._assert_visited_urls() - self._assert_scheduled_requests(count=9) - self._assert_downloaded_responses(count=9) - self._assert_signals_caught() - self._assert_headers_received() - self._assert_bytes_received() + self._assert_visited_urls(run) + self._assert_scheduled_requests(run, count=9) + self._assert_downloaded_responses(run, count=9) + self._assert_signals_caught(run) + self._assert_headers_received(run) + self._assert_bytes_received(run) - def _assert_bytes_received(self): - self.assertEqual(9, len(self.run.bytes)) - for request, data in self.run.bytes.items(): + def _assert_bytes_received(self, run: CrawlerRun): + self.assertEqual(9, len(run.bytes)) + for request, data in run.bytes.items(): joined_data = b"".join(data) self.assertTrue(len(data) == 1) # signal was fired only once - if self.run.getpath(request.url) == "/numbers": + if run.getpath(request.url) == "/numbers": # Received bytes are not the complete response. The exact amount depends # on the buffer size, which can vary, so we only check that the amount # of received bytes is strictly less than the full response. diff --git a/tests/test_engine_stop_download_headers.py b/tests/test_engine_stop_download_headers.py index fad6643ad..8975d0e3f 100644 --- a/tests/test_engine_stop_download_headers.py +++ b/tests/test_engine_stop_download_headers.py @@ -23,34 +23,32 @@ class HeadersReceivedEngineTest(EngineTest): @defer.inlineCallbacks def test_crawler(self): for spider in (TestSpider, DictItemsSpider, AttrsItemsSpider, DataClassItemsSpider): - if spider is None: - continue - self.run = HeadersReceivedCrawlerRun(spider) + run = HeadersReceivedCrawlerRun(spider) with LogCapture() as log: - yield self.run.run() + yield run.run() log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for from" + f"Download stopped for from" " signal handler HeadersReceivedCrawlerRun.headers_received")) log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for from signal" + f"Download stopped for from signal" " handler HeadersReceivedCrawlerRun.headers_received")) log.check_present(("scrapy.core.downloader.handlers.http11", "DEBUG", - f"Download stopped for from" + f"Download stopped for from" " signal handler HeadersReceivedCrawlerRun.headers_received")) - self._assert_visited_urls() - self._assert_downloaded_responses(count=6) - self._assert_signals_caught() - self._assert_bytes_received() - self._assert_headers_received() + self._assert_visited_urls(run) + self._assert_downloaded_responses(run, count=6) + self._assert_signals_caught(run) + self._assert_bytes_received(run) + self._assert_headers_received(run) - def _assert_bytes_received(self): - self.assertEqual(0, len(self.run.bytes)) + def _assert_bytes_received(self, run: CrawlerRun): + self.assertEqual(0, len(run.bytes)) - def _assert_visited_urls(self): + def _assert_visited_urls(self, run: CrawlerRun): must_be_visited = ["/", "/redirect", "/redirected"] - urls_visited = {rp[0].url for rp in self.run.respplug} - urls_expected = {self.run.geturl(p) for p in must_be_visited} + urls_visited = {rp[0].url for rp in run.respplug} + urls_expected = {run.geturl(p) for p in must_be_visited} assert urls_expected <= urls_visited, f"URLs not visited: {list(urls_expected - urls_visited)}" diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index ec48f8d4a..ecd1b59d3 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -22,6 +22,7 @@ from urllib.parse import urljoin, quote from urllib.request import pathname2url import lxml.etree +import pytest from testfixtures import LogCapture from twisted.internet import defer from twisted.trial import unittest @@ -30,7 +31,6 @@ from zope.interface import implementer from zope.interface.verify import verifyObject import scrapy -from scrapy.crawler import CrawlerRunner from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.exporters import CsvItemExporter from scrapy.extensions.feedexport import ( @@ -697,9 +697,9 @@ class FeedExportTest(FeedExportTestBase): content = {} try: with MockServer() as s: - runner = CrawlerRunner(Settings(settings)) spider_cls.start_urls = [s.url('/')] - yield runner.crawl(spider_cls) + crawler = get_crawler(spider_cls, settings) + yield crawler.crawl() for file_path, feed_options in FEEDS.items(): if not os.path.exists(str(file_path)): @@ -1554,9 +1554,9 @@ class FeedPostProcessedExportsTest(FeedExportTestBase): content = {} try: with MockServer() as s: - runner = CrawlerRunner(Settings(settings)) spider_cls.start_urls = [s.url('/')] - yield runner.crawl(spider_cls) + crawler = get_crawler(spider_cls, settings) + yield crawler.crawl() for file_path, feed_options in FEEDS.items(): if not os.path.exists(str(file_path)): @@ -2026,9 +2026,9 @@ class BatchDeliveriesTest(FeedExportTestBase): content = defaultdict(list) try: with MockServer() as s: - runner = CrawlerRunner(Settings(settings)) spider_cls.start_urls = [s.url('/')] - yield runner.crawl(spider_cls) + crawler = get_crawler(spider_cls, settings) + yield crawler.crawl() for path, feed in FEEDS.items(): dir_name = os.path.dirname(path) @@ -2048,7 +2048,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'jl', self._file_mark): {'format': 'jl'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') rows = [{k: v for k, v in row.items() if v} for row in rows] data = yield self.exported_data(items, settings) for batch in data['jl']: @@ -2064,7 +2064,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'csv', self._file_mark): {'format': 'csv'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') data = yield self.exported_data(items, settings) for batch in data['csv']: got_batch = csv.DictReader(to_unicode(batch).splitlines()) @@ -2080,7 +2080,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'xml', self._file_mark): {'format': 'xml'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') rows = [{k: v for k, v in row.items() if v} for row in rows] data = yield self.exported_data(items, settings) for batch in data['xml']: @@ -2098,7 +2098,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'json', self._file_mark): {'format': 'json'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') rows = [{k: v for k, v in row.items() if v} for row in rows] data = yield self.exported_data(items, settings) # XML @@ -2123,7 +2123,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'pickle', self._file_mark): {'format': 'pickle'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') rows = [{k: v for k, v in row.items() if v} for row in rows] data = yield self.exported_data(items, settings) import pickle @@ -2140,7 +2140,7 @@ class BatchDeliveriesTest(FeedExportTestBase): os.path.join(self._random_temp_filename(), 'marshal', self._file_mark): {'format': 'marshal'}, }, }) - batch_size = settings.getint('FEED_EXPORT_BATCH_ITEM_COUNT') + batch_size = Settings(settings).getint('FEED_EXPORT_BATCH_ITEM_COUNT') rows = [{k: v for k, v in row.items() if v} for row in rows] data = yield self.exported_data(items, settings) import marshal @@ -2166,7 +2166,7 @@ class BatchDeliveriesTest(FeedExportTestBase): 'FEED_EXPORT_BATCH_ITEM_COUNT': 2 } header = self.MyItem.fields.keys() - yield self.assertExported(items, header, rows, settings=Settings(settings)) + yield self.assertExported(items, header, rows, settings=settings) def test_wrong_path(self): """ If path is without %(batch_time)s and %(batch_id) an exception must be raised """ @@ -2382,9 +2382,9 @@ class BatchDeliveriesTest(FeedExportTestBase): yield item with MockServer() as server: - runner = CrawlerRunner(Settings(settings)) TestSpider.start_urls = [server.url('/')] - yield runner.crawl(TestSpider) + crawler = get_crawler(TestSpider, settings) + yield crawler.crawl() self.assertEqual(len(CustomS3FeedStorage.stubs), len(items) + 1) for stub in CustomS3FeedStorage.stubs[:-1]: @@ -2434,25 +2434,16 @@ class StdoutFeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': StdoutFeedStorageWithoutFeedOptions }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") - with warnings.catch_warnings(record=True) as w: + with pytest.warns(ScrapyDeprecationWarning, + match="StdoutFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "StdoutFeedStorageWithoutFeedOptions does not support " - "the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) class FileFeedStorageWithoutFeedOptions(FileFeedStorage): @@ -2476,25 +2467,16 @@ class FileFeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': FileFeedStorageWithoutFeedOptions }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) spider = scrapy.Spider("default") - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="FileFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "FileFeedStorageWithoutFeedOptions does not support " - "the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) class S3FeedStorageWithoutFeedOptions(S3FeedStorage): @@ -2524,26 +2506,18 @@ class S3FeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': S3FeedStorageWithoutFeedOptions }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="S3FeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "S3FeedStorageWithoutFeedOptions does not support " - "the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) def test_from_crawler(self): settings_dict = { @@ -2552,26 +2526,18 @@ class S3FeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': S3FeedStorageWithoutFeedOptionsWithFromCrawler }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="S3FeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "S3FeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler " - "does not support the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) class FTPFeedStorageWithoutFeedOptions(FTPFeedStorage): @@ -2601,26 +2567,18 @@ class FTPFeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': FTPFeedStorageWithoutFeedOptions }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="FTPFeedStorageWithoutFeedOptions does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "FTPFeedStorageWithoutFeedOptions does not support " - "the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) def test_from_crawler(self): settings_dict = { @@ -2629,50 +2587,50 @@ class FTPFeedStoragePreFeedOptionsTest(unittest.TestCase): 'file': FTPFeedStorageWithoutFeedOptionsWithFromCrawler }, } - crawler = get_crawler(settings_dict=settings_dict) - feed_exporter = FeedExporter.from_crawler(crawler) + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings_dict) + feed_exporter = FeedExporter.from_crawler(crawler) + spider = scrapy.Spider("default") spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="FTPFeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler does not support " + "the 'feed_options' keyword argument."): feed_exporter.open_spider(spider) - messages = tuple(str(item.message) for item in w - if item.category is ScrapyDeprecationWarning) - self.assertEqual( - messages, - ( - ( - "FTPFeedStorageWithoutFeedOptionsWithFromCrawler.from_crawler " - "does not support the 'feed_options' keyword argument. Add a " - "'feed_options' parameter to its signature to remove " - "this warning. This parameter will become mandatory " - "in a future version of Scrapy." - ), - ) - ) class URIParamsTest: spider_name = "uri_params_spider" + deprecated_options = False def build_settings(self, uri='file:///tmp/foobar', uri_params=None): raise NotImplementedError + def _crawler_feed_exporter(self, settings): + if self.deprecated_options: + with pytest.warns(ScrapyDeprecationWarning, + match="The `FEED_URI` and `FEED_FORMAT` settings have been deprecated"): + crawler = get_crawler(settings_dict=settings) + feed_exporter = FeedExporter.from_crawler(crawler) + else: + crawler = get_crawler(settings_dict=settings) + feed_exporter = FeedExporter.from_crawler(crawler) + return crawler, feed_exporter + def test_default(self): settings = self.build_settings( uri='file:///tmp/%(name)s', ) - crawler = get_crawler(settings_dict=settings) - feed_exporter = FeedExporter.from_crawler(crawler) + crawler, feed_exporter = self._crawler_feed_exporter(settings) spider = scrapy.Spider(self.spider_name) spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) feed_exporter.open_spider(spider) - messages = tuple( - str(item.message) for item in w - if item.category is ScrapyDeprecationWarning - ) - self.assertEqual(messages, tuple()) self.assertEqual( feed_exporter.slots[0].uri, @@ -2687,28 +2645,13 @@ class URIParamsTest: uri='file:///tmp/%(name)s', uri_params=uri_params, ) - crawler = get_crawler(settings_dict=settings) - feed_exporter = FeedExporter.from_crawler(crawler) + crawler, feed_exporter = self._crawler_feed_exporter(settings) spider = scrapy.Spider(self.spider_name) spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with pytest.warns(ScrapyDeprecationWarning, + match="Modifying the params dictionary in-place"): feed_exporter.open_spider(spider) - messages = tuple( - str(item.message) for item in w - if item.category is ScrapyDeprecationWarning - ) - self.assertEqual( - messages, - ( - ( - 'Modifying the params dictionary in-place in the ' - 'function defined in the FEED_URI_PARAMS setting or ' - 'in the uri_params key of the FEEDS setting is ' - 'deprecated. The function must return a new ' - 'dictionary instead.' - ), - ) - ) self.assertEqual( feed_exporter.slots[0].uri, @@ -2723,18 +2666,14 @@ class URIParamsTest: uri='file:///tmp/%(name)s', uri_params=uri_params, ) - crawler = get_crawler(settings_dict=settings) - feed_exporter = FeedExporter.from_crawler(crawler) + crawler, feed_exporter = self._crawler_feed_exporter(settings) spider = scrapy.Spider(self.spider_name) spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) with self.assertRaises(KeyError): feed_exporter.open_spider(spider) - messages = tuple( - str(item.message) for item in w - if item.category is ScrapyDeprecationWarning - ) - self.assertEqual(messages, tuple()) def test_params_as_is(self): def uri_params(params, spider): @@ -2744,17 +2683,12 @@ class URIParamsTest: uri='file:///tmp/%(name)s', uri_params=uri_params, ) - crawler = get_crawler(settings_dict=settings) - feed_exporter = FeedExporter.from_crawler(crawler) + crawler, feed_exporter = self._crawler_feed_exporter(settings) spider = scrapy.Spider(self.spider_name) spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) feed_exporter.open_spider(spider) - messages = tuple( - str(item.message) for item in w - if item.category is ScrapyDeprecationWarning - ) - self.assertEqual(messages, tuple()) self.assertEqual( feed_exporter.slots[0].uri, @@ -2769,17 +2703,12 @@ class URIParamsTest: uri='file:///tmp/%(foo)s', uri_params=uri_params, ) - crawler = get_crawler(settings_dict=settings) - feed_exporter = FeedExporter.from_crawler(crawler) + crawler, feed_exporter = self._crawler_feed_exporter(settings) spider = scrapy.Spider(self.spider_name) spider.crawler = crawler - with warnings.catch_warnings(record=True) as w: + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) feed_exporter.open_spider(spider) - messages = tuple( - str(item.message) for item in w - if item.category is ScrapyDeprecationWarning - ) - self.assertEqual(messages, tuple()) self.assertEqual( feed_exporter.slots[0].uri, @@ -2788,6 +2717,7 @@ class URIParamsTest: class URIParamsSettingTest(URIParamsTest, unittest.TestCase): + deprecated_options = True def build_settings(self, uri='file:///tmp/foobar', uri_params=None): extra_settings = {} @@ -2800,6 +2730,7 @@ class URIParamsSettingTest(URIParamsTest, unittest.TestCase): class URIParamsFeedOptionTest(URIParamsTest, unittest.TestCase): + deprecated_options = False def build_settings(self, uri='file:///tmp/foobar', uri_params=None): options = { diff --git a/tests/test_logformatter.py b/tests/test_logformatter.py index 6381f895b..f3bb23bda 100644 --- a/tests/test_logformatter.py +++ b/tests/test_logformatter.py @@ -5,8 +5,8 @@ from twisted.internet import defer from twisted.python.failure import Failure from twisted.trial.unittest import TestCase as TwistedTestCase -from scrapy.crawler import CrawlerRunner from scrapy.exceptions import DropItem +from scrapy.utils.test import get_crawler from scrapy.http import Request, Response from scrapy.item import Item, Field from scrapy.logformatter import LogFormatter @@ -202,7 +202,7 @@ class ShowOrSkipMessagesTestCase(TwistedTestCase): @defer.inlineCallbacks def test_show_messages(self): - crawler = CrawlerRunner(self.base_settings).create_crawler(ItemSpider) + crawler = get_crawler(ItemSpider, self.base_settings) with LogCapture() as lc: yield crawler.crawl(mockserver=self.mockserver) self.assertIn("Scraped from <200 http://127.0.0.1:", str(lc)) @@ -213,7 +213,7 @@ class ShowOrSkipMessagesTestCase(TwistedTestCase): def test_skip_messages(self): settings = self.base_settings.copy() settings['LOG_FORMATTER'] = SkipMessagesLogFormatter - crawler = CrawlerRunner(settings).create_crawler(ItemSpider) + crawler = get_crawler(ItemSpider, settings) with LogCapture() as lc: yield crawler.crawl(mockserver=self.mockserver) self.assertNotIn("Scraped from <200 http://127.0.0.1:", str(lc)) diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py index f49fda701..e46532a1c 100644 --- a/tests/test_pipeline_crawl.py +++ b/tests/test_pipeline_crawl.py @@ -64,6 +64,7 @@ class FileDownloadCrawlTestCase(TestCase): self.tmpmediastore = self.mktemp() os.mkdir(self.tmpmediastore) self.settings = { + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION', 'ITEM_PIPELINES': {self.pipeline_class: 1}, self.store_setting_key: self.tmpmediastore, } @@ -78,8 +79,10 @@ class FileDownloadCrawlTestCase(TestCase): def _on_item_scraped(self, item): self.items.append(item) - def _create_crawler(self, spider_class, **kwargs): - crawler = self.runner.create_crawler(spider_class, **kwargs) + def _create_crawler(self, spider_class, runner=None, **kwargs): + if runner is None: + runner = self.runner + crawler = runner.create_crawler(spider_class, **kwargs) crawler.signals.connect(self._on_item_scraped, signals.item_scraped) return crawler @@ -167,9 +170,8 @@ class FileDownloadCrawlTestCase(TestCase): def test_download_media_redirected_allowed(self): settings = dict(self.settings) settings.update({'MEDIA_ALLOW_REDIRECTS': True}) - self.runner = CrawlerRunner(settings) - - crawler = self._create_crawler(RedirectedMediaDownloadSpider) + runner = CrawlerRunner(settings) + crawler = self._create_crawler(RedirectedMediaDownloadSpider, runner=runner) with LogCapture() as log: yield crawler.crawl( self.mockserver.url("/files/images/"), diff --git a/tests/test_request_attribute_binding.py b/tests/test_request_attribute_binding.py index 25d9657d5..0406d906f 100644 --- a/tests/test_request_attribute_binding.py +++ b/tests/test_request_attribute_binding.py @@ -2,8 +2,8 @@ from twisted.internet import defer from twisted.trial.unittest import TestCase from scrapy import Request, signals -from scrapy.crawler import CrawlerRunner from scrapy.http.response import Response +from scrapy.utils.test import get_crawler from testfixtures import LogCapture @@ -71,7 +71,7 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_response_200(self): url = self.mockserver.url("/status?n=200") - crawler = CrawlerRunner().create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) yield crawler.crawl(seed=url, mockserver=self.mockserver) response = crawler.spider.meta["responses"][0] self.assertEqual(response.request.url, url) @@ -80,7 +80,7 @@ class CrawlTestCase(TestCase): def test_response_error(self): for status in ("404", "500"): url = self.mockserver.url(f"/status?n={status}") - crawler = CrawlerRunner().create_crawler(SingleRequestSpider) + crawler = get_crawler(SingleRequestSpider) yield crawler.crawl(seed=url, mockserver=self.mockserver) failure = crawler.spider.meta["failure"] response = failure.value.response @@ -90,12 +90,11 @@ class CrawlTestCase(TestCase): @defer.inlineCallbacks def test_downloader_middleware_raise_exception(self): url = self.mockserver.url("/status?n=200") - runner = CrawlerRunner(settings={ + crawler = get_crawler(SingleRequestSpider, { "DOWNLOADER_MIDDLEWARES": { RaiseExceptionRequestMiddleware: 590, }, }) - crawler = runner.create_crawler(SingleRequestSpider) yield crawler.crawl(seed=url, mockserver=self.mockserver) failure = crawler.spider.meta["failure"] self.assertEqual(failure.request.url, url) @@ -117,12 +116,11 @@ class CrawlTestCase(TestCase): signal_params["request"] = request url = self.mockserver.url("/status?n=200") - runner = CrawlerRunner(settings={ + crawler = get_crawler(SingleRequestSpider, { "DOWNLOADER_MIDDLEWARES": { ProcessResponseMiddleware: 595, } }) - crawler = runner.create_crawler(SingleRequestSpider) crawler.signals.connect(signal_handler, signal=signals.response_received) with LogCapture() as log: @@ -147,13 +145,12 @@ class CrawlTestCase(TestCase): The spider callback should receive the overridden response.request """ url = self.mockserver.url("/status?n=200") - runner = CrawlerRunner(settings={ + crawler = get_crawler(SingleRequestSpider, { "DOWNLOADER_MIDDLEWARES": { RaiseExceptionRequestMiddleware: 590, CatchExceptionOverrideRequestMiddleware: 595, }, }) - crawler = runner.create_crawler(SingleRequestSpider) yield crawler.crawl(seed=url, mockserver=self.mockserver) response = crawler.spider.meta["responses"][0] self.assertEqual(response.body, b"Caught ZeroDivisionError") @@ -168,13 +165,12 @@ class CrawlTestCase(TestCase): The spider callback should receive the original response.request """ url = self.mockserver.url("/status?n=200") - runner = CrawlerRunner(settings={ + crawler = get_crawler(SingleRequestSpider, { "DOWNLOADER_MIDDLEWARES": { RaiseExceptionRequestMiddleware: 590, CatchExceptionDoNotOverrideRequestMiddleware: 595, }, }) - crawler = runner.create_crawler(SingleRequestSpider) yield crawler.crawl(seed=url, mockserver=self.mockserver) response = crawler.spider.meta["responses"][0] self.assertEqual(response.body, b"Caught ZeroDivisionError") @@ -186,12 +182,11 @@ class CrawlTestCase(TestCase): Downloader middleware which returns a response with a specific 'request' attribute, with an alternative callback """ - runner = CrawlerRunner(settings={ + crawler = get_crawler(AlternativeCallbacksSpider, { "DOWNLOADER_MIDDLEWARES": { AlternativeCallbacksMiddleware: 595, } }) - crawler = runner.create_crawler(AlternativeCallbacksSpider) with LogCapture() as log: url = self.mockserver.url("/status?n=200") diff --git a/tests/test_request_cb_kwargs.py b/tests/test_request_cb_kwargs.py index 473a93e69..002a04358 100644 --- a/tests/test_request_cb_kwargs.py +++ b/tests/test_request_cb_kwargs.py @@ -3,7 +3,7 @@ from twisted.internet import defer from twisted.trial.unittest import TestCase from scrapy.http import Request -from scrapy.crawler import CrawlerRunner +from scrapy.utils.test import get_crawler from tests.spiders import MockServerSpider from tests.mockserver import MockServer @@ -140,14 +140,13 @@ class CallbackKeywordArgumentsTestCase(TestCase): def setUp(self): self.mockserver = MockServer() self.mockserver.__enter__() - self.runner = CrawlerRunner() def tearDown(self): self.mockserver.__exit__(None, None, None) @defer.inlineCallbacks def test_callback_kwargs(self): - crawler = self.runner.create_crawler(KeywordArgumentsSpider) + crawler = get_crawler(KeywordArgumentsSpider) with LogCapture() as log: yield crawler.crawl(mockserver=self.mockserver) self.assertTrue(all(crawler.spider.checks)) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 2d4bfa165..ac66056ba 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -52,6 +52,7 @@ class MockCrawler(Crawler): SCHEDULER_PRIORITY_QUEUE=priority_queue_cls, JOBDIR=jobdir, DUPEFILTER_CLASS='scrapy.dupefilters.BaseDupeFilter', + REQUEST_FINGERPRINTER_IMPLEMENTATION='VERSION', ) super().__init__(Spider, settings) self.engine = MockEngine(downloader=MockDownloader()) @@ -334,7 +335,7 @@ class TestIncompatibility(unittest.TestCase): SCHEDULER_PRIORITY_QUEUE='scrapy.pqueues.DownloaderAwarePriorityQueue', CONCURRENT_REQUESTS_PER_IP=1, ) - crawler = Crawler(Spider, settings) + crawler = get_crawler(Spider, settings) scheduler = Scheduler.from_crawler(crawler) spider = Spider(name='spider') scheduler.open(spider) diff --git a/tests/test_scheduler_base.py b/tests/test_scheduler_base.py index bf90b4320..fc234a83d 100644 --- a/tests/test_scheduler_base.py +++ b/tests/test_scheduler_base.py @@ -7,10 +7,10 @@ from twisted.internet import defer from twisted.trial.unittest import TestCase as TwistedTestCase from scrapy.core.scheduler import BaseScheduler -from scrapy.crawler import CrawlerRunner from scrapy.http import Request from scrapy.spiders import Spider -from scrapy.utils.request import request_fingerprint +from scrapy.utils.request import fingerprint +from scrapy.utils.test import get_crawler from tests.mockserver import MockServer @@ -21,13 +21,13 @@ URLS = [urljoin("https://example.org", p) for p in PATHS] class MinimalScheduler: def __init__(self) -> None: - self.requests: Dict[str, Request] = {} + self.requests: Dict[bytes, Request] = {} def has_pending_requests(self) -> bool: return bool(self.requests) def enqueue_request(self, request: Request) -> bool: - fp = request_fingerprint(request) + fp = fingerprint(request) if fp not in self.requests: self.requests[fp] = request return True @@ -147,9 +147,12 @@ class MinimalSchedulerCrawlTest(TwistedTestCase): @defer.inlineCallbacks def test_crawl(self): with MockServer() as mockserver: - settings = {"SCHEDULER": self.scheduler_cls} + settings = { + "SCHEDULER": self.scheduler_cls, + } with LogCapture() as log: - yield CrawlerRunner(settings).crawl(TestSpider, mockserver) + crawler = get_crawler(TestSpider, settings) + yield crawler.crawl(mockserver) for path in PATHS: self.assertIn(f"{{'path': '{path}'}}", str(log)) self.assertIn(f"'item_scraped_count': {len(PATHS)}", str(log)) diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index 8a35e9fd7..3719c7c9f 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -96,7 +96,10 @@ class SpiderLoaderTest(unittest.TestCase): def test_crawler_runner_loading(self): module = 'tests.test_spiderloader.test_spiders.spider1' - runner = CrawlerRunner({'SPIDER_MODULES': [module]}) + runner = CrawlerRunner({ + 'SPIDER_MODULES': [module], + 'REQUEST_FINGERPRINTER_IMPLEMENTATION': 'VERSION', + }) self.assertRaisesRegex(KeyError, 'Spider not found', runner.create_crawler, 'spider2') diff --git a/tests/test_utils_project.py b/tests/test_utils_project.py index 1ef4eeb14..46452415a 100644 --- a/tests/test_utils_project.py +++ b/tests/test_utils_project.py @@ -3,6 +3,7 @@ import os import tempfile import shutil import contextlib +import warnings from pytest import warns @@ -68,20 +69,21 @@ class GetProjectSettingsTestCase(unittest.TestCase): envvars = { 'SCRAPY_SETTINGS_MODULE': value, } - with set_env(**envvars), warns(None) as warnings: - settings = get_project_settings() - assert not warnings + with warnings.catch_warnings(): + warnings.simplefilter("error") + with set_env(**envvars): + settings = get_project_settings() + assert settings.get('SETTINGS_MODULE') == value def test_invalid_envvar(self): envvars = { 'SCRAPY_FOO': 'bar', } - with set_env(**envvars), warns(None) as warnings: - get_project_settings() - assert len(warnings) == 1 - assert warnings[0].category == ScrapyDeprecationWarning - assert str(warnings[0].message).endswith(': FOO') + with warns(ScrapyDeprecationWarning, match=': FOO') as record: + with set_env(**envvars): + get_project_settings() + assert len(record) == 1 def test_valid_and_invalid_envvars(self): value = 'tests.test_cmdline.settings' @@ -89,9 +91,8 @@ class GetProjectSettingsTestCase(unittest.TestCase): 'SCRAPY_FOO': 'bar', 'SCRAPY_SETTINGS_MODULE': value, } - with set_env(**envvars), warns(None) as warnings: - settings = get_project_settings() - assert len(warnings) == 1 - assert warnings[0].category == ScrapyDeprecationWarning - assert str(warnings[0].message).endswith(': FOO') + with warns(ScrapyDeprecationWarning, match=': FOO') as record: + with set_env(**envvars): + settings = get_project_settings() + assert len(record) == 1 assert settings.get('SETTINGS_MODULE') == value diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index e9edfee98..5ee772c0b 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -308,13 +308,22 @@ class RequestFingerprintTest(FingerprintTest): ), ) + def setUp(self) -> None: + warnings.simplefilter("ignore", ScrapyDeprecationWarning) + + def tearDown(self) -> None: + warnings.simplefilter("default", ScrapyDeprecationWarning) + @pytest.mark.xfail(reason='known bug kept for backward compatibility', strict=True) def test_part_separation(self): super().test_part_separation() + +class RequestFingerprintDeprecationTest(unittest.TestCase): + def test_deprecation_default_parameters(self): with pytest.warns(ScrapyDeprecationWarning) as warnings: - self.function(Request("http://www.example.com")) + request_fingerprint(Request("http://www.example.com")) messages = [str(warning.message) for warning in warnings] self.assertTrue( any( @@ -326,7 +335,7 @@ class RequestFingerprintTest(FingerprintTest): def test_deprecation_non_default_parameters(self): with pytest.warns(ScrapyDeprecationWarning) as warnings: - self.function(Request("http://www.example.com"), keep_fragments=True) + request_fingerprint(Request("http://www.example.com"), keep_fragments=True) messages = [str(warning.message) for warning in warnings] self.assertTrue( any( diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 0a09f6109..d20852e62 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -1,7 +1,9 @@ import os import unittest +import warnings from urllib.parse import urlparse +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response, TextResponse, HtmlResponse from scrapy.utils.python import to_bytes from scrapy.utils.response import (response_httprepr, open_in_browser, @@ -15,14 +17,21 @@ class ResponseUtilsTest(unittest.TestCase): dummy_response = TextResponse(url='http://example.org/', body=b'dummy_response') def test_response_httprepr(self): - r1 = Response("http://www.example.com") - self.assertEqual(response_httprepr(r1), b'HTTP/1.1 200 OK\r\n\r\n') + with warnings.catch_warnings(): + warnings.simplefilter("ignore", ScrapyDeprecationWarning) - r1 = Response("http://www.example.com", status=404, headers={"Content-type": "text/html"}, body=b"Some body") - self.assertEqual(response_httprepr(r1), b'HTTP/1.1 404 Not Found\r\nContent-Type: text/html\r\n\r\nSome body') + r1 = Response("http://www.example.com") + self.assertEqual(response_httprepr(r1), b'HTTP/1.1 200 OK\r\n\r\n') - r1 = Response("http://www.example.com", status=6666, headers={"Content-type": "text/html"}, body=b"Some body") - self.assertEqual(response_httprepr(r1), b'HTTP/1.1 6666 \r\nContent-Type: text/html\r\n\r\nSome body') + r1 = Response("http://www.example.com", status=404, + headers={"Content-type": "text/html"}, body=b"Some body") + self.assertEqual(response_httprepr(r1), + b'HTTP/1.1 404 Not Found\r\nContent-Type: text/html\r\n\r\nSome body') + + r1 = Response("http://www.example.com", status=6666, + headers={"Content-type": "text/html"}, body=b"Some body") + self.assertEqual(response_httprepr(r1), + b'HTTP/1.1 6666 \r\nContent-Type: text/html\r\n\r\nSome body') def test_open_in_browser(self): url = "http:///www.example.com/some/page.html" From f60c7ae768fa2f238ea6d7646098fe99ef8ad461 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Wed, 20 Jul 2022 14:01:22 +0500 Subject: [PATCH 54/54] Fixed heading levels in downloader middleware docs (#5567) --- docs/topics/downloader-middleware.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 29e350651..986da0476 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -955,7 +955,7 @@ default because HTTP specs say so. .. setting:: RETRY_PRIORITY_ADJUST RETRY_PRIORITY_ADJUST ---------------------- +^^^^^^^^^^^^^^^^^^^^^ Default: ``-1`` @@ -1119,7 +1119,7 @@ In order to use this parser: .. _support-for-new-robots-parser: Implementing support for a new parser -------------------------------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ You can implement support for a new robots.txt_ parser by subclassing the abstract base class :class:`~scrapy.robotstxt.RobotParser` and