From ada37c5409047291ee5852fb2220e8e256424402 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 6 Jul 2019 22:37:31 -0300 Subject: [PATCH 01/10] Export to multiple formats in a single crawl --- docs/topics/feed-exports.rst | 85 ++++++--- scrapy/commands/crawl.py | 23 +-- scrapy/commands/runspider.py | 20 +- scrapy/extensions/feedexport.py | 191 +++++++++++-------- scrapy/settings/default_settings.py | 3 +- scrapy/utils/conf.py | 68 ++++++- tests/test_commands.py | 32 +++- tests/test_feedexport.py | 273 ++++++++++++++++++++-------- tests/test_utils_conf.py | 50 ++++- 9 files changed, 511 insertions(+), 234 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 42f1cad90..6d6ba33c9 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -12,7 +12,7 @@ generating an "export file" with the scraped data (commonly called "export feed") to be consumed by other systems. Scrapy provides this functionality out of the box with the Feed Exports, which -allows you to generate a feed with the scraped items, using multiple +allows you to generate feeds with the scraped items, using multiple serialization formats and storage backends. .. _topics-feed-format: @@ -36,7 +36,7 @@ But you can also extend the supported format through the JSON ---- - * :setting:`FEED_FORMAT`: ``json`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``json`` * Exporter used: :class:`~scrapy.exporters.JsonItemExporter` * See :ref:`this warning ` if you're using JSON with large feeds. @@ -46,7 +46,7 @@ JSON JSON lines ---------- - * :setting:`FEED_FORMAT`: ``jsonlines`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``jsonlines`` * Exporter used: :class:`~scrapy.exporters.JsonLinesItemExporter` .. _topics-feed-format-csv: @@ -54,7 +54,7 @@ JSON lines CSV --- - * :setting:`FEED_FORMAT`: ``csv`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``csv`` * Exporter used: :class:`~scrapy.exporters.CsvItemExporter` * To specify columns to export and their order use :setting:`FEED_EXPORT_FIELDS`. Other feed exporters can also use this @@ -66,7 +66,7 @@ CSV XML --- - * :setting:`FEED_FORMAT`: ``xml`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``xml`` * Exporter used: :class:`~scrapy.exporters.XmlItemExporter` .. _topics-feed-format-pickle: @@ -74,7 +74,7 @@ XML Pickle ------ - * :setting:`FEED_FORMAT`: ``pickle`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``pickle`` * Exporter used: :class:`~scrapy.exporters.PickleItemExporter` .. _topics-feed-format-marshal: @@ -82,7 +82,7 @@ Pickle Marshal ------- - * :setting:`FEED_FORMAT`: ``marshal`` + * Value for the ``format`` key in the :setting:`FEEDS` setting: ``marshal`` * Exporter used: :class:`~scrapy.exporters.MarshalItemExporter` @@ -91,8 +91,8 @@ Marshal Storages ======== -When using the feed exports you define where to store the feed using a URI_ -(through the :setting:`FEED_URI` setting). The feed exports supports multiple +When using the feed exports you define where to store the feed using one or multiple URIs_ +(through the :setting:`FEEDS` setting). The feed exports supports multiple storage backend types which are defined by the URI scheme. The storages backends supported out of the box are: @@ -211,41 +211,66 @@ Settings These are the settings used for configuring the feed exports: - * :setting:`FEED_URI` (mandatory) - * :setting:`FEED_FORMAT` + * :setting:`FEEDS` (mandatory) + * :setting:`FEED_EXPORT_ENCODING` + * :setting:`FEED_STORE_EMPTY` + * :setting:`FEED_EXPORT_FIELDS` + * :setting:`FEED_EXPORT_INDENT` * :setting:`FEED_STORAGES` * :setting:`FEED_STORAGE_FTP_ACTIVE` * :setting:`FEED_STORAGE_S3_ACL` * :setting:`FEED_EXPORTERS` - * :setting:`FEED_STORE_EMPTY` - * :setting:`FEED_EXPORT_ENCODING` - * :setting:`FEED_EXPORT_FIELDS` - * :setting:`FEED_EXPORT_INDENT` .. currentmodule:: scrapy.extensions.feedexport -.. setting:: FEED_URI +.. setting:: FEEDS -FEED_URI --------- +FEEDS +----- -Default: ``None`` +.. versionadded:: 2.1 -The URI of the export feed. See :ref:`topics-feed-storage-backends` for -supported URI schemes. +Default: ``{}`` -This setting is required for enabling the feed exports. +A dictionary in which every key is a feed URI (or a :class:`pathlib.Path` +object) and each value is a nested dictionary containing configuration +parameters for the specific feed. +This setting is required for enabling the feed export feature. -.. versionchanged:: 2.0 - Added :class:`pathlib.Path` support. +See :ref:`topics-feed-storage-backends` for supported URI schemes. -.. setting:: FEED_FORMAT +For instance:: -FEED_FORMAT ------------ + { + 'items.json': { + 'format': 'json', + 'encoding': 'utf8', + 'store_empty': False, + 'fields': None, + 'indent': 4, + }, + 'items.xml': { + 'format': 'xml', + 'fields': ['name', 'price'], + 'encoding': 'latin1', + 'indent': 8, + }, + pathlib.Path('items.csv'): { + 'format': 'csv', + 'fields': ['price', 'name'], + }, + } -The serialization format to be used for the feed. See -:ref:`topics-feed-format` for possible values. +The following is a list of the accepted keys and the setting that is used +as a fallback value if that key is not provided for a specific feed definition. + +* ``format``: the serialization format to be used for the feed. + See :ref:`topics-feed-format` for possible values. + Mandatory, no fallback setting +* ``encoding``: falls back to :setting:`FEED_EXPORT_ENCODING` +* ``fields``: falls back to :setting:`FEED_EXPORT_FIELDS` +* ``indent``: falls back to :setting:`FEED_EXPORT_INDENT` +* ``store_empty``: falls back to :setting:`FEED_STORE_EMPTY` .. setting:: FEED_EXPORT_ENCODING @@ -400,7 +425,7 @@ format in :setting:`FEED_EXPORTERS`. E.g., to disable the built-in CSV exporter 'csv': None, } -.. _URI: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier +.. _URIs: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier .. _Amazon S3: https://aws.amazon.com/s3/ .. _botocore: https://github.com/boto/botocore .. _Canned ACL: https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index 7b417e2eb..4b2f9484b 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -1,7 +1,5 @@ -import os from scrapy.commands import ScrapyCommand -from scrapy.utils.conf import arglist_to_dict -from scrapy.utils.python import without_none_values +from scrapy.utils.conf import arglist_to_dict, feed_process_params_from_cli from scrapy.exceptions import UsageError @@ -19,7 +17,7 @@ class Command(ScrapyCommand): ScrapyCommand.add_options(self, parser) parser.add_option("-a", dest="spargs", action="append", default=[], metavar="NAME=VALUE", help="set spider argument (may be repeated)") - parser.add_option("-o", "--output", metavar="FILE", + parser.add_option("-o", "--output", metavar="FILE", action="append", help="dump scraped items into FILE (use - for stdout)") parser.add_option("-t", "--output-format", metavar="FORMAT", help="format to use for dumping items with -o") @@ -31,21 +29,8 @@ class Command(ScrapyCommand): except ValueError: raise UsageError("Invalid -a value, use -a NAME=VALUE", print_help=False) if opts.output: - if opts.output == '-': - self.settings.set('FEED_URI', 'stdout:', priority='cmdline') - else: - self.settings.set('FEED_URI', opts.output, priority='cmdline') - feed_exporters = without_none_values( - self.settings.getwithbase('FEED_EXPORTERS')) - valid_output_formats = feed_exporters.keys() - if not opts.output_format: - opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") - if opts.output_format not in valid_output_formats: - raise UsageError("Unrecognized output format '%s', set one" - " using the '-t' switch or as a file extension" - " from the supported list %s" % (opts.output_format, - tuple(valid_output_formats))) - self.settings.set('FEED_FORMAT', opts.output_format, priority='cmdline') + feeds = feed_process_params_from_cli(self.settings, opts.output, opts.output_format) + self.settings.set('FEEDS', feeds, priority='cmdline') def run(self, args, opts): if len(args) < 1: diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 57d8471ca..62510609a 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -5,8 +5,7 @@ from importlib import import_module from scrapy.utils.spider import iter_spider_classes from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError -from scrapy.utils.conf import arglist_to_dict -from scrapy.utils.python import without_none_values +from scrapy.utils.conf import arglist_to_dict, feed_process_params_from_cli def _import_file(filepath): @@ -43,7 +42,7 @@ class Command(ScrapyCommand): ScrapyCommand.add_options(self, parser) parser.add_option("-a", dest="spargs", action="append", default=[], metavar="NAME=VALUE", help="set spider argument (may be repeated)") - parser.add_option("-o", "--output", metavar="FILE", + parser.add_option("-o", "--output", metavar="FILE", action="append", help="dump scraped items into FILE (use - for stdout)") parser.add_option("-t", "--output-format", metavar="FORMAT", help="format to use for dumping items with -o") @@ -55,19 +54,8 @@ class Command(ScrapyCommand): except ValueError: raise UsageError("Invalid -a value, use -a NAME=VALUE", print_help=False) if opts.output: - if opts.output == '-': - self.settings.set('FEED_URI', 'stdout:', priority='cmdline') - else: - self.settings.set('FEED_URI', opts.output, priority='cmdline') - feed_exporters = without_none_values(self.settings.getwithbase('FEED_EXPORTERS')) - if not opts.output_format: - opts.output_format = os.path.splitext(opts.output)[1].replace(".", "") - if opts.output_format not in feed_exporters: - raise UsageError("Unrecognized output format '%s', set one" - " using the '-t' switch or as a file extension" - " from the supported list %s" % (opts.output_format, - tuple(feed_exporters))) - self.settings.set('FEED_FORMAT', opts.output_format, priority='cmdline') + feeds = feed_process_params_from_cli(self.settings, opts.output, opts.output_format) + self.settings.set('FEEDS', feeds, priority='cmdline') def run(self, args, opts): if len(args) != 1: diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index f1b101780..108b6d35c 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -4,24 +4,27 @@ Feed Exports extension See documentation in docs/topics/feed-exports.rst """ +import logging import os import sys -import logging -from tempfile import NamedTemporaryFile +import warnings from datetime import datetime -from urllib.parse import urlparse, unquote +from tempfile import NamedTemporaryFile +from urllib.parse import unquote, urlparse -from zope.interface import Interface, implementer from twisted.internet import defer, threads from w3lib.url import file_uri_to_path +from zope.interface import implementer, Interface from scrapy import signals -from scrapy.utils.ftp import ftp_store_file -from scrapy.exceptions import NotConfigured -from scrapy.utils.misc import create_instance, load_object -from scrapy.utils.log import failure_to_exc_info -from scrapy.utils.python import without_none_values +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.utils.boto import is_botocore +from scrapy.utils.conf import feed_complete_default_values_from_settings +from scrapy.utils.ftp import ftp_store_file +from scrapy.utils.log import failure_to_exc_info +from scrapy.utils.misc import create_instance, load_object +from scrapy.utils.python import without_none_values + logger = logging.getLogger(__name__) @@ -98,8 +101,6 @@ class S3FeedStorage(BlockingFeedStorage): from scrapy.utils.project import get_project_settings settings = get_project_settings() if 'AWS_ACCESS_KEY_ID' in settings or 'AWS_SECRET_ACCESS_KEY' in settings: - import warnings - from scrapy.exceptions import ScrapyDeprecationWarning warnings.warn( "Initialising `scrapy.extensions.feedexport.S3FeedStorage` " "without AWS keys is deprecated. Please supply credentials or " @@ -178,88 +179,117 @@ class FTPFeedStorage(BlockingFeedStorage): ) -class SpiderSlot(object): - def __init__(self, file, exporter, storage, uri): +class _FeedSlot(object): + def __init__(self, file, exporter, storage, uri, format, store_empty): self.file = file self.exporter = exporter self.storage = storage + # feed params self.uri = uri + self.format = format + self.store_empty = store_empty + # flags self.itemcount = 0 + self._exporting = False + + def start_exporting(self): + if not self._exporting: + self.exporter.start_exporting() + self._exporting = True + + def finish_exporting(self): + if self._exporting: + self.exporter.finish_exporting() + self._exporting = False class FeedExporter(object): - def __init__(self, settings): - self.settings = settings - if not settings['FEED_URI']: - raise NotConfigured - self.urifmt = str(settings['FEED_URI']) - self.format = settings['FEED_FORMAT'].lower() - self.export_encoding = settings['FEED_EXPORT_ENCODING'] - self.storages = self._load_components('FEED_STORAGES') - self.exporters = self._load_components('FEED_EXPORTERS') - if not self._storage_supported(self.urifmt): - raise NotConfigured - if not self._exporter_supported(self.format): - raise NotConfigured - self.store_empty = settings.getbool('FEED_STORE_EMPTY') - self._exporting = False - self.export_fields = settings.getlist('FEED_EXPORT_FIELDS') or None - self.indent = None - if settings.get('FEED_EXPORT_INDENT') is not None: - self.indent = settings.getint('FEED_EXPORT_INDENT') - uripar = settings['FEED_URI_PARAMS'] - self._uripar = load_object(uripar) if uripar else lambda x, y: None - @classmethod def from_crawler(cls, crawler): - o = cls(crawler.settings) - o.crawler = crawler - crawler.signals.connect(o.open_spider, signals.spider_opened) - crawler.signals.connect(o.close_spider, signals.spider_closed) - crawler.signals.connect(o.item_scraped, signals.item_scraped) - return o + exporter = cls(crawler) + crawler.signals.connect(exporter.open_spider, signals.spider_opened) + crawler.signals.connect(exporter.close_spider, signals.spider_closed) + crawler.signals.connect(exporter.item_scraped, signals.item_scraped) + return exporter + + def __init__(self, crawler): + self.crawler = crawler + self.settings = crawler.settings + self.feeds = {} + self.slots = [] + + if not self.settings['FEEDS'] and not self.settings['FEED_URI']: + raise NotConfigured + + # Begin: Backward compatibility for FEED_URI and FEED_FORMAT settings + if self.settings['FEED_URI']: + warnings.warn( + 'The `FEED_URI` and `FEED_FORMAT` settings have been deprecated in favor of ' + 'the `FEEDS` setting. Please see the `FEEDS` setting docs for more details', + category=ScrapyDeprecationWarning, stacklevel=2, + ) + uri = str(self.settings['FEED_URI']) # handle pathlib.Path objects + feed = {'format': self.settings.get('FEED_FORMAT', 'jsonlines')} + self.feeds[uri] = feed_complete_default_values_from_settings(feed, self.settings) + # End: Backward compatibility for FEED_URI and FEED_FORMAT settings + + # 'FEEDS' setting takes precedence over 'FEED_URI' + for uri, feed in self.settings.getdict('FEEDS').items(): + uri = str(uri) # handle pathlib.Path objects + self.feeds[uri] = feed_complete_default_values_from_settings(feed, self.settings) + + self.storages = self._load_components('FEED_STORAGES') + self.exporters = self._load_components('FEED_EXPORTERS') + for uri, feed in self.feeds.items(): + if not self._storage_supported(uri): + raise NotConfigured + if not self._exporter_supported(feed['format']): + raise NotConfigured def open_spider(self, spider): - uri = self.urifmt % self._get_uri_params(spider) - storage = self._get_storage(uri) - file = storage.open(spider) - exporter = self._get_exporter(file, fields_to_export=self.export_fields, - encoding=self.export_encoding, indent=self.indent) - if self.store_empty: - exporter.start_exporting() - self._exporting = True - self.slot = SpiderSlot(file, exporter, storage, uri) + for uri, feed in self.feeds.items(): + uri = uri % self._get_uri_params(spider, feed['uri_params']) + storage = self._get_storage(uri) + file = storage.open(spider) + exporter = self._get_exporter( + file=file, + format=feed['format'], + fields_to_export=feed['fields'], + encoding=feed['encoding'], + indent=feed['indent'], + ) + slot = _FeedSlot(file, exporter, storage, uri, feed['format'], feed['store_empty']) + self.slots.append(slot) + if slot.store_empty: + slot.start_exporting() def close_spider(self, spider): - slot = self.slot - if not slot.itemcount and not self.store_empty: - # We need to call slot.storage.store nonetheless to get the file - # properly closed. - return defer.maybeDeferred(slot.storage.store, slot.file) - if self._exporting: - slot.exporter.finish_exporting() - self._exporting = False - logfmt = "%s %%(format)s feed (%%(itemcount)d items) in: %%(uri)s" - log_args = {'format': self.format, - 'itemcount': slot.itemcount, - 'uri': slot.uri} - d = defer.maybeDeferred(slot.storage.store, slot.file) - d.addCallback(lambda _: logger.info(logfmt % "Stored", log_args, - extra={'spider': spider})) - d.addErrback(lambda f: logger.error(logfmt % "Error storing", log_args, - exc_info=failure_to_exc_info(f), - extra={'spider': spider})) - return d + deferred_list = [] + for slot in self.slots: + if not slot.itemcount and not slot.store_empty: + # We need to call slot.storage.store nonetheless to get the file + # properly closed. + return defer.maybeDeferred(slot.storage.store, slot.file) + slot.finish_exporting() + logfmt = "%s %%(format)s feed (%%(itemcount)d items) in: %%(uri)s" + log_args = {'format': slot.format, + 'itemcount': slot.itemcount, + 'uri': slot.uri} + d = defer.maybeDeferred(slot.storage.store, slot.file) + d.addCallback(lambda _: logger.info(logfmt % "Stored", log_args, + extra={'spider': spider})) + d.addErrback(lambda f: logger.error(logfmt % "Error storing", log_args, + exc_info=failure_to_exc_info(f), + extra={'spider': spider})) + deferred_list.append(d) + return defer.DeferredList(deferred_list) if deferred_list else None def item_scraped(self, item, spider): - slot = self.slot - if not self._exporting: - slot.exporter.start_exporting() - self._exporting = True - slot.exporter.export_item(item) - slot.itemcount += 1 - return item + for slot in self.slots: + slot.start_exporting() + slot.exporter.export_item(item) + slot.itemcount += 1 def _load_components(self, setting_prefix): conf = without_none_values(self.settings.getwithbase(setting_prefix)) @@ -295,17 +325,18 @@ class FeedExporter(object): objcls, self.settings, getattr(self, 'crawler', None), *args, **kwargs) - def _get_exporter(self, *args, **kwargs): - return self._get_instance(self.exporters[self.format], *args, **kwargs) + def _get_exporter(self, file, format, *args, **kwargs): + return self._get_instance(self.exporters[format], file, *args, **kwargs) def _get_storage(self, uri): return self._get_instance(self.storages[urlparse(uri).scheme], uri) - def _get_uri_params(self, spider): + def _get_uri_params(self, spider, uri_params): params = {} for k in dir(spider): params[k] = getattr(spider, k) ts = datetime.utcnow().replace(microsecond=0).isoformat().replace(':', '-') params['time'] = ts - self._uripar(params, spider) + uripar_function = load_object(uri_params) if uri_params else lambda x, y: None + uripar_function(params, spider) return params diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index f8a0457ce..077317c81 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -133,9 +133,8 @@ EXTENSIONS_BASE = { } FEED_TEMPDIR = None -FEED_URI = None +FEEDS = {} FEED_URI_PARAMS = None # a function to extend uri arguments -FEED_FORMAT = 'jsonlines' FEED_STORE_EMPTY = False FEED_EXPORT_ENCODING = None FEED_EXPORT_FIELDS = None diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index 23306ca28..e01027491 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -1,9 +1,12 @@ +import numbers import os import sys -import numbers +import warnings from configparser import ConfigParser from operator import itemgetter +from scrapy.exceptions import ScrapyDeprecationWarning, UsageError + from scrapy.settings import BaseSettings from scrapy.utils.deprecate import update_classpath from scrapy.utils.python import without_none_values @@ -106,3 +109,66 @@ def get_sources(use_closest=True): if use_closest: sources.append(closest_scrapy_cfg()) return sources + + +def feed_complete_default_values_from_settings(feed, settings): + out = feed.copy() + if 'encoding' not in out: + out['encoding'] = settings['FEED_EXPORT_ENCODING'] + if 'fields' not in out: + out['fields'] = settings.getlist('FEED_EXPORT_FIELDS') or None + if 'indent' not in out: + out['indent'] = None if settings['FEED_EXPORT_INDENT'] is None else settings.getint('FEED_EXPORT_INDENT') + if 'store_empty' not in out: + out['store_empty'] = settings.getbool('FEED_STORE_EMPTY') + if 'uri_params' not in out: + out['uri_params'] = settings['FEED_URI_PARAMS'] + return out + + +def feed_process_params_from_cli(settings, output, output_format=None): + """ + Receives feed export params (from the 'crawl' or 'runspider' commands), + checks for inconsistencies in their quantities and returns a dictionary + suitable to be used as the FEEDS setting. + """ + valid_output_formats = without_none_values( + settings.getwithbase('FEED_EXPORTERS') + ).keys() + + def check_valid_format(output_format): + if output_format not in valid_output_formats: + raise UsageError("Unrecognized output format '%s', set one after a" + " colon using the -o option (i.e. -o :)" + " or as a file extension, from the supported list %s" % + (output_format, tuple(valid_output_formats))) + + if output_format: + if len(output) == 1: + check_valid_format(output_format) + warnings.warn('The -t command line option is deprecated in favor' + ' of specifying the output format within the -o' + ' option, please check the -o option docs for more details', + category=ScrapyDeprecationWarning, stacklevel=2) + return {output[0]: {'format': output_format}} + else: + raise UsageError('The -t command line option cannot be used if multiple' + ' output files are specified with the -o option') + + result = {} + for element in output: + try: + feed_uri, feed_format = element.rsplit(':', 1) + except ValueError: + feed_uri = element + feed_format = os.path.splitext(element)[1].replace('.', '') + else: + if feed_uri == '-': + feed_uri = 'stdout:' + check_valid_format(feed_format) + result[feed_uri] = {'format': feed_format} + + # FEEDS setting should take precedence over the -o and -t CLI options + result.update(settings.getdict('FEEDS')) + + return result diff --git a/tests/test_commands.py b/tests/test_commands.py index 3612b70c9..24a341759 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,22 +1,46 @@ import inspect +import json +import optparse import os -import sys import subprocess +import sys import tempfile +from contextlib import contextmanager from os.path import exists, join, abspath from shutil import rmtree, copytree from tempfile import mkdtemp -from contextlib import contextmanager from threading import Timer from twisted.trial import unittest import scrapy +from scrapy.commands import ScrapyCommand +from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import get_testenv + from tests.test_crawler import ExceptionSpider, NoRequestsSpider +class CommandSettings(unittest.TestCase): + + def setUp(self): + self.command = ScrapyCommand() + self.command.settings = Settings() + self.parser = optparse.OptionParser( + formatter=optparse.TitledHelpFormatter(), + conflict_handler='resolve', + ) + self.command.add_options(self.parser) + + def test_settings_json_string(self): + feeds_json = '{"data.json": {"format": "json"}, "data.xml": {"format": "xml"}}' + opts, args = self.parser.parse_args(args=['-s', 'FEEDS={}'.format(feeds_json), 'spider.py']) + self.command.process_options(args, opts) + self.assertIsInstance(self.command.settings['FEEDS'], scrapy.settings.BaseSettings) + self.assertEqual(dict(self.command.settings['FEEDS']), json.loads(feeds_json)) + + class ProjectTest(unittest.TestCase): project_name = 'testproject' @@ -34,7 +58,7 @@ class ProjectTest(unittest.TestCase): with tempfile.TemporaryFile() as out: args = (sys.executable, '-m', 'scrapy.cmdline') + new_args return subprocess.call(args, stdout=out, stderr=out, cwd=self.cwd, - env=self.env, **kwargs) + env=self.env, **kwargs) def proc(self, *new_args, **popen_kwargs): args = (sys.executable, '-m', 'scrapy.cmdline') + new_args @@ -310,6 +334,6 @@ class BenchCommandTest(CommandTest): def test_run(self): _, _, log = self.proc('bench', '-s', 'LOGSTATS_INTERVAL=0.001', - '-s', 'CLOSESPIDER_TIMEOUT=0.01') + '-s', 'CLOSESPIDER_TIMEOUT=0.01') self.assertIn('INFO: Crawled', log) self.assertNotIn('Unhandled Error', log) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 2ca57c19d..08e8dfc41 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1,32 +1,34 @@ -import os import csv import json -import warnings -import tempfile +import os +import random import shutil import string +import tempfile +import warnings from io import BytesIO from pathlib import Path +from string import ascii_letters, digits from unittest import mock from urllib.parse import urljoin, urlparse, quote from urllib.request import pathname2url -from zope.interface.verify import verifyObject -from twisted.trial import unittest +import lxml.etree from twisted.internet import defer -from scrapy.crawler import CrawlerRunner -from scrapy.settings import Settings -from tests.mockserver import MockServer +from twisted.trial import unittest from w3lib.url import path_to_file_uri +from zope.interface.verify import verifyObject import scrapy +from scrapy.crawler import CrawlerRunner from scrapy.exporters import CsvItemExporter -from scrapy.extensions.feedexport import ( - IFeedStorage, FileFeedStorage, FTPFeedStorage, - S3FeedStorage, StdoutFeedStorage, - BlockingFeedStorage) -from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete, get_crawler +from scrapy.extensions.feedexport import (BlockingFeedStorage, FileFeedStorage, FTPFeedStorage, + IFeedStorage, S3FeedStorage, StdoutFeedStorage) +from scrapy.settings import Settings from scrapy.utils.python import to_unicode +from scrapy.utils.test import assert_aws_environ, get_crawler, get_s3_content_and_delete + +from tests.mockserver import MockServer class FileFeedStorageTest(unittest.TestCase): @@ -395,29 +397,41 @@ class FeedExportTest(unittest.TestCase): egg = scrapy.Field() baz = scrapy.Field() + def setUp(self): + self.temp_dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.temp_dir, ignore_errors=True) + + def _random_temp_filename(self): + chars = [random.choice(ascii_letters + digits) for _ in range(15)] + filename = ''.join(chars) + return os.path.join(self.temp_dir, filename) + @defer.inlineCallbacks - def run_and_export(self, spider_cls, settings=None): + def run_and_export(self, spider_cls, settings): """ Run spider with specified settings; return exported data. """ - tmpdir = tempfile.mkdtemp() - res_path = os.path.join(tmpdir, 'res') - res_uri = urljoin('file:', pathname2url(res_path)) - defaults = { - 'FEED_URI': res_uri, - 'FEED_FORMAT': 'csv', - 'FEED_PATH': res_path + + FEEDS = settings.get('FEEDS') or {} + settings['FEEDS'] = { + urljoin('file:', pathname2url(str(file_path))): feed + for file_path, feed in FEEDS.items() } - defaults.update(settings or {}) + + content = {} try: with MockServer() as s: - runner = CrawlerRunner(Settings(defaults)) + runner = CrawlerRunner(Settings(settings)) spider_cls.start_urls = [s.url('/')] yield runner.crawl(spider_cls) - with open(str(defaults['FEED_PATH']), 'rb') as f: - content = f.read() + for file_path, feed in FEEDS.items(): + with open(str(file_path), 'rb') as f: + content[feed['format']] = f.read() finally: - shutil.rmtree(tmpdir) + for file_path in FEEDS.keys(): + os.remove(str(file_path)) defer.returnValue(content) @@ -453,10 +467,14 @@ class FeedExportTest(unittest.TestCase): @defer.inlineCallbacks def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): settings = settings or {} - settings.update({'FEED_FORMAT': 'csv'}) + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'csv'}, + }, + }) data = yield self.exported_data(items, settings) - reader = csv.DictReader(to_unicode(data).splitlines()) + reader = csv.DictReader(to_unicode(data['csv']).splitlines()) got_rows = list(reader) if ordered: self.assertEqual(reader.fieldnames, header) @@ -468,51 +486,87 @@ class FeedExportTest(unittest.TestCase): @defer.inlineCallbacks def assertExportedJsonLines(self, items, rows, settings=None): settings = settings or {} - settings.update({'FEED_FORMAT': 'jl'}) + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'jl'}, + }, + }) data = yield self.exported_data(items, settings) - parsed = [json.loads(to_unicode(line)) for line in data.splitlines()] + parsed = [json.loads(to_unicode(line)) for line in data['jl'].splitlines()] rows = [{k: v for k, v in row.items() if v} for row in rows] self.assertEqual(rows, parsed) @defer.inlineCallbacks def assertExportedXml(self, items, rows, settings=None): settings = settings or {} - settings.update({'FEED_FORMAT': 'xml'}) + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'xml'}, + }, + }) data = yield self.exported_data(items, settings) rows = [{k: v for k, v in row.items() if v} for row in rows] - import lxml.etree - root = lxml.etree.fromstring(data) + root = lxml.etree.fromstring(data['xml']) got_rows = [{e.tag: e.text for e in it} for it in root.findall('item')] self.assertEqual(rows, got_rows) + @defer.inlineCallbacks + def assertExportedMultiple(self, items, rows, settings=None): + settings = settings or {} + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'xml'}, + self._random_temp_filename(): {'format': 'json'}, + }, + }) + data = yield self.exported_data(items, settings) + rows = [{k: v for k, v in row.items() if v} for row in rows] + # XML + root = lxml.etree.fromstring(data['xml']) + xml_rows = [{e.tag: e.text for e in it} for it in root.findall('item')] + self.assertEqual(rows, xml_rows) + # JSON + json_rows = json.loads(to_unicode(data['json'])) + self.assertEqual(rows, json_rows) + def _load_until_eof(self, data, load_func): - bytes_output = BytesIO(data) result = [] - while True: - try: - result.append(load_func(bytes_output)) - except EOFError: - break + with tempfile.TemporaryFile() as temp: + temp.write(data) + temp.seek(0) + while True: + try: + result.append(load_func(temp)) + except EOFError: + break return result @defer.inlineCallbacks def assertExportedPickle(self, items, rows, settings=None): settings = settings or {} - settings.update({'FEED_FORMAT': 'pickle'}) + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'pickle'}, + }, + }) data = yield self.exported_data(items, settings) expected = [{k: v for k, v in row.items() if v} for row in rows] import pickle - result = self._load_until_eof(data, load_func=pickle.load) + result = self._load_until_eof(data['pickle'], load_func=pickle.load) self.assertEqual(expected, result) @defer.inlineCallbacks def assertExportedMarshal(self, items, rows, settings=None): settings = settings or {} - settings.update({'FEED_FORMAT': 'marshal'}) + settings.update({ + 'FEEDS': { + self._random_temp_filename(): {'format': 'marshal'}, + }, + }) data = yield self.exported_data(items, settings) expected = [{k: v for k, v in row.items() if v} for row in rows] import marshal - result = self._load_until_eof(data, load_func=marshal.load) + result = self._load_until_eof(data['marshal'], load_func=marshal.load) self.assertEqual(expected, result) @defer.inlineCallbacks @@ -521,6 +575,8 @@ class FeedExportTest(unittest.TestCase): yield self.assertExportedJsonLines(items, rows, settings) yield self.assertExportedXml(items, rows, settings) yield self.assertExportedPickle(items, rows, settings) + yield self.assertExportedMarshal(items, rows, settings) + yield self.assertExportedMultiple(items, rows, settings) @defer.inlineCallbacks def test_export_items(self): @@ -538,15 +594,14 @@ class FeedExportTest(unittest.TestCase): @defer.inlineCallbacks def test_export_no_items_not_store_empty(self): - formats = ('json', - 'jsonlines', - 'xml', - 'csv',) - - for fmt in formats: - settings = {'FEED_FORMAT': fmt} + for fmt in ('json', 'jsonlines', 'xml', 'csv'): + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': fmt}, + }, + } data = yield self.exported_no_data(settings) - self.assertEqual(data, b'') + self.assertEqual(data[fmt], b'') @defer.inlineCallbacks def test_export_no_items_store_empty(self): @@ -558,9 +613,15 @@ class FeedExportTest(unittest.TestCase): ) for fmt, expctd in formats: - settings = {'FEED_FORMAT': fmt, 'FEED_STORE_EMPTY': True, 'FEED_EXPORT_INDENT': None} + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': fmt}, + }, + 'FEED_STORE_EMPTY': True, + 'FEED_EXPORT_INDENT': None, + } data = yield self.exported_no_data(settings) - self.assertEqual(data, expctd) + self.assertEqual(data[fmt], expctd) @defer.inlineCallbacks def test_export_multiple_item_classes(self): @@ -581,9 +642,9 @@ class FeedExportTest(unittest.TestCase): header = self.MyItem.fields.keys() rows_csv = [ {'egg': 'spam1', 'foo': 'bar1', 'baz': ''}, - {'egg': '', 'foo': 'bar2', 'baz': ''}, + {'egg': '', 'foo': 'bar2', 'baz': ''}, {'egg': 'spam3', 'foo': 'bar3', 'baz': 'quux3'}, - {'egg': 'spam4', 'foo': '', 'baz': ''}, + {'egg': 'spam4', 'foo': '', 'baz': ''}, ] rows_jl = [dict(row) for row in items] yield self.assertExportedCsv(items, header, rows_csv, ordered=False) @@ -598,10 +659,10 @@ class FeedExportTest(unittest.TestCase): header = ["foo", "baz", "hello"] settings = {'FEED_EXPORT_FIELDS': header} rows = [ - {'foo': 'bar1', 'baz': '', 'hello': ''}, - {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, + {'foo': 'bar1', 'baz': '', 'hello': ''}, + {'foo': 'bar2', 'baz': '', 'hello': 'world2'}, {'foo': 'bar3', 'baz': 'quux3', 'hello': ''}, - {'foo': '', 'baz': '', 'hello': 'world4'}, + {'foo': '', 'baz': '', 'hello': 'world4'}, ] yield self.assertExported(items, header, rows, settings=settings, ordered=True) @@ -663,10 +724,15 @@ class FeedExportTest(unittest.TestCase): 'csv': u'foo\r\nTest\xd6\r\n'.encode('utf-8'), } - for format, expected in formats.items(): - settings = {'FEED_FORMAT': format, 'FEED_EXPORT_INDENT': None} + for fmt, expected in formats.items(): + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': fmt}, + }, + 'FEED_EXPORT_INDENT': None, + } data = yield self.exported_data(items, settings) - self.assertEqual(expected, data) + self.assertEqual(expected, data[fmt]) formats = { 'json': u'[{"foo": "Test\xd6"}]'.encode('latin-1'), @@ -675,11 +741,53 @@ class FeedExportTest(unittest.TestCase): 'csv': u'foo\r\nTest\xd6\r\n'.encode('latin-1'), } - settings = {'FEED_EXPORT_INDENT': None, 'FEED_EXPORT_ENCODING': 'latin-1'} - for format, expected in formats.items(): - settings['FEED_FORMAT'] = format + for fmt, expected in formats.items(): + settings = { + 'FEEDS': { + self._random_temp_filename(): {'format': fmt}, + }, + 'FEED_EXPORT_INDENT': None, + 'FEED_EXPORT_ENCODING': 'latin-1', + } data = yield self.exported_data(items, settings) - self.assertEqual(expected, data) + self.assertEqual(expected, data[fmt]) + + @defer.inlineCallbacks + def test_export_multiple_configs(self): + items = [dict({'foo': u'FOO', 'bar': u'BAR'})] + + formats = { + 'json': u'[\n{"bar": "BAR"}\n]'.encode('utf-8'), + 'xml': u'\n\n \n FOO\n \n'.encode('latin-1'), + 'csv': u'bar,foo\r\nBAR,FOO\r\n'.encode('utf-8'), + } + + settings = { + 'FEEDS': { + self._random_temp_filename(): { + 'format': 'json', + 'indent': 0, + 'fields': ['bar'], + 'encoding': 'utf-8', + }, + self._random_temp_filename(): { + 'format': 'xml', + 'indent': 2, + 'fields': ['foo'], + 'encoding': 'latin-1', + }, + self._random_temp_filename(): { + 'format': 'csv', + 'indent': None, + 'fields': ['bar', 'foo'], + 'encoding': 'utf-8', + }, + }, + } + + data = yield self.exported_data(items, settings) + for fmt, expected in formats.items(): + self.assertEqual(expected, data[fmt]) @defer.inlineCallbacks def test_export_indentation(self): @@ -827,33 +935,38 @@ class FeedExportTest(unittest.TestCase): ] for row in test_cases: - settings = {'FEED_FORMAT': row['format'], 'FEED_EXPORT_INDENT': row['indent']} + settings = { + 'FEEDS': { + self._random_temp_filename(): { + 'format': row['format'], + 'indent': row['indent'], + }, + }, + } data = yield self.exported_data(items, settings) - print(row['format'], row['indent']) - self.assertEqual(row['expected'], data) + self.assertEqual(row['expected'], data[row['format']]) @defer.inlineCallbacks def test_init_exporters_storages_with_crawler(self): settings = { - 'FEED_EXPORTERS': {'csv': 'tests.test_feedexport.' - 'FromCrawlerCsvItemExporter'}, - 'FEED_STORAGES': {'file': 'tests.test_feedexport.' - 'FromCrawlerFileFeedStorage'}, + 'FEED_EXPORTERS': {'csv': 'tests.test_feedexport.FromCrawlerCsvItemExporter'}, + 'FEED_STORAGES': {'file': 'tests.test_feedexport.FromCrawlerFileFeedStorage'}, + 'FEEDS': { + self._random_temp_filename(): {'format': 'csv'}, + }, } - yield self.exported_data({}, settings) + yield self.exported_data(items=[], settings=settings) self.assertTrue(FromCrawlerCsvItemExporter.init_with_crawler) self.assertTrue(FromCrawlerFileFeedStorage.init_with_crawler) @defer.inlineCallbacks def test_pathlib_uri(self): - tmpdir = tempfile.mkdtemp() - feed_uri = Path(tmpdir) / 'res' + feed_path = Path(self._random_temp_filename()) settings = { - 'FEED_FORMAT': 'csv', 'FEED_STORE_EMPTY': True, - 'FEED_URI': feed_uri, - 'FEED_PATH': feed_uri + 'FEEDS': { + feed_path: {'format': 'csv'} + }, } data = yield self.exported_no_data(settings) - self.assertEqual(data, b'') - shutil.rmtree(tmpdir, ignore_errors=True) + self.assertEqual(data['csv'], b'') diff --git a/tests/test_utils_conf.py b/tests/test_utils_conf.py index 61e110845..f064a646c 100644 --- a/tests/test_utils_conf.py +++ b/tests/test_utils_conf.py @@ -1,7 +1,9 @@ import unittest +import warnings -from scrapy.settings import BaseSettings -from scrapy.utils.conf import build_component_list, arglist_to_dict +from scrapy.exceptions import UsageError, ScrapyDeprecationWarning +from scrapy.settings import BaseSettings, Settings +from scrapy.utils.conf import build_component_list, arglist_to_dict, feed_process_params_from_cli class BuildComponentListTest(unittest.TestCase): @@ -90,5 +92,49 @@ class UtilsConfTestCase(unittest.TestCase): {'arg1': 'val1', 'arg2': 'val2'}) +class FeedExportConfigTestCase(unittest.TestCase): + + def test_feed_export_config_invalid_format(self): + settings = Settings() + self.assertRaises(UsageError, feed_process_params_from_cli, settings, ['items.dat'], 'noformat') + + def test_feed_export_config_mismatch(self): + settings = Settings() + self.assertRaises( + UsageError, + feed_process_params_from_cli, settings, ['items1.dat', 'items2.dat'], 'noformat' + ) + + def test_feed_export_config_backward_compatible(self): + with warnings.catch_warnings(record=True) as cw: + settings = Settings() + self.assertEqual( + {'items.dat': {'format': 'csv'}}, + feed_process_params_from_cli(settings, ['items.dat'], 'csv') + ) + self.assertEqual(cw[0].category, ScrapyDeprecationWarning) + + def test_feed_export_config_explicit_formats(self): + settings = Settings() + self.assertEqual( + {'items_1.dat': {'format': 'json'}, 'items_2.dat': {'format': 'xml'}, 'items_3.dat': {'format': 'csv'}}, + feed_process_params_from_cli(settings, ['items_1.dat:json', 'items_2.dat:xml', 'items_3.dat:csv']) + ) + + def test_feed_export_config_implicit_formats(self): + settings = Settings() + self.assertEqual( + {'items_1.json': {'format': 'json'}, 'items_2.xml': {'format': 'xml'}, 'items_3.csv': {'format': 'csv'}}, + feed_process_params_from_cli(settings, ['items_1.json', 'items_2.xml', 'items_3.csv']) + ) + + def test_feed_export_config_stdout(self): + settings = Settings() + self.assertEqual( + {'stdout:': {'format': 'pickle'}}, + feed_process_params_from_cli(settings, ['-:pickle']) + ) + + if __name__ == "__main__": unittest.main() From 915e363db5cb208de9043b61015b79c91ed0a6bb Mon Sep 17 00:00:00 2001 From: nyov Date: Sat, 7 Mar 2020 18:03:25 +0000 Subject: [PATCH 02/10] Remove a 'twisted.test.proto_helpers' deprecation warning --- tests/test_webclient.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 99a998a46..6253d5c3f 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -9,7 +9,12 @@ import OpenSSL.SSL from twisted.trial import unittest from twisted.web import server, static, util, resource from twisted.internet import reactor, defer -from twisted.test.proto_helpers import StringTransport +try: + from twisted.internet.testing import StringTransport +except ImportError: + # deprecated in Twisted 19.7.0 + # (remove once we bump our requirement past that version) + from twisted.test.proto_helpers import StringTransport from twisted.python.filepath import FilePath from twisted.protocols.policies import WrappingFactory from twisted.internet.defer import inlineCallbacks From 49156f2ecb0197c96e3889805a233b9a626c6d65 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Mar 2020 20:45:54 -0300 Subject: [PATCH 03/10] [doc] Feed exports: full local path as example --- docs/topics/feed-exports.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 6d6ba33c9..9e5968a29 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -249,7 +249,7 @@ For instance:: 'fields': None, 'indent': 4, }, - 'items.xml': { + '/home/user/documents/items.xml': { 'format': 'xml', 'fields': ['name', 'price'], 'encoding': 'latin1', From f3bab819ab92cc0750c9b73141abdcbc8da7c4ac Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Mar 2020 20:56:25 -0300 Subject: [PATCH 04/10] Add tests for scrapy.utils.conf.feed_complete_default_values_from_settings --- tests/test_utils_conf.py | 45 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/test_utils_conf.py b/tests/test_utils_conf.py index f064a646c..332120021 100644 --- a/tests/test_utils_conf.py +++ b/tests/test_utils_conf.py @@ -3,7 +3,12 @@ import warnings from scrapy.exceptions import UsageError, ScrapyDeprecationWarning from scrapy.settings import BaseSettings, Settings -from scrapy.utils.conf import build_component_list, arglist_to_dict, feed_process_params_from_cli +from scrapy.utils.conf import ( + arglist_to_dict, + build_component_list, + feed_complete_default_values_from_settings, + feed_process_params_from_cli +) class BuildComponentListTest(unittest.TestCase): @@ -135,6 +140,44 @@ class FeedExportConfigTestCase(unittest.TestCase): feed_process_params_from_cli(settings, ['-:pickle']) ) + def test_feed_complete_default_values_from_settings_empty(self): + feed = {} + settings = Settings({ + "FEED_EXPORT_ENCODING": "custom encoding", + "FEED_EXPORT_FIELDS": ["f1", "f2", "f3"], + "FEED_EXPORT_INDENT": 42, + "FEED_STORE_EMPTY": True, + "FEED_URI_PARAMS": (1, 2, 3, 4), + }) + new_feed = feed_complete_default_values_from_settings(feed, settings) + self.assertEqual(new_feed, { + "encoding": "custom encoding", + "fields": ["f1", "f2", "f3"], + "indent": 42, + "store_empty": True, + "uri_params": (1, 2, 3, 4), + }) + + def test_feed_complete_default_values_from_settings_non_empty(self): + feed = { + "encoding": "other encoding", + "fields": None, + } + settings = Settings({ + "FEED_EXPORT_ENCODING": "custom encoding", + "FEED_EXPORT_FIELDS": ["f1", "f2", "f3"], + "FEED_EXPORT_INDENT": 42, + "FEED_STORE_EMPTY": True, + }) + new_feed = feed_complete_default_values_from_settings(feed, settings) + self.assertEqual(new_feed, { + "encoding": "other encoding", + "fields": None, + "indent": 42, + "store_empty": True, + "uri_params": None, + }) + if __name__ == "__main__": unittest.main() From c886a70eae35e628040da0504af3fd9aaa6aea75 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 11 Mar 2020 21:06:51 -0300 Subject: [PATCH 05/10] Use dict.setdefault in scrapy.utils.conf.feed_complete_default_values_from_settings --- scrapy/utils/conf.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index e01027491..5921f82bf 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -113,16 +113,14 @@ def get_sources(use_closest=True): def feed_complete_default_values_from_settings(feed, settings): out = feed.copy() - if 'encoding' not in out: - out['encoding'] = settings['FEED_EXPORT_ENCODING'] - if 'fields' not in out: - out['fields'] = settings.getlist('FEED_EXPORT_FIELDS') or None - if 'indent' not in out: - out['indent'] = None if settings['FEED_EXPORT_INDENT'] is None else settings.getint('FEED_EXPORT_INDENT') - if 'store_empty' not in out: - out['store_empty'] = settings.getbool('FEED_STORE_EMPTY') - if 'uri_params' not in out: - out['uri_params'] = settings['FEED_URI_PARAMS'] + out.setdefault("encoding", settings["FEED_EXPORT_ENCODING"]) + out.setdefault("fields", settings.getlist("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: + out.setdefault("indent", None) + else: + out.setdefault("indent", settings.getint("FEED_EXPORT_INDENT")) return out From 8d30dc08882e3b97dbaa17b3254de708c358d0d2 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 12 Mar 2020 09:36:15 -0300 Subject: [PATCH 06/10] Response.follow_all: return empty generators for empty sequences --- scrapy/http/response/text.py | 8 +++++--- tests/test_http_response.py | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 33a485328..2f0f3820c 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -188,9 +188,11 @@ class TextResponse(Response): selectors from which links cannot be obtained (for instance, anchor tags without an ``href`` attribute) """ - arg_count = len(list(filter(None, (urls, css, xpath)))) - if arg_count != 1: - raise ValueError('Please supply exactly one of the following arguments: urls, css, xpath') + arguments = [x for x in (urls, css, xpath) if x is not None] + if len(arguments) != 1: + raise ValueError( + "Please supply exactly one of the following arguments: urls, css, xpath" + ) if not urls: if css: urls = self.css(css) diff --git a/tests/test_http_response.py b/tests/test_http_response.py index be17dfd6b..eafc3560e 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -215,6 +215,10 @@ class BaseResponseTest(unittest.TestCase): links = map(Link, absolute) self._assert_followed_all_urls(links, absolute) + def test_follow_all_empty(self): + r = self.response_class("http://example.com") + self.assertEqual([], list(r.follow_all([]))) + def test_follow_all_invalid(self): r = self.response_class("http://example.com") if self.response_class == Response: From 3b0820d747e11a1a7722f0777baf923607fc0485 Mon Sep 17 00:00:00 2001 From: nyov Date: Thu, 12 Mar 2020 19:15:49 +0000 Subject: [PATCH 07/10] Deprecate Spider.make_requests_from_url, part 2 (#4412) --- scrapy/spiders/__init__.py | 6 ++++++ tests/test_spider.py | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 9429f6cb2..ba1c866f8 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -78,6 +78,12 @@ class Spider(object_ref): def make_requests_from_url(self, url): """ This method is deprecated. """ + warnings.warn( + "Spider.make_requests_from_url method is deprecated: " + "it will be removed and not be called by the default " + "Spider.start_requests method in future Scrapy releases. " + "Please override Spider.start_requests method instead." + ) return Request(url, dont_filter=True) def parse(self, response): diff --git a/tests/test_spider.py b/tests/test_spider.py index 317a27076..bb00c8f42 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -602,13 +602,19 @@ class DeprecationTest(unittest.TestCase): self.assertEqual(len(list(spider1.start_requests())), 1) self.assertEqual(len(w), 0) + # spider without overridden make_requests_from_url method + # should issue a warning when called directly + request = spider1.make_requests_from_url("http://www.example.com") + self.assertTrue(isinstance(request, Request)) + self.assertEqual(len(w), 1) + # spider with overridden make_requests_from_url issues a warning, # but the method still works spider2 = MySpider5() requests = list(spider2.start_requests()) self.assertEqual(len(requests), 1) self.assertEqual(requests[0].url, 'http://example.com/foo') - self.assertEqual(len(w), 1) + self.assertEqual(len(w), 2) class NoParseMethodSpiderTest(unittest.TestCase): From ccc4d88779cf2827431ef9e73f976f755c04fe0e Mon Sep 17 00:00:00 2001 From: Lukas Anzinger Date: Thu, 12 Mar 2020 20:42:14 +0100 Subject: [PATCH 08/10] Ignore a domain in allowed_domains with port and issue a warning (#4413) --- scrapy/spidermiddlewares/offsite.py | 11 ++++++++++- tests/test_spidermiddleware_offsite.py | 15 +++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index 36f809699..2fab572e6 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -53,7 +53,8 @@ class OffsiteMiddleware(object): allowed_domains = getattr(spider, 'allowed_domains', None) if not allowed_domains: return re.compile('') # allow all by default - url_pattern = re.compile("^https?://.*$") + url_pattern = re.compile(r"^https?://.*$") + port_pattern = re.compile(r":\d+$") domains = [] for domain in allowed_domains: if domain is None: @@ -62,6 +63,10 @@ class OffsiteMiddleware(object): message = ("allowed_domains accepts only domains, not URLs. " "Ignoring URL entry %s in allowed_domains." % domain) warnings.warn(message, URLWarning) + elif port_pattern.search(domain): + message = ("allowed_domains accepts only domains without ports. " + "Ignoring entry %s in allowed_domains." % domain) + warnings.warn(message, PortWarning) else: domains.append(re.escape(domain)) regex = r'^(.*\.)?(%s)$' % '|'.join(domains) @@ -74,3 +79,7 @@ class OffsiteMiddleware(object): class URLWarning(Warning): pass + + +class PortWarning(Warning): + pass diff --git a/tests/test_spidermiddleware_offsite.py b/tests/test_spidermiddleware_offsite.py index 51c328943..b96807bc2 100644 --- a/tests/test_spidermiddleware_offsite.py +++ b/tests/test_spidermiddleware_offsite.py @@ -4,7 +4,7 @@ import warnings from scrapy.http import Response, Request from scrapy.spiders import Spider -from scrapy.spidermiddlewares.offsite import OffsiteMiddleware, URLWarning +from scrapy.spidermiddlewares.offsite import OffsiteMiddleware, URLWarning, PortWarning from scrapy.utils.test import get_crawler @@ -26,7 +26,8 @@ class TestOffsiteMiddleware(TestCase): Request('http://scrapy.org/1'), Request('http://sub.scrapy.org/1'), Request('http://offsite.tld/letmepass', dont_filter=True), - Request('http://scrapy.test.org/')] + Request('http://scrapy.test.org/'), + Request('http://scrapy.test.org:8000/')] offsite_reqs = [Request('http://scrapy2.org'), Request('http://offsite.tld/'), Request('http://offsite.tld/scrapytest.org'), @@ -80,3 +81,13 @@ class TestOffsiteMiddleware5(TestOffsiteMiddleware4): warnings.simplefilter("always") self.mw.get_host_regex(self.spider) assert issubclass(w[-1].category, URLWarning) + + +class TestOffsiteMiddleware6(TestOffsiteMiddleware4): + + def test_get_host_regex(self): + self.spider.allowed_domains = ['scrapytest.org:8000', 'scrapy.org', 'scrapy.test.org'] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.mw.get_host_regex(self.spider) + assert issubclass(w[-1].category, PortWarning) From 3f6cdcabceff5c5b2ac2935f8073b6efe817645b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 13 Mar 2020 13:25:53 +0100 Subject: [PATCH 09/10] Restrict pytest to versions prior to 5.4 --- tests/requirements-py3.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index d97c4b8ee..d207c5fb0 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -2,7 +2,7 @@ jmespath mitmproxy; python_version >= '3.6' mitmproxy<4.0.0; python_version < '3.6' -pytest +pytest < 5.4 pytest-cov pytest-twisted >= 1.11 pytest-xdist From f9bf4b8d4dd64a1d65e949927b8ea7ad34e756d3 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Date: Sat, 14 Mar 2020 15:09:00 +0530 Subject: [PATCH 10/10] Remove all top-level imports for twisted.internet.reactor (#4406) --- docs/topics/settings.rst | 61 ++++++++++++++++++++++- scrapy/core/downloader/__init__.py | 3 +- scrapy/core/downloader/handlers/ftp.py | 2 +- scrapy/core/downloader/handlers/http10.py | 3 +- scrapy/core/downloader/handlers/http11.py | 6 ++- scrapy/extensions/closespider.py | 3 +- scrapy/mail.py | 5 +- scrapy/utils/benchserver.py | 2 +- scrapy/utils/testproc.py | 3 +- scrapy/utils/testsite.py | 3 +- 10 files changed, 78 insertions(+), 13 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index a70023efa..c01202a10 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1473,7 +1473,66 @@ If a reactor is already installed, :meth:`CrawlerRunner.__init__ ` raises :exc:`Exception` if the installed reactor does not match the -:setting:`TWISTED_REACTOR` setting. +:setting:`TWISTED_REACTOR` setting; therfore, having top-level +:mod:`~twisted.internet.reactor` imports in project files and imported +third-party libraries will make Scrapy raise :exc:`Exception` when +it checks which reactor is installed. + +In order to use the reactor installed by Scrapy:: + + import scrapy + from twisted.internet import reactor + + + class QuotesSpider(scrapy.Spider): + name = 'quotes' + + def __init__(self, *args, **kwargs): + self.timeout = int(kwargs.pop('timeout', '60')) + super(QuotesSpider, self).__init__(*args, **kwargs) + + def start_requests(self): + reactor.callLater(self.timeout, self.stop) + + urls = ['http://quotes.toscrape.com/page/1'] + for url in urls: + yield scrapy.Request(url=url, callback=self.parse) + + def parse(self, response): + for quote in response.css('div.quote'): + yield {'text': quote.css('span.text::text').get()} + + def stop(self): + self.crawler.engine.close_spider(self, 'timeout') + + +which raises :exc:`Exception`, becomes:: + + import scrapy + + + class QuotesSpider(scrapy.Spider): + name = 'quotes' + + def __init__(self, *args, **kwargs): + self.timeout = int(kwargs.pop('timeout', '60')) + super(QuotesSpider, self).__init__(*args, **kwargs) + + def start_requests(self): + from twisted.internet import reactor + reactor.callLater(self.timeout, self.stop) + + urls = ['http://quotes.toscrape.com/page/1'] + for url in urls: + yield scrapy.Request(url=url, callback=self.parse) + + def parse(self, response): + for quote in response.css('div.quote'): + yield {'text': quote.css('span.text::text').get()} + + def stop(self): + self.crawler.engine.close_spider(self, 'timeout') + The default value of the :setting:`TWISTED_REACTOR` setting is ``None``, which means that Scrapy will not attempt to install any specific reactor, and the diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 5a2fdadf5..644be121f 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -3,7 +3,7 @@ from time import time from datetime import datetime from collections import deque -from twisted.internet import reactor, defer, task +from twisted.internet import defer, task from scrapy.utils.defer import mustbe_deferred from scrapy.utils.httpobj import urlparse_cached @@ -133,6 +133,7 @@ class Downloader(object): return deferred def _process_queue(self, spider, slot): + from twisted.internet import reactor if slot.latercall and slot.latercall.active(): return diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 1681c6df8..432cb1831 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -32,7 +32,6 @@ import re from io import BytesIO from urllib.parse import unquote -from twisted.internet import reactor from twisted.internet.protocol import ClientCreator, Protocol from twisted.protocols.ftp import CommandFailed, FTPClient @@ -81,6 +80,7 @@ class FTPDownloadHandler: return cls(crawler.settings) def download_request(self, request, spider): + from twisted.internet import reactor parsed_url = urlparse_cached(request) user = request.meta.get("ftp_user", self.default_user) password = request.meta.get("ftp_password", self.default_password) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index d4aa51bd1..c0146a0a6 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -1,7 +1,5 @@ """Download handlers for http and https schemes """ -from twisted.internet import reactor - from scrapy.utils.misc import create_instance, load_object from scrapy.utils.python import to_unicode @@ -26,6 +24,7 @@ class HTTP10DownloadHandler: return factory.deferred def _connect(self, factory): + from twisted.internet import reactor host, port = to_unicode(factory.host), factory.port if factory.scheme == b'https': client_context_factory = create_instance( diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 93951d3b5..04a8d617a 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -8,7 +8,7 @@ from io import BytesIO from time import time from urllib.parse import urldefrag -from twisted.internet import defer, protocol, reactor, ssl +from twisted.internet import defer, protocol, ssl from twisted.internet.endpoints import TCP4ClientEndpoint from twisted.internet.error import TimeoutError from twisted.web.client import Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, URI @@ -33,6 +33,7 @@ class HTTP11DownloadHandler: lazy = False def __init__(self, settings, crawler=None): + from twisted.internet import reactor self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN') self._pool._factory.noisy = False @@ -81,6 +82,7 @@ class HTTP11DownloadHandler: return agent.download_request(request) def close(self): + from twisted.internet import reactor d = self._pool.closeCachedConnections() # closeCachedConnections will hang on network or server issues, so # we'll manually timeout the deferred. @@ -284,6 +286,7 @@ class ScrapyAgent(object): self._txresponse = None def _get_agent(self, request, timeout): + from twisted.internet import reactor bindaddress = request.meta.get('bindaddress') or self._bindAddress proxy = request.meta.get('proxy') if proxy: @@ -326,6 +329,7 @@ class ScrapyAgent(object): ) def download_request(self, request): + from twisted.internet import reactor timeout = request.meta.get('download_timeout') or self._connectTimeout agent = self._get_agent(request, timeout) diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py index afb2ed049..260b2e86e 100644 --- a/scrapy/extensions/closespider.py +++ b/scrapy/extensions/closespider.py @@ -6,8 +6,6 @@ See documentation in docs/topics/extensions.rst from collections import defaultdict -from twisted.internet import reactor - from scrapy import signals from scrapy.exceptions import NotConfigured @@ -54,6 +52,7 @@ class CloseSpider(object): self.crawler.engine.close_spider(spider, 'closespider_pagecount') def spider_opened(self, spider): + from twisted.internet import reactor self.task = reactor.callLater(self.close_on['timeout'], self.crawler.engine.close_spider, spider, reason='closespider_timeout') diff --git a/scrapy/mail.py b/scrapy/mail.py index 9655b8114..b2a24a3db 100644 --- a/scrapy/mail.py +++ b/scrapy/mail.py @@ -12,7 +12,7 @@ from email.mime.text import MIMEText from email.utils import COMMASPACE, formatdate from io import BytesIO -from twisted.internet import defer, reactor, ssl +from twisted.internet import defer, ssl from scrapy.utils.misc import arg_to_iter from scrapy.utils.python import to_bytes @@ -28,7 +28,6 @@ def _to_bytes_or_none(text): class MailSender(object): - def __init__(self, smtphost='localhost', mailfrom='scrapy@localhost', smtpuser=None, smtppass=None, smtpport=25, smtptls=False, smtpssl=False, debug=False): self.smtphost = smtphost @@ -47,6 +46,7 @@ class MailSender(object): settings.getbool('MAIL_TLS'), settings.getbool('MAIL_SSL')) def send(self, to, subject, body, cc=None, attachs=(), mimetype='text/plain', charset=None, _callback=None): + from twisted.internet import reactor if attachs: msg = MIMEMultipart() else: @@ -111,6 +111,7 @@ class MailSender(object): def _sendmail(self, to_addrs, msg): # Import twisted.mail here because it is not available in python3 + from twisted.internet import reactor from twisted.mail.smtp import ESMTPSenderFactory msg = BytesIO(msg) d = defer.Deferred() diff --git a/scrapy/utils/benchserver.py b/scrapy/utils/benchserver.py index cdbe21942..9d8d64612 100644 --- a/scrapy/utils/benchserver.py +++ b/scrapy/utils/benchserver.py @@ -3,7 +3,6 @@ from urllib.parse import urlencode from twisted.web.server import Site from twisted.web.resource import Resource -from twisted.internet import reactor class Root(Resource): @@ -34,6 +33,7 @@ def _getarg(request, name, default=None, type=str): if __name__ == '__main__': + from twisted.internet import reactor root = Root() factory = Site(root) httpPort = reactor.listenTCP(8998, Site(root)) diff --git a/scrapy/utils/testproc.py b/scrapy/utils/testproc.py index 0f15cf60a..37803b287 100644 --- a/scrapy/utils/testproc.py +++ b/scrapy/utils/testproc.py @@ -1,7 +1,7 @@ import sys import os -from twisted.internet import reactor, defer, protocol +from twisted.internet import defer, protocol class ProcessTest(object): @@ -11,6 +11,7 @@ class ProcessTest(object): cwd = os.getcwd() # trial chdirs to temp dir def execute(self, args, check_code=True, settings=None): + from twisted.internet import reactor env = os.environ.copy() if settings is not None: env['SCRAPY_SETTINGS_MODULE'] = settings diff --git a/scrapy/utils/testsite.py b/scrapy/utils/testsite.py index 6f5c21624..9e1598805 100644 --- a/scrapy/utils/testsite.py +++ b/scrapy/utils/testsite.py @@ -1,12 +1,12 @@ from urllib.parse import urljoin -from twisted.internet import reactor from twisted.web import server, resource, static, util class SiteTest(object): def setUp(self): + from twisted.internet import reactor super(SiteTest, self).setUp() self.site = reactor.listenTCP(0, test_site(), interface="127.0.0.1") self.baseurl = "http://localhost:%d/" % self.site.getHost().port @@ -38,6 +38,7 @@ def test_site(): if __name__ == '__main__': + from twisted.internet import reactor port = reactor.listenTCP(0, test_site(), interface="127.0.0.1") print("http://localhost:%d/" % port.getHost().port) reactor.run()