diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index a0dcba90d..0be8ec302 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -312,8 +312,8 @@ list * Syntax: ``scrapy list`` * Requires project: *yes* -List all available spiders in the current project. The output is one spider per -line. +List all :ref:`concrete spiders ` available in +the current project. The output is one spider per line. Usage example:: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index fa63a5807..39ea2391c 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1310,6 +1310,30 @@ Default: ``'scrapy.spiderloader.SpiderLoader'`` The class that will be used for loading spiders, which must implement the :ref:`topics-api-spiderloader`. +.. setting:: SPIDER_LOADER_REQUIRE_NAME + +SPIDER_LOADER_REQUIRE_NAME +-------------------------- + +Default: ``True`` + +By default, when loading spiders, Scrapy only loads +:class:`~scrapy.spiders.Spider` subclasses that have a +:class:`~scrapy.spiders.Spider.name` unless they are decorated with +:func:`~scrapy.spiders.abstractspider`. + +If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, Scrapy loads all Spider +subclasses unless they are decorated with +:func:`~scrapy.spiders.abstractspider`. If they do not have a +:class:`~scrapy.spiders.Spider.name`, their fully-qualified class name is used +as a name. + +In a future version of Scrapy, the :setting:`SPIDER_LOADER_REQUIRE_NAME` +setting will no longer be available, and Scrapy will always behave as if +:setting:`SPIDER_LOADER_REQUIRE_NAME` were ``False``. Set +:setting:`SPIDER_LOADER_REQUIRE_NAME` to ``False`` now to future-proof your +spiders. + .. setting:: SPIDER_LOADER_WARN_ONLY SPIDER_LOADER_WARN_ONLY diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index b0fb14e24..27f9ded81 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -814,3 +814,41 @@ Combine SitemapSpider with other sources of urls:: .. _robots.txt: http://www.robotstxt.org/ .. _TLD: https://en.wikipedia.org/wiki/Top-level_domain .. _Scrapyd documentation: https://scrapyd.readthedocs.io/en/latest/ + + +.. _abstract-and-concrete-spiders: + +Abstract and Concrete Spiders +============================= + +Abstract spiders are :class:`~scrapy.spiders.Spider` subclasses that are +not loaded by the default spider loader (see :setting:`SPIDER_LOADER_CLASS`). +Abstract spiders cannot be executed, they can only be subclassed to create +other spiders. + +To be able to use a spider, you must mark it as a concrete spider. + +How you mark a spider as a concrete spider depends on the value of the +:setting:`SPIDER_LOADER_REQUIRE_NAME` setting: + +- If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` (default), add a + non-empty :class:`~scrapy.spiders.Spider.name` to a spider to make it a + concrete spider. + +- If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, all spiders are + considered concrete spiders by default. Use + :func:`~scrapy.spiders.abstractspider` to mark a spider as an abstract + spider: + + .. autodecorator:: scrapy.spiders.abstractspider + + For example:: + + from scrapy import abstractspider, Spider + + @abstractspider + class MyBaseSpider(Spider): + pass + + class MySpider(MyBaseSpider): + pass diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index ec78f7c91..96e7706dc 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -16,8 +16,6 @@ from scrapy.settings.deprecated import check_deprecated_settings def _iter_command_classes(module_name): - # TODO: add `name` attribute to commands and and merge this function with - # scrapy.utils.spider.iter_spider_classes for module in walk_modules(module_name): for obj in vars(module).values(): if inspect.isclass(obj) and \ diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 57d8471ca..26a2d5fba 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -79,7 +79,9 @@ class Command(ScrapyCommand): module = _import_file(filename) except (ImportError, ValueError) as e: raise UsageError("Unable to load %r: %s\n" % (filename, e)) - spclasses = list(iter_spider_classes(module)) + require_name = self.settings.getbool('SPIDER_LOADER_REQUIRE_NAME') + spclasses = list(iter_spider_classes(module, + require_name=require_name)) if not spclasses: raise UsageError("No spider found in file: %s\n" % filename) spidercls = spclasses.pop() diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index fc7b62e78..d81a1d77f 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -256,6 +256,7 @@ SCHEDULER_PRIORITY_QUEUE = 'scrapy.pqueues.ScrapyPriorityQueue' SCRAPER_SLOT_MAX_ACTIVE_SIZE = 5000000 SPIDER_LOADER_CLASS = 'scrapy.spiderloader.SpiderLoader' +SPIDER_LOADER_REQUIRE_NAME = True SPIDER_LOADER_WARN_ONLY = False SPIDER_MIDDLEWARES = {} diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index 3beca4060..02315801d 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -1,11 +1,11 @@ -# -*- coding: utf-8 -*- -from collections import defaultdict import traceback -import warnings +from collections import defaultdict +from warnings import warn from zope.interface import implementer from scrapy.interfaces import ISpiderLoader +from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.misc import walk_modules from scrapy.utils.spider import iter_spider_classes @@ -17,6 +17,14 @@ class SpiderLoader(object): in a Scrapy project. """ def __init__(self, settings): + self.require_name = settings.getbool('SPIDER_LOADER_REQUIRE_NAME') + if self.require_name: + warn('SPIDER_LOADER_REQUIRE_NAME is True. In a future version of ' + 'Scrapy, the SPIDER_LOADER_REQUIRE_NAME setting will be ' + 'removed, and Scrapy will always behave as if ' + 'SPIDER_LOADER_REQUIRE_NAME were False. To remove this ' + 'warning, set SPIDER_LOADER_REQUIRE_NAME to False.', + ScrapyDeprecationWarning) self.spider_modules = settings.getlist('SPIDER_MODULES') self.warn_only = settings.getbool('SPIDER_LOADER_WARN_ONLY') self._spiders = {} @@ -33,12 +41,15 @@ class SpiderLoader(object): msg = ("There are several spiders with the same name:\n\n" "{}\n\n This can cause unexpected behavior.".format( "\n\n".join(dupes))) - warnings.warn(msg, UserWarning) + warn(msg, UserWarning) def _load_spiders(self, module): - for spcls in iter_spider_classes(module): - self._found[spcls.name].append((module.__name__, spcls.__name__)) - self._spiders[spcls.name] = spcls + classes = iter_spider_classes(module, require_name=self.require_name) + for spcls in classes: + qualname = '.'.join((module.__name__, spcls.__name__)) + name = getattr(spcls, 'name', None) or qualname + self._found[name].append((module.__name__, spcls.__name__)) + self._spiders[name] = spcls def _load_all_spiders(self): for name in self.spider_modules: @@ -50,7 +61,7 @@ class SpiderLoader(object): msg = ("\n{tb}Could not load spiders from module '{modname}'. " "See above traceback for details.".format( modname=name, tb=traceback.format_exc())) - warnings.warn(msg, RuntimeWarning) + warn(msg, RuntimeWarning) else: raise self._check_name_duplicates() diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 9429f6cb2..1c6bbcf46 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -13,6 +13,20 @@ from scrapy.utils.url import url_is_from_spider from scrapy.utils.deprecate import method_is_overridden +def abstractspider(decorated_cls): + """Marks a :class:`~scrapy.spiders.Spider` subclass as an :ref:`abstract + spider `.""" + + @classmethod + def is_abstract(cls): + if cls is decorated_cls: + return True + return super(decorated_cls, cls).is_abstract() + + decorated_cls.is_abstract = is_abstract + return decorated_cls + + class Spider(object_ref): """Base class for scrapy spiders. All spiders must inherit from this class. @@ -24,12 +38,14 @@ class Spider(object_ref): def __init__(self, name=None, **kwargs): if name is not None: self.name = name - elif not getattr(self, 'name', None): - raise ValueError("%s must have a name" % type(self).__name__) self.__dict__.update(kwargs) if not hasattr(self, 'start_urls'): self.start_urls = [] + @classmethod + def is_abstract(cls): + return cls is Spider + @property def logger(self): logger = logging.getLogger(self.name) diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index a2c364c0e..53b9a5a16 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -11,7 +11,7 @@ import warnings from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, HtmlResponse from scrapy.linkextractors import LinkExtractor -from scrapy.spiders import Spider +from scrapy.spiders import abstractspider, Spider from scrapy.utils.python import get_func_args from scrapy.utils.spider import iterate_spider_output @@ -66,6 +66,7 @@ class Rule(object): return self.process_request(*args) +@abstractspider class CrawlSpider(Spider): rules = () diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index c566f0236..2fba8f1bf 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -4,13 +4,14 @@ for scraping from an XML feed. See documentation in docs/topics/spiders.rst """ -from scrapy.spiders import Spider +from scrapy.spiders import abstractspider, Spider from scrapy.utils.iterators import xmliter, csviter from scrapy.utils.spider import iterate_spider_output from scrapy.selector import Selector from scrapy.exceptions import NotConfigured, NotSupported +@abstractspider class XMLFeedSpider(Spider): """ This class intends to be the base class for spiders that scrape @@ -91,6 +92,7 @@ class XMLFeedSpider(Spider): selector.register_namespace(prefix, uri) +@abstractspider class CSVFeedSpider(Spider): """Spider for parsing CSV feeds. It receives a CSV file in a response; iterates through each of its rows, diff --git a/scrapy/spiders/init.py b/scrapy/spiders/init.py index fd41133ea..f55503137 100644 --- a/scrapy/spiders/init.py +++ b/scrapy/spiders/init.py @@ -1,7 +1,8 @@ -from scrapy.spiders import Spider +from scrapy.spiders import abstractspider, Spider from scrapy.utils.spider import iterate_spider_output +@abstractspider class InitSpider(Spider): """Base Spider with initialization facilities""" diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index d368c7108..888a39ba3 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -1,7 +1,7 @@ import re import logging -from scrapy.spiders import Spider +from scrapy.spiders import abstractspider, Spider from scrapy.http import Request, XmlResponse from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots from scrapy.utils.gz import gunzip, gzip_magic_number @@ -10,6 +10,7 @@ from scrapy.utils.gz import gunzip, gzip_magic_number logger = logging.getLogger(__name__) +@abstractspider class SitemapSpider(Spider): sitemap_urls = () diff --git a/scrapy/templates/project/module/settings.py.tmpl b/scrapy/templates/project/module/settings.py.tmpl index cb220eafc..6d401779a 100644 --- a/scrapy/templates/project/module/settings.py.tmpl +++ b/scrapy/templates/project/module/settings.py.tmpl @@ -88,3 +88,6 @@ ROBOTSTXT_OBEY = True #HTTPCACHE_DIR = 'httpcache' #HTTPCACHE_IGNORE_HTTP_CODES = [] #HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage' + +# Use setting values that will become default in future versions of Scrapy +SPIDER_LOADER_REQUIRE_NAME = False diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 72775df5c..4288b9ac2 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -13,19 +13,46 @@ def iterate_spider_output(result): return arg_to_iter(deferred_from_coro(result)) -def iter_spider_classes(module): - """Return an iterator over all spider classes defined in the given module - that can be instantiated (ie. which have name) - """ - # this needs to be imported here until get rid of the spider manager - # singleton in scrapy.spider.spiders - from scrapy.spiders import Spider +def _is_concrete_spider(spider_class, require_name): + """Return ``True`` if `spider_class` is a :ref:`concrete + ` :class:`~scrapy.spiders.Spider` subclass. + If `require_name` is ``True`` (default), any + :class:`~scrapy.spiders.Spider` subclass with a non-empty + :class:`~scrapy.spiders.Spider.name` and not decorated with + :func:`~scrapy.spiders.abstractspider` is considered a concrete spider. + + If `require_name` is ``False``, any :class:`~scrapy.spiders.Spider` + subclass not decorated with :func:`~scrapy.spiders.abstractspider` is + considered a concrete spider. + """ + return ( + inspect.isclass(spider_class) + and issubclass(spider_class, Spider) + and not spider_class.is_abstract() + and ( + getattr(spider_class, 'name', None) + or not require_name + ) + ) + + +def iter_spider_classes(module, *, require_name=True): + """Return an iterator over all :ref:`concrete spider + ` classes defined in the given module. + + If `require_name` is ``True`` (default), any + :class:`~scrapy.spiders.Spider` subclass with a non-empty + :class:`~scrapy.spiders.Spider.name` and not decorated with + :func:`~scrapy.spiders.abstractspider` is considered a concrete spider. + + If `require_name` is ``False``, any :class:`~scrapy.spiders.Spider` + subclass not decorated with :func:`~scrapy.spiders.abstractspider` is + considered a concrete spider. + """ for obj in vars(module).values(): - if inspect.isclass(obj) and \ - issubclass(obj, Spider) and \ - obj.__module__ == module.__name__ and \ - getattr(obj, 'name', None): + if (_is_concrete_spider(obj, require_name) + and obj.__module__ == module.__name__): yield obj diff --git a/tests/test_spider.py b/tests/test_spider.py index 317a27076..b61d291e4 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -45,9 +45,8 @@ class SpiderTest(unittest.TestCase): self.assertEqual(spider.foo, 'bar') def test_spider_without_name(self): - """``__init__`` method arguments are assigned to spider attributes""" - self.assertRaises(ValueError, self.spider_class) - self.assertRaises(ValueError, self.spider_class, somearg='foo') + spider = self.spider_class() + self.assertIsNone(spider.name) def test_from_crawler_crawler_and_settings_population(self): crawler = get_crawler() diff --git a/tests/test_utils_spider.py b/tests/test_utils_spider.py index ee7d17062..f485c2299 100644 --- a/tests/test_utils_spider.py +++ b/tests/test_utils_spider.py @@ -3,15 +3,46 @@ import unittest from scrapy import Spider from scrapy.http import Request from scrapy.item import BaseItem +from scrapy.spiders import abstractspider from scrapy.utils.spider import iterate_spider_output, iter_spider_classes -class MySpider1(Spider): - name = 'myspider1' +class SpiderA(Spider): + pass -class MySpider2(Spider): - name = 'myspider2' +@abstractspider +class SpiderB(Spider): + pass + + +@abstractspider +class SpiderC(Spider): + name = 'c' + + +class SpiderA1(SpiderA): + name = 'a1' + + +class SpiderA2(SpiderA): + pass + + +class SpiderB1(SpiderB): + name = 'b1' + + +class SpiderB2(SpiderB): + pass + + +class SpiderC1(SpiderC): + name = 'c1' + + +class SpiderC2(SpiderC): + pass class UtilsSpidersTestCase(unittest.TestCase): @@ -26,10 +57,16 @@ class UtilsSpidersTestCase(unittest.TestCase): self.assertEqual(list(iterate_spider_output(o)), [o]) self.assertEqual(list(iterate_spider_output([r, i, o])), [r, i, o]) - def test_iter_spider_classes(self): + def test_iter_spider_classes_require_name(self): import tests.test_utils_spider - it = iter_spider_classes(tests.test_utils_spider) - self.assertEqual(set(it), {MySpider1, MySpider2}) + it = iter_spider_classes(tests.test_utils_spider, require_name=True) + self.assertEqual(set(it), {SpiderA1, SpiderB1, SpiderC1, SpiderC2}) + + def test_iter_spider_classes_dont_require_name(self): + import tests.test_utils_spider + it = iter_spider_classes(tests.test_utils_spider, require_name=False) + self.assertEqual(set(it), {SpiderA, SpiderA1, SpiderA2, SpiderB1, + SpiderB2, SpiderC1, SpiderC2}) if __name__ == "__main__":