From 9d74a6f045e68b62202823162fb757b7dbdd9aec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 11 Feb 2020 00:43:05 +0100 Subject: [PATCH 01/15] Make spider names optional --- docs/topics/commands.rst | 4 +- docs/topics/settings.rst | 24 +++++++++ docs/topics/spiders.rst | 38 ++++++++++++++ scrapy/cmdline.py | 2 - scrapy/commands/runspider.py | 4 +- scrapy/settings/default_settings.py | 1 + scrapy/spiderloader.py | 27 +++++++--- scrapy/spiders/__init__.py | 20 +++++++- scrapy/spiders/crawl.py | 3 +- scrapy/spiders/feed.py | 4 +- scrapy/spiders/init.py | 3 +- scrapy/spiders/sitemap.py | 3 +- .../templates/project/module/settings.py.tmpl | 3 ++ scrapy/utils/spider.py | 49 ++++++++++++++---- tests/test_spider.py | 5 +- tests/test_utils_spider.py | 51 ++++++++++++++++--- 16 files changed, 201 insertions(+), 40 deletions(-) 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__": From 2ef9d52c235af11584907cf2cca6b278d13b5aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 11 Feb 2020 00:56:20 +0100 Subject: [PATCH 02/15] Clarify the scope of @abstractspider It can be used regardless of the value of SPIDER_LOADER_REQUIRE_NAME. --- docs/topics/spiders.rst | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 27f9ded81..991a5131a 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -836,19 +836,20 @@ How you mark a spider as a concrete spider depends on the value of the 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: + considered concrete spiders by default. - .. autodecorator:: scrapy.spiders.abstractspider +Use :func:`~scrapy.spiders.abstractspider` to mark a spider as an abstract +spider: - For example:: +.. autodecorator:: scrapy.spiders.abstractspider - from scrapy import abstractspider, Spider +For example:: - @abstractspider - class MyBaseSpider(Spider): - pass + from scrapy import abstractspider, Spider - class MySpider(MyBaseSpider): - pass + @abstractspider + class MyBaseSpider(Spider): + pass + + class MySpider(MyBaseSpider): + pass From f19ca03924ab9ca2c0368431769e4f62a8bd4190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 11 Feb 2020 00:58:40 +0100 Subject: [PATCH 03/15] Fix the abstractspider code example --- docs/topics/spiders.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 991a5131a..f8ddf2135 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -845,7 +845,7 @@ spider: For example:: - from scrapy import abstractspider, Spider + from scrapy.spiders import abstractspider, Spider @abstractspider class MyBaseSpider(Spider): From 182282b2e0c6d97c5d272b0734e8fa76491ee107 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 11 Feb 2020 00:59:34 +0100 Subject: [PATCH 04/15] Remove the docstring of a private method --- scrapy/utils/spider.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 4288b9ac2..252ea46e0 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -14,18 +14,6 @@ def iterate_spider_output(result): 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) From 7aafb76c50e00d3a70fa03281b00c79fd65e1661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Feb 2020 19:42:10 +0100 Subject: [PATCH 05/15] Update tests affected by SPIDER_LOADER_REQUIRE_NAME --- tests/__init__.py | 10 ++++++++++ tests/test_crawler.py | 6 ++++-- tests/test_spiderloader/__init__.py | 15 ++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 12ce79fa9..0eed7f209 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -25,6 +25,16 @@ tests_datadir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'sample_data') +# Settings that, while not part of the default settings for backward +# compatibility reasons, are encouraged in the documentation. +# +# Not using these settings can cause some backward-compatibility warnings to be +# logged, breaking tests that check logged warnings. +FUTURE_PROOF_SETTINGS = { + 'SPIDER_LOADER_REQUIRE_NAME': False, +} + + def get_testdata(*paths): """Return test data""" path = os.path.join(tests_datadir, *paths) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index f8fa26def..6bc7f055d 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -20,6 +20,8 @@ from scrapy.extensions.throttle import AutoThrottle from scrapy.extensions import telnet from scrapy.utils.test import get_testenv +from tests import FUTURE_PROOF_SETTINGS + class BaseCrawlerTest(unittest.TestCase): @@ -31,7 +33,7 @@ class BaseCrawlerTest(unittest.TestCase): class CrawlerTestCase(BaseCrawlerTest): def setUp(self): - self.crawler = Crawler(DefaultSpider, Settings()) + self.crawler = Crawler(DefaultSpider, FUTURE_PROOF_SETTINGS) def test_deprecated_attribute_spiders(self): with warnings.catch_warnings(record=True) as w: @@ -173,7 +175,7 @@ class CrawlerRunnerTestCase(BaseCrawlerTest): def test_deprecated_attribute_spiders(self): with warnings.catch_warnings(record=True) as w: - runner = CrawlerRunner(Settings()) + runner = CrawlerRunner(FUTURE_PROOF_SETTINGS) spiders = runner.spiders self.assertEqual(len(w), 1) self.assertIn("CrawlerRunner.spiders", str(w[0].message)) diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index d8be6e277..5c344e3ff 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -17,6 +17,8 @@ from scrapy.settings import Settings from scrapy.http import Request from scrapy.crawler import CrawlerRunner +from tests import FUTURE_PROOF_SETTINGS + module_dir = os.path.dirname(os.path.abspath(__file__)) @@ -101,7 +103,8 @@ class SpiderLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: module = 'tests.test_spiderloader.test_spiders.doesnotexist' - settings = Settings({'SPIDER_MODULES': [module], + settings = Settings({**FUTURE_PROOF_SETTINGS, + 'SPIDER_MODULES': [module], 'SPIDER_LOADER_WARN_ONLY': True}) spider_loader = SpiderLoader.from_settings(settings) self.assertIn("Could not load spiders from module", str(w[0].message)) @@ -133,8 +136,9 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) - self.assertEqual(len(w), 1) - msg = str(w[0].message) + # We ignore the warning about SPIDER_LOADER_REQUIRE_NAME + self.assertEqual(len(w), 2) + msg = str(w[1].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider3'", msg) @@ -152,8 +156,9 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) - self.assertEqual(len(w), 1) - msg = str(w[0].message) + # We ignore the warning about SPIDER_LOADER_REQUIRE_NAME + self.assertEqual(len(w), 2) + msg = str(w[1].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider1'", msg) self.assertIn("'spider2'", msg) From af453758b54fbf78f6cfd25f10c5d3973db39357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 6 May 2020 21:43:05 +0200 Subject: [PATCH 06/15] =?UTF-8?q?abstractspider=20=E2=86=92=20basespider?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/commands.rst | 8 ++++-- docs/topics/settings.rst | 4 +-- docs/topics/spiders.rst | 56 +++++++++++++++++--------------------- scrapy/spiders/__init__.py | 6 ++-- scrapy/spiders/crawl.py | 4 +-- scrapy/spiders/feed.py | 6 ++-- scrapy/spiders/init.py | 4 +-- scrapy/spiders/sitemap.py | 4 +-- scrapy/utils/spider.py | 14 +++++----- tests/test_utils_spider.py | 6 ++-- 10 files changed, 55 insertions(+), 57 deletions(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index 0be8ec302..157526f0c 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -312,8 +312,9 @@ list * Syntax: ``scrapy list`` * Requires project: *yes* -List all :ref:`concrete spiders ` available in -the current project. The output is one spider per line. +List all :ref:`spiders ` available in the current project, +excluding :ref:`base spiders `. The output is one spider per +line. Usage example:: @@ -321,6 +322,9 @@ Usage example:: spider1 spider2 +Which spiders are listed depends on the configured +:setting:`SPIDER_LOADER_CLASS`. + .. command:: edit edit diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 6b2fd79cd..47456ae57 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1300,11 +1300,11 @@ 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`. +:func:`~scrapy.spiders.basespider`. 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 +:func:`~scrapy.spiders.basespider`. If they do not have a :class:`~scrapy.spiders.Spider.name`, their fully-qualified class name is used as a name. diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 072c101ba..771464e84 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -61,11 +61,14 @@ scrapy.Spider .. attribute:: name - A string which defines the name for this spider. The spider name is how - the spider is located (and instantiated) by Scrapy, so it must be - unique. However, nothing prevents you from instantiating more than one - instance of the same spider. This is the most important spider attribute - and it's required. + A string which defines the name for this spider. + + If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` (default) and you + use the default Scrapy spider loader (see + :setting:`SPIDER_LOADER_CLASS`), a non-empty name is required for the + spider to be discoverable by Scrapy, and the spider name must be + unique to one spider class; however, nothing prevents you from + instantiating more than one instance of the same spider. If the spider scrapes a single domain, a common practice is to name the spider after the domain, with or without the `TLD`_. So, for example, a @@ -817,40 +820,31 @@ Combine SitemapSpider with other sources of urls:: .. _Scrapyd documentation: https://scrapyd.readthedocs.io/en/latest/ -.. _abstract-and-concrete-spiders: +.. _base-spiders: -Abstract and Concrete Spiders -============================= +Base 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. +Base spiders are :class:`~scrapy.spiders.Spider` subclasses that are not meant +to be run by Scrapy. They are only meant to be subclassed to create regular +spiders. They are one way to share code between two or more spiders. -To be able to use a spider, you must mark it as a concrete spider. +Use the :func:`~scrapy.spiders.basespider` decorator to mark a spider class as +a base spider, so that the default Scrapy spider loader (see +:setting:`SPIDER_LOADER_CLASS`) ignores that spider class, hence preventing +Scrapy from running or listing (see the :command:`list` command) that spider +class. -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 +.. autodecorator:: scrapy.spiders.basespider For example:: - from scrapy.spiders import abstractspider, Spider + from scrapy.spiders import basespider, Spider - @abstractspider + @basespider class MyBaseSpider(Spider): pass - class MySpider(MyBaseSpider): - pass +If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` (default), any +:class:`~scrapy.spiders.Spider` subclass without a ``name`` class attribute is +also treated as a base spider. diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 62b7116be..8641ea729 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -13,9 +13,9 @@ 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 `.""" +def basespider(decorated_cls): + """Marks a :class:`~scrapy.spiders.Spider` subclass as a :ref:`base spider + `.""" @classmethod def is_abstract(cls): diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index 07984cb5e..c194f84e2 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 abstractspider, Spider +from scrapy.spiders import basespider, Spider from scrapy.utils.python import get_func_args from scrapy.utils.spider import iterate_spider_output @@ -66,7 +66,7 @@ class Rule: return self.process_request(*args) -@abstractspider +@basespider class CrawlSpider(Spider): rules = () diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 2fba8f1bf..bcd1cff32 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -4,14 +4,14 @@ for scraping from an XML feed. See documentation in docs/topics/spiders.rst """ -from scrapy.spiders import abstractspider, Spider +from scrapy.spiders import basespider, 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 +@basespider class XMLFeedSpider(Spider): """ This class intends to be the base class for spiders that scrape @@ -92,7 +92,7 @@ class XMLFeedSpider(Spider): selector.register_namespace(prefix, uri) -@abstractspider +@basespider 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 f55503137..92dae52b3 100644 --- a/scrapy/spiders/init.py +++ b/scrapy/spiders/init.py @@ -1,8 +1,8 @@ -from scrapy.spiders import abstractspider, Spider +from scrapy.spiders import basespider, Spider from scrapy.utils.spider import iterate_spider_output -@abstractspider +@basespider class InitSpider(Spider): """Base Spider with initialization facilities""" diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 888a39ba3..65947dbe8 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -1,7 +1,7 @@ import re import logging -from scrapy.spiders import abstractspider, Spider +from scrapy.spiders import basespider, 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,7 +10,7 @@ from scrapy.utils.gz import gunzip, gzip_magic_number logger = logging.getLogger(__name__) -@abstractspider +@basespider class SitemapSpider(Spider): sitemap_urls = () diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index a1c30190d..4f038044b 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -21,7 +21,7 @@ def iterate_spider_output(result): return arg_to_iter(deferred_from_coro(result)) -def _is_concrete_spider(spider_class, require_name): +def _is_non_base_spider(spider_class, require_name): return ( inspect.isclass(spider_class) and issubclass(spider_class, Spider) @@ -34,20 +34,20 @@ def _is_concrete_spider(spider_class, require_name): def iter_spider_classes(module, *, require_name=True): - """Return an iterator over all :ref:`concrete spider - ` classes defined in the given module. + """Return an iterator over all :ref:`spider ` classes + defined in the given module, excluding :ref:`base spiders `. 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. + :func:`~scrapy.spiders.basespider` is yielded. If `require_name` is ``False``, any :class:`~scrapy.spiders.Spider` - subclass not decorated with :func:`~scrapy.spiders.abstractspider` is - considered a concrete spider. + subclass not decorated with :func:`~scrapy.spiders.basespider` is + yielded. """ for obj in vars(module).values(): - if (_is_concrete_spider(obj, require_name) + if (_is_non_base_spider(obj, require_name) and obj.__module__ == module.__name__): yield obj diff --git a/tests/test_utils_spider.py b/tests/test_utils_spider.py index f485c2299..aa8128c4e 100644 --- a/tests/test_utils_spider.py +++ b/tests/test_utils_spider.py @@ -3,7 +3,7 @@ import unittest from scrapy import Spider from scrapy.http import Request from scrapy.item import BaseItem -from scrapy.spiders import abstractspider +from scrapy.spiders import basespider from scrapy.utils.spider import iterate_spider_output, iter_spider_classes @@ -11,12 +11,12 @@ class SpiderA(Spider): pass -@abstractspider +@basespider class SpiderB(Spider): pass -@abstractspider +@basespider class SpiderC(Spider): name = 'c' From 30643c142714ea48a0d9429808e31c64e4fbd2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 7 May 2020 11:22:42 +0200 Subject: [PATCH 07/15] =?UTF-8?q?warn=20=E2=86=92=20warnings.warn?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/spiderloader.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index 8a0b6ed82..b3dd79417 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -1,6 +1,6 @@ import traceback +import warnings from collections import defaultdict -from warnings import warn from zope.interface import implementer @@ -19,12 +19,14 @@ class SpiderLoader: 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) + message = ( + '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.' + ) + warnings.warn(message, ScrapyDeprecationWarning) self.spider_modules = settings.getlist('SPIDER_MODULES') self.warn_only = settings.getbool('SPIDER_LOADER_WARN_ONLY') self._spiders = {} @@ -41,7 +43,7 @@ class SpiderLoader: msg = ("There are several spiders with the same name:\n\n" "{}\n\n This can cause unexpected behavior.".format( "\n\n".join(dupes))) - warn(msg, UserWarning) + warnings.warn(msg, UserWarning) def _load_spiders(self, module): classes = iter_spider_classes(module, require_name=self.require_name) @@ -61,7 +63,7 @@ class SpiderLoader: msg = ("\n{tb}Could not load spiders from module '{modname}'. " "See above traceback for details.".format( modname=name, tb=traceback.format_exc())) - warn(msg, RuntimeWarning) + warnings.warn(msg, RuntimeWarning) else: raise self._check_name_duplicates() From 435f1980b7dc51c37ed5ab4d25aa5460671ff468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 May 2020 20:36:54 +0200 Subject: [PATCH 08/15] Do not deprecate SPIDER_LOADER_REQUIRE_NAME=True --- docs/topics/settings.rst | 10 +++++----- scrapy/spiderloader.py | 10 ---------- scrapy/templates/project/module/settings.py.tmpl | 2 +- tests/__init__.py | 10 ---------- tests/test_crawler.py | 6 ++---- tests/test_spiderloader/__init__.py | 15 +++++---------- 6 files changed, 13 insertions(+), 40 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 47456ae57..102d16c94 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1308,11 +1308,11 @@ subclasses unless they are decorated with :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. +.. note:: + + While the default value is ``True`` for historical reasons, this option is + disabled by default in the ``settings.py`` file generated by the + :command:`startproject` command. .. setting:: SPIDER_LOADER_WARN_ONLY diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index a908d6874..e11553b39 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -5,7 +5,6 @@ from collections import defaultdict 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 @@ -19,15 +18,6 @@ class SpiderLoader: def __init__(self, settings): self.require_name = settings.getbool('SPIDER_LOADER_REQUIRE_NAME') - if self.require_name: - message = ( - '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.' - ) - warnings.warn(message, ScrapyDeprecationWarning) self.spider_modules = settings.getlist('SPIDER_MODULES') self.warn_only = settings.getbool('SPIDER_LOADER_WARN_ONLY') self._spiders = {} diff --git a/scrapy/templates/project/module/settings.py.tmpl b/scrapy/templates/project/module/settings.py.tmpl index 6d401779a..55d2b9230 100644 --- a/scrapy/templates/project/module/settings.py.tmpl +++ b/scrapy/templates/project/module/settings.py.tmpl @@ -89,5 +89,5 @@ ROBOTSTXT_OBEY = True #HTTPCACHE_IGNORE_HTTP_CODES = [] #HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage' -# Use setting values that will become default in future versions of Scrapy +# Allow listing and running spiders that do not have a name SPIDER_LOADER_REQUIRE_NAME = False diff --git a/tests/__init__.py b/tests/__init__.py index 0eed7f209..12ce79fa9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -25,16 +25,6 @@ tests_datadir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'sample_data') -# Settings that, while not part of the default settings for backward -# compatibility reasons, are encouraged in the documentation. -# -# Not using these settings can cause some backward-compatibility warnings to be -# logged, breaking tests that check logged warnings. -FUTURE_PROOF_SETTINGS = { - 'SPIDER_LOADER_REQUIRE_NAME': False, -} - - def get_testdata(*paths): """Return test data""" path = os.path.join(tests_datadir, *paths) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 1f38e60f8..9151278a5 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -20,8 +20,6 @@ from scrapy.extensions.throttle import AutoThrottle from scrapy.extensions import telnet from scrapy.utils.test import get_testenv -from tests import FUTURE_PROOF_SETTINGS - class BaseCrawlerTest(unittest.TestCase): @@ -33,7 +31,7 @@ class BaseCrawlerTest(unittest.TestCase): class CrawlerTestCase(BaseCrawlerTest): def setUp(self): - self.crawler = Crawler(DefaultSpider, FUTURE_PROOF_SETTINGS) + self.crawler = Crawler(DefaultSpider, Settings()) def test_populate_spidercls_settings(self): spider_settings = {'TEST1': 'spider', 'TEST2': 'spider'} @@ -161,7 +159,7 @@ class CrawlerRunnerTestCase(BaseCrawlerTest): def test_deprecated_attribute_spiders(self): with warnings.catch_warnings(record=True) as w: - runner = CrawlerRunner(FUTURE_PROOF_SETTINGS) + runner = CrawlerRunner(Settings()) spiders = runner.spiders self.assertEqual(len(w), 1) self.assertIn("CrawlerRunner.spiders", str(w[0].message)) diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index 5c344e3ff..d8be6e277 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -17,8 +17,6 @@ from scrapy.settings import Settings from scrapy.http import Request from scrapy.crawler import CrawlerRunner -from tests import FUTURE_PROOF_SETTINGS - module_dir = os.path.dirname(os.path.abspath(__file__)) @@ -103,8 +101,7 @@ class SpiderLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: module = 'tests.test_spiderloader.test_spiders.doesnotexist' - settings = Settings({**FUTURE_PROOF_SETTINGS, - 'SPIDER_MODULES': [module], + settings = Settings({'SPIDER_MODULES': [module], 'SPIDER_LOADER_WARN_ONLY': True}) spider_loader = SpiderLoader.from_settings(settings) self.assertIn("Could not load spiders from module", str(w[0].message)) @@ -136,9 +133,8 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) - # We ignore the warning about SPIDER_LOADER_REQUIRE_NAME - self.assertEqual(len(w), 2) - msg = str(w[1].message) + self.assertEqual(len(w), 1) + msg = str(w[0].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider3'", msg) @@ -156,9 +152,8 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) - # We ignore the warning about SPIDER_LOADER_REQUIRE_NAME - self.assertEqual(len(w), 2) - msg = str(w[1].message) + self.assertEqual(len(w), 1) + msg = str(w[0].message) self.assertIn("several spiders with the same name", msg) self.assertIn("'spider1'", msg) self.assertIn("'spider2'", msg) From 137e9926780f461803e4b89c8e7e578812dc2a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 6 Nov 2020 22:30:39 +0100 Subject: [PATCH 09/15] =?UTF-8?q?@basespider=20=E2=86=92=20@ignore=5Fspide?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/commands.rst | 5 ++--- docs/topics/settings.rst | 10 +++++----- docs/topics/spiders.rst | 29 ++++++++++++++--------------- scrapy/commands/runspider.py | 3 +-- scrapy/spiders/__init__.py | 18 +++++++++++------- scrapy/spiders/crawl.py | 4 ++-- scrapy/spiders/feed.py | 6 +++--- scrapy/spiders/init.py | 4 ++-- scrapy/spiders/sitemap.py | 4 ++-- scrapy/utils/spider.py | 32 ++++++++++++++------------------ tests/test_utils_spider.py | 6 +++--- 11 files changed, 59 insertions(+), 62 deletions(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index 3c2763917..311c0661b 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -310,9 +310,8 @@ list * Syntax: ``scrapy list`` * Requires project: *yes* -List all :ref:`spiders ` available in the current project, -excluding :ref:`base spiders `. The output is one spider per -line. +List all :ref:`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 8480381c9..b7e568c72 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1359,13 +1359,13 @@ 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` subclasses that have a non-empty :class:`~scrapy.spiders.Spider.name` unless they are decorated with -:func:`~scrapy.spiders.basespider`. +:func:`~scrapy.spiders.ignore_spider`. -If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, Scrapy loads all Spider -subclasses unless they are decorated with -:func:`~scrapy.spiders.basespider`. If they do not have a +If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, Scrapy loads all +:class:`~scrapy.spiders.Spider` subclasses unless they are decorated with +:func:`~scrapy.spiders.ignore_spider`. If they do not have a non-empty :class:`~scrapy.spiders.Spider.name`, their fully-qualified class name is used as a name. diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index c640e1d11..1bedea7cb 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -66,9 +66,12 @@ scrapy.Spider If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` (default) and you use the default Scrapy spider loader (see :setting:`SPIDER_LOADER_CLASS`), a non-empty name is required for the - spider to be discoverable by Scrapy, and the spider name must be - unique to one spider class; however, nothing prevents you from - instantiating more than one instance of the same spider. + spider to be discoverable by the Scrapy commands :command:`crawl`, + :command:`list`, and :command:`runspider`. + + The spider name must be unique to one spider class. If two or more + spiders have the same name, Scrapy commands :command:`crawl` and + :command:`runspider` will only be able to run one of the spiders. If the spider scrapes a single domain, a common practice is to name the spider after the domain, with or without the `TLD`_. So, for example, a @@ -831,31 +834,27 @@ Combine SitemapSpider with other sources of urls:: .. _Scrapyd documentation: https://scrapyd.readthedocs.io/en/latest/ -.. _base-spiders: - Base spiders ============ Base spiders are :class:`~scrapy.spiders.Spider` subclasses that are not meant to be run by Scrapy. They are only meant to be subclassed to create regular -spiders. They are one way to share code between two or more spiders. +spiders or other base spiders. They are one way to share code between two or +more spiders. -Use the :func:`~scrapy.spiders.basespider` decorator to mark a spider class as -a base spider, so that the default Scrapy spider loader (see -:setting:`SPIDER_LOADER_CLASS`) ignores that spider class, hence preventing -Scrapy from running or listing (see the :command:`list` command) that spider -class. +Use the :func:`~scrapy.spiders.ignore_spider` decorator to mark any base spider +class: -.. autodecorator:: scrapy.spiders.basespider +.. autodecorator:: scrapy.spiders.ignore_spider For example:: - from scrapy.spiders import basespider, Spider + from scrapy.spiders import ignore_spider, Spider - @basespider + @ignore_spider class MyBaseSpider(Spider): pass If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` (default), any :class:`~scrapy.spiders.Spider` subclass without a ``name`` class attribute is -also treated as a base spider. +also ignored. diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 079a7792c..1034a8d7c 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -48,8 +48,7 @@ class Command(BaseRunSpiderCommand): except (ImportError, ValueError) as e: raise UsageError(f"Unable to load {filename!r}: {e}\n") require_name = self.settings.getbool('SPIDER_LOADER_REQUIRE_NAME') - spclasses = list(iter_spider_classes(module, - require_name=require_name)) + spclasses = list(iter_spider_classes(module, require_name=require_name)) if not spclasses: raise UsageError(f"No spider found in file: {filename}\n") spidercls = spclasses.pop() diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 30af09124..efe8bec13 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -14,17 +14,21 @@ from scrapy.utils.url import url_is_from_spider from scrapy.utils.deprecate import method_is_overridden -def basespider(decorated_cls): - """Marks a :class:`~scrapy.spiders.Spider` subclass as a :ref:`base spider - `.""" +def ignore_spider(decorated_cls): + """Mark a :class:`~scrapy.spiders.Spider` subclass to be ignored. + + The default spider loader (see :setting:`SPIDER_LOADER_CLASS`) does not + make marked spider classes available for the :command:`crawl`, + :command:`list`, and :command:`runspider` commands. + """ @classmethod - def is_abstract(cls): + def _is_ignored(cls): if cls is decorated_cls: return True - return super(decorated_cls, cls).is_abstract() + return super(decorated_cls, cls)._is_ignored() - decorated_cls.is_abstract = is_abstract + decorated_cls._is_ignored = _is_ignored return decorated_cls @@ -44,7 +48,7 @@ class Spider(object_ref): self.start_urls = [] @classmethod - def is_abstract(cls): + def _is_ignored(cls): return cls is Spider @property diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index 3709c585e..64f9ecb9d 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -10,7 +10,7 @@ from typing import Sequence from scrapy.http import Request, HtmlResponse from scrapy.linkextractors import LinkExtractor -from scrapy.spiders import basespider, Spider +from scrapy.spiders import ignore_spider, Spider from scrapy.utils.spider import iterate_spider_output @@ -59,7 +59,7 @@ class Rule: self.process_request = _get_method(self.process_request, spider) -@basespider +@ignore_spider class CrawlSpider(Spider): rules: Sequence[Rule] = () diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 6e0812404..2ff35045a 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -4,14 +4,14 @@ for scraping from an XML feed. See documentation in docs/topics/spiders.rst """ -from scrapy.spiders import basespider, Spider +from scrapy.spiders import ignore_spider, 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 -@basespider +@ignore_spider class XMLFeedSpider(Spider): """ This class intends to be the base class for spiders that scrape @@ -92,7 +92,7 @@ class XMLFeedSpider(Spider): selector.register_namespace(prefix, uri) -@basespider +@ignore_spider 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 396ca5184..224d1de90 100644 --- a/scrapy/spiders/init.py +++ b/scrapy/spiders/init.py @@ -1,8 +1,8 @@ -from scrapy.spiders import basespider, Spider +from scrapy.spiders import ignore_spider, Spider from scrapy.utils.spider import iterate_spider_output -@basespider +@ignore_spider class InitSpider(Spider): """Base Spider with initialization facilities""" diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 678651e43..c7935e083 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -1,7 +1,7 @@ import re import logging -from scrapy.spiders import basespider, Spider +from scrapy.spiders import ignore_spider, 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,7 +10,7 @@ from scrapy.utils.gz import gunzip, gzip_magic_number logger = logging.getLogger(__name__) -@basespider +@ignore_spider class SitemapSpider(Spider): sitemap_urls = () diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 6576f9959..2b13f8617 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -25,33 +25,29 @@ def iterate_spider_output(result): return arg_to_iter(result) -def _is_non_base_spider(spider_class, require_name): +def _is_ignored(spider_class, *, require_name): 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 - ) + not inspect.isclass(spider_class) + or not issubclass(spider_class, Spider) + or spider_class._is_ignored() + or require_name and not getattr(spider_class, 'name', None) ) def iter_spider_classes(module, *, require_name=True): - """Return an iterator over all :ref:`spider ` classes - defined in the given module, excluding :ref:`base spiders `. + """Return an iterator over all :class:`~scrapy.spiders.Spider` subclasses + defined in the given module, excluding those marked with + :func:`scrapy.spiders.ignore_spider`. 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.basespider` is yielded. - - If `require_name` is ``False``, any :class:`~scrapy.spiders.Spider` - subclass not decorated with :func:`~scrapy.spiders.basespider` is - yielded. + :class:`~scrapy.spiders.Spider` subclass without a non-empty + :class:`~scrapy.spiders.Spider.name` is also excluded. """ for obj in vars(module).values(): - if _is_non_base_spider(obj, require_name) and obj.__module__ == module.__name__: + if ( + not _is_ignored(obj, require_name=require_name) + and obj.__module__ == module.__name__ + ): yield obj diff --git a/tests/test_utils_spider.py b/tests/test_utils_spider.py index 22304016b..c2d3ef099 100644 --- a/tests/test_utils_spider.py +++ b/tests/test_utils_spider.py @@ -2,7 +2,7 @@ import unittest from scrapy import Spider from scrapy.http import Request -from scrapy.spiders import basespider +from scrapy.spiders import ignore_spider from scrapy.item import Item from scrapy.utils.spider import iterate_spider_output, iter_spider_classes @@ -11,12 +11,12 @@ class SpiderA(Spider): pass -@basespider +@ignore_spider class SpiderB(Spider): pass -@basespider +@ignore_spider class SpiderC(Spider): name = 'c' From b82836529322f217d3a7ff8f9b28c5cdb24c55bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 13:50:39 +0100 Subject: [PATCH 10/15] Use Python 3.11 for pylint --- .github/workflows/checks.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index d6fc0f6c5..0f1381bd0 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -12,7 +12,9 @@ jobs: fail-fast: false matrix: include: - - python-version: "3.12" + # pylint < 3 is needed due to a bug in later versions affecting us, + # but Python 3.12 was introduced in later versions. + - python-version: "3.11" env: TOXENV: pylint - python-version: 3.8 From 6a1e7f2f1179be94fa3c1580beca30de8e7f092b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 16 Jan 2024 16:41:44 +0100 Subject: [PATCH 11/15] Revert partial indentation change --- docs/topics/spiders.rst | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index ef2e436dd..69f8b0e61 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -59,22 +59,22 @@ scrapy.Spider .. attribute:: name - A string which defines the name for this spider. + A string which defines the name for this spider. - If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` and you use the - default Scrapy spider loader (see :setting:`SPIDER_LOADER_CLASS`), a - non-empty name is required for the spider to be discoverable by the - Scrapy commands :command:`crawl`, :command:`list`, and - :command:`runspider`. + If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` and you use the + default Scrapy spider loader (see :setting:`SPIDER_LOADER_CLASS`), a + non-empty name is required for the spider to be discoverable by the + Scrapy commands :command:`crawl`, :command:`list`, and + :command:`runspider`. - The spider name must be unique to one spider class. If two or more - spiders have the same name, Scrapy commands :command:`crawl` and - :command:`runspider` will only be able to run one of the spiders. + The spider name must be unique to one spider class. If two or more + spiders have the same name, Scrapy commands :command:`crawl` and + :command:`runspider` will only be able to run one of the spiders. - If the spider scrapes a single domain, a common practice is to name the - spider after the domain, with or without the `TLD`_. So, for example, a - spider that crawls ``mywebsite.com`` would often be called - ``mywebsite``. + If the spider scrapes a single domain, a common practice is to name the + spider after the domain, with or without the `TLD`_. So, for example, a + spider that crawls ``mywebsite.com`` would often be called + ``mywebsite``. .. attribute:: allowed_domains From 5a596a86201c773f86dff93e69f9006df4b9dc11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 17 Jan 2024 17:28:44 +0100 Subject: [PATCH 12/15] Use the spider class import path as name if no name is specified otherwise --- scrapy/spiders/__init__.py | 3 +++ tests/test_spider.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 761e2e8e8..2b8c956c1 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -12,6 +12,7 @@ from twisted.internet.defer import Deferred from scrapy import signals from scrapy.http import Request, Response +from scrapy.utils.python import global_object_name from scrapy.utils.trackref import object_ref from scrapy.utils.url import url_is_from_spider @@ -52,6 +53,8 @@ class Spider(object_ref): def __init__(self, name: Optional[str] = None, **kwargs: Any): if name is not None: self.name = name + elif not getattr(self, "name", None): + self.name = global_object_name(self.__class__) self.__dict__.update(kwargs) if not hasattr(self, "start_urls"): self.start_urls: List[str] = [] diff --git a/tests/test_spider.py b/tests/test_spider.py index fbc2a43bb..bbb52612e 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -24,6 +24,7 @@ from scrapy.spiders import ( XMLFeedSpider, ) from scrapy.spiders.init import InitSpider +from scrapy.utils.python import global_object_name from scrapy.utils.test import get_crawler from tests import get_testdata @@ -54,8 +55,9 @@ class SpiderTest(unittest.TestCase): self.assertEqual(spider.foo, "bar") def test_spider_without_name(self): + self.assertFalse(hasattr(self.spider_class, "name")) spider = self.spider_class() - self.assertFalse(hasattr(spider, "name")) + self.assertEqual(spider.name, global_object_name(self.spider_class)) def test_from_crawler_crawler_and_settings_population(self): crawler = get_crawler() From 77f71e1610740f6e07d6fd13153a4306b4bce83e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 19 Jan 2024 16:45:54 +0100 Subject: [PATCH 13/15] Use global_object_name --- scrapy/spiderloader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index ec10c01dd..11b843944 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -12,6 +12,7 @@ from scrapy import Request, Spider from scrapy.interfaces import ISpiderLoader from scrapy.settings import BaseSettings from scrapy.utils.misc import walk_modules +from scrapy.utils.python import global_object_name from scrapy.utils.spider import iter_spider_classes if TYPE_CHECKING: @@ -56,7 +57,7 @@ class SpiderLoader: def _load_spiders(self, module: ModuleType) -> None: classes = iter_spider_classes(module, require_name=self.require_name) for spcls in classes: - qualname = ".".join((module.__name__, spcls.__name__)) + qualname = global_object_name(spcls) name = getattr(spcls, "name", None) or qualname self._found[name].append((module.__name__, spcls.__name__)) self._spiders[name] = spcls From 170028d9428c3b6b8283ac61f4ed77dc07f9c8be Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 31 Jul 2026 21:03:55 +0200 Subject: [PATCH 14/15] Modernize --- docs/topics/settings.rst | 4 +- docs/topics/spiders.rst | 3 +- scrapy/commands/runspider.py | 10 +-- scrapy/spiderloader.py | 7 +- scrapy/spiders/__init__.py | 28 ++++---- scrapy/utils/spider.py | 16 ++--- tests/test_command_check.py | 19 ++++++ tests/test_command_crawl.py | 18 +++++ tests/test_command_runspider.py | 33 ++++++++++ tests/test_commands.py | 25 +++++++ tests/test_spiderloader/__init__.py | 65 +++++++++++++++++++ .../nameless_spiders/__init__.py | 0 .../nameless_spiders/ignored.py | 10 +++ .../nameless_spiders/nameless1.py | 5 ++ .../nameless_spiders/nameless2.py | 7 ++ tests/utils/bases/spider.py | 10 +++ 16 files changed, 224 insertions(+), 36 deletions(-) create mode 100644 tests/test_spiderloader/nameless_spiders/__init__.py create mode 100644 tests/test_spiderloader/nameless_spiders/ignored.py create mode 100644 tests/test_spiderloader/nameless_spiders/nameless1.py create mode 100644 tests/test_spiderloader/nameless_spiders/nameless2.py diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index defd5fd14..3bcfe3a20 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -2046,13 +2046,13 @@ Default: ``True`` By default, when loading spiders, Scrapy only loads :class:`~scrapy.spiders.Spider` subclasses that have a non-empty -:class:`~scrapy.spiders.Spider.name` unless they are decorated with +:attr:`~scrapy.Spider.name` unless they are decorated with :func:`~scrapy.spiders.ignore_spider`. If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, Scrapy loads all :class:`~scrapy.spiders.Spider` subclasses unless they are decorated with :func:`~scrapy.spiders.ignore_spider`. If they do not have a non-empty -:class:`~scrapy.spiders.Spider.name`, their fully-qualified class name is used +:attr:`~scrapy.Spider.name`, their fully-qualified class name is used as a name. .. note:: This is a :ref:`pre-crawler setting `. diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index dea0cc793..68c384881 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -45,8 +45,7 @@ scrapy.Spider If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` and you use the default Scrapy spider loader (see :setting:`SPIDER_LOADER_CLASS`), a non-empty name is required for the spider to be discoverable by the - Scrapy commands :command:`crawl`, :command:`list`, and - :command:`runspider`. + Scrapy commands :command:`crawl` and :command:`list`. The spider name must be unique to one spider class. If two or more spiders have the same name, Scrapy commands :command:`crawl` and diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 5d82a2b61..d24b85918 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -53,12 +53,14 @@ class Command(BaseRunSpiderCommand): module = _import_file(filename) except (ImportError, ValueError) as e: raise UsageError(f"Unable to load {str(filename)!r}: {e}\n") from e - assert self.settings is not None - require_name = self.settings.getbool("SPIDER_LOADER_REQUIRE_NAME") - spclasses = list(iter_spider_classes(module, require_name=require_name)) + # The spider is looked up by file name, so it does not need a name of + # its own. Named spiders still win over nameless ones, which in a file + # with both are usually base spiders. + spclasses = list(iter_spider_classes(module, require_name=False)) if not spclasses: raise UsageError(f"No spider found in file: {filename}\n") - spidercls = spclasses.pop() + named = [spcls for spcls in spclasses if getattr(spcls, "name", None)] + spidercls = (named or spclasses).pop() assert self.crawler_process self.crawler_process.crawl(spidercls, **opts.spargs) diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index c10179a3d..199f2d859 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -8,7 +8,6 @@ from typing import TYPE_CHECKING, Protocol, cast # working around https://github.com/sphinx-doc/sphinx/issues/10400 from scrapy import Request, Spider # noqa: TC001 from scrapy.utils.misc import load_object, walk_modules_iter -from scrapy.utils.python import global_object_name from scrapy.utils.spider import iter_spider_classes if TYPE_CHECKING: @@ -84,10 +83,8 @@ class SpiderLoader: ) def _load_spiders(self, module: ModuleType) -> None: - classes = iter_spider_classes(module, require_name=self.require_name) - for spcls in classes: - qualname = global_object_name(spcls) - name = getattr(spcls, "name", None) or qualname + for spcls in iter_spider_classes(module, require_name=self.require_name): + name = spcls._default_name() self._found[name].append((module.__name__, spcls.__name__)) self._spiders[name] = spcls diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index adb1b0eaa..cc5f9e487 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -33,21 +33,19 @@ if TYPE_CHECKING: _SpiderT = TypeVar("_SpiderT", bound="type[Spider]") -# Only the classes themselves are ignored, never their subclasses. -_ignored_spiders: set[type[Spider]] = set() - def ignore_spider(cls: _SpiderT) -> _SpiderT: """Mark a :class:`~scrapy.spiders.Spider` subclass to be ignored. - The default spider loader (see :setting:`SPIDER_LOADER_CLASS`) does not - make marked spider classes available for the :command:`crawl`, - :command:`list`, and :command:`runspider` commands. + Marked spider classes are not available to the :command:`crawl`, + :command:`list` and :command:`runspider` commands. Only the decorated + class is marked; its subclasses are unaffected. """ - _ignored_spiders.add(cls) + cls._ignore_spider = True return cls +@ignore_spider class Spider(object_ref): """Base class that any spider must subclass. @@ -58,6 +56,7 @@ class Spider(object_ref): name: str custom_settings: dict[str, Any] | None = None + _ignore_spider: bool #: Start URLs. See :meth:`start`. start_urls: list[str] @@ -66,14 +65,22 @@ class Spider(object_ref): if name is not None: self.name: str = name elif not getattr(self, "name", None): - self.name = global_object_name(self.__class__) + self.name = type(self)._default_name() self.__dict__.update(kwargs) if not hasattr(self, "start_urls"): self.start_urls: list[str] = [] + @classmethod + def _default_name(cls) -> str: + """Return the name under which spider loaders and commands know this + spider class, which falls back to its import path.""" + return getattr(cls, "name", None) or global_object_name(cls) + @classmethod def _is_ignored(cls) -> bool: - return cls in _ignored_spiders + # The mark set by ignore_spider() is read from the class __dict__ so + # that subclasses do not inherit it. + return "_ignore_spider" in cls.__dict__ @property def logger(self) -> SpiderLoggerAdapter: @@ -204,9 +211,6 @@ class Spider(object_ref): return f"<{type(self).__name__} {self.name!r} at 0x{id(self):0x}>" -ignore_spider(Spider) - - # Top-level imports from scrapy.spiders.crawl import CrawlSpider, Rule from scrapy.spiders.feed import CSVFeedSpider, XMLFeedSpider diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 323c86576..f1904e8f0 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -47,15 +47,6 @@ def iterate_spider_output( return arg_to_iter(d) -def _is_ignored(obj: Any, *, require_name: bool) -> bool: - return ( - not inspect.isclass(obj) - or not issubclass(obj, Spider) - or obj._is_ignored() - or (require_name and not getattr(obj, "name", None)) - ) - - def iter_spider_classes( module: ModuleType, *, @@ -67,12 +58,15 @@ def iter_spider_classes( If `require_name` is ``True`` (default), any :class:`~scrapy.spiders.Spider` subclass without a non-empty - :class:`~scrapy.spiders.Spider.name` is also excluded. + :attr:`~scrapy.Spider.name` is also excluded. """ for obj in vars(module).values(): if ( - not _is_ignored(obj, require_name=require_name) + inspect.isclass(obj) + and issubclass(obj, Spider) and obj.__module__ == module.__name__ + and not obj._is_ignored() + and (not require_name or getattr(obj, "name", None)) ): yield obj diff --git a/tests/test_command_check.py b/tests/test_command_check.py index 240f44584..a8ccf92c1 100644 --- a/tests/test_command_check.py +++ b/tests/test_command_check.py @@ -122,6 +122,25 @@ class CheckSpider(scrapy.Spider): """ self._test_contract(proj_path, contracts, parse_def) + def test_check_list_nameless_spider(self, proj_path: Path) -> None: + spider = proj_path / self.project_name / "spiders" / "namelessspider.py" + spider.write_text( + ''' +import scrapy + +class NamelessSpider(scrapy.Spider): + def parse(self, response): + """ + @url data:, + """ +''', + encoding="utf-8", + ) + name = f"{self.project_name}.spiders.namelessspider.NamelessSpider" + ret, out, err = proc("check", "-l", name, cwd=proj_path) + assert ret == 0, err + assert out == f"{name}\n * parse\n" + def test_SCRAPY_CHECK_set(self, proj_path: Path) -> None: parse_def = """ import os diff --git a/tests/test_command_crawl.py b/tests/test_command_crawl.py index 5306e3bf8..94b1cc21a 100644 --- a/tests/test_command_crawl.py +++ b/tests/test_command_crawl.py @@ -35,6 +35,24 @@ class TestCrawlCommand(TestProjectBase): "running 'scrapy crawl' with more than one spider is not supported" in err ) + def test_nameless_spider(self, proj_path: Path) -> None: + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + async def start(self): + self.logger.debug('It works!') + return + yield +""" + (proj_path / self.project_name / "spiders" / "myspider.py").write_text( + spider_code, encoding="utf-8" + ) + name = f"{self.project_name}.spiders.myspider.MySpider" + _, _, log = proc("crawl", name, cwd=proj_path) + assert f"[{name}] DEBUG: It works!" in log + assert "Spider closed (finished)" in log + def test_no_output(self, proj_path: Path) -> None: spider_code = """ import scrapy diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index 11036eaeb..6e8213b67 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -132,6 +132,39 @@ class MySpider(scrapy.Spider): assert ("[scrapy]" in log1) is value assert ("[scrapy.core.engine]" in log1) is not value + def test_runspider_nameless_spider(self, tmp_path: Path) -> None: + nameless_spider = """ +import scrapy + +class MySpider(scrapy.Spider): + async def start(self): + self.logger.debug("It Works!") + return + yield +""" + log = self.get_log(tmp_path, nameless_spider) + assert "[myspider.MySpider] DEBUG: It Works!" in log + assert "INFO: Spider closed (finished)" in log + + def test_runspider_prefers_named_spider(self, tmp_path: Path) -> None: + """A base spider defined after the spider itself does not shadow it.""" + base_last_spider = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + async def start(self): + self.logger.debug("It Works!") + return + yield + +class MyBaseSpider(scrapy.Spider): + pass +""" + log = self.get_log(tmp_path, base_last_spider) + assert "[myspider] DEBUG: It Works!" in log + def test_runspider_no_spider_found(self, tmp_path: Path) -> None: log = self.get_log(tmp_path, "from scrapy.spiders import Spider\n") assert "No spider found in file" in log diff --git a/tests/test_commands.py b/tests/test_commands.py index f20ecc153..943a9c43b 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -432,6 +432,31 @@ class TestMiscCommands(TestProjectBase): subdir.mkdir(exist_ok=True) assert call("list", cwd=subdir) == 0 + @pytest.mark.parametrize("require_name", [False, True]) + def test_list_nameless(self, proj_path: Path, require_name: bool) -> None: + (proj_path / self.project_name / "spiders" / "nameless.py").write_text( + "from scrapy import Spider\n" + "\n" + "\n" + "class NamelessSpider(Spider):\n" + " pass\n" + "\n" + "\n" + "class NamedSpider(Spider):\n" + ' name = "named"\n', + encoding="utf-8", + ) + returncode, out, err = proc( + "list", + "-s", + f"SPIDER_LOADER_REQUIRE_NAME={require_name}", + cwd=proj_path, + ) + assert returncode == 0, err + nameless = f"{self.project_name}.spiders.nameless.NamelessSpider" + expected = ["named"] if require_name else ["named", nameless] + assert out.split() == expected + class TestCommandListing(TestProjectBase): """Tests for the command list that ``scrapy`` prints when called without a diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index 81840f7e0..951d4fa76 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -1,6 +1,7 @@ import contextlib import shutil import sys +import warnings from pathlib import Path from unittest import mock @@ -13,6 +14,7 @@ from scrapy.crawler import CrawlerRunner from scrapy.http import Request from scrapy.settings import Settings from scrapy.spiderloader import DummySpiderLoader, SpiderLoader, get_spider_loader +from tests.test_spiderloader.nameless_spiders.nameless1 import NamelessSpider module_dir = Path(__file__).resolve().parent @@ -165,6 +167,69 @@ class TestSpiderLoader: assert not spiders +class TestNamelessSpiderLoader: + module = "tests.test_spiderloader.nameless_spiders" + nameless1 = f"{module}.nameless1.NamelessSpider" + nameless2 = f"{module}.nameless2.NamelessSpider" + + @pytest.fixture + def spider_loader(self): + settings = Settings( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + return SpiderLoader.from_settings(settings) + + def test_list(self, spider_loader): + assert set(spider_loader.list()) == { + "subclass", + self.nameless1, + self.nameless2, + } + + def test_list_require_name(self): + settings = Settings({"SPIDER_MODULES": [self.module]}) + spider_loader = SpiderLoader.from_settings(settings) + assert set(spider_loader.list()) == {"subclass"} + + def test_load(self, spider_loader): + assert spider_loader.load(self.nameless1) is NamelessSpider + + def test_instance_name(self, spider_loader): + """Spiders are instantiated with the name that the loader knows them + by.""" + for name in spider_loader.list(): + assert spider_loader.load(name)().name == name + + def test_find_by_request(self, spider_loader): + assert spider_loader.find_by_request( + Request("https://nameless.example.com") + ) == [self.nameless1] + + def test_no_dupename_warning(self): + settings = Settings( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + SpiderLoader.from_settings(settings) + + def test_crawler_runner_loading(self, spider_loader): + runner = CrawlerRunner( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + crawler = runner.create_crawler(self.nameless1) + assert crawler.spidercls is NamelessSpider + + class TestDuplicateSpiderNameLoader: def test_dupename_warning(self, spider_loader_env): settings, spiders_dir = spider_loader_env diff --git a/tests/test_spiderloader/nameless_spiders/__init__.py b/tests/test_spiderloader/nameless_spiders/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/test_spiderloader/nameless_spiders/ignored.py b/tests/test_spiderloader/nameless_spiders/ignored.py new file mode 100644 index 000000000..fd9113eae --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/ignored.py @@ -0,0 +1,10 @@ +from scrapy.spiders import Spider, ignore_spider + + +@ignore_spider +class IgnoredSpider(Spider): + name = "ignored" + + +class SubclassSpider(IgnoredSpider): + name = "subclass" diff --git a/tests/test_spiderloader/nameless_spiders/nameless1.py b/tests/test_spiderloader/nameless_spiders/nameless1.py new file mode 100644 index 000000000..dc910cdd4 --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/nameless1.py @@ -0,0 +1,5 @@ +from scrapy.spiders import Spider + + +class NamelessSpider(Spider): + allowed_domains = ["nameless.example.com"] diff --git a/tests/test_spiderloader/nameless_spiders/nameless2.py b/tests/test_spiderloader/nameless_spiders/nameless2.py new file mode 100644 index 000000000..83c0364d8 --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/nameless2.py @@ -0,0 +1,7 @@ +from scrapy.spiders import Spider + + +# Same class name as in the nameless1 module, to check that nameless spiders +# are told apart by their full import path. +class NamelessSpider(Spider): + pass diff --git a/tests/utils/bases/spider.py b/tests/utils/bases/spider.py index df7420130..db3b0033e 100644 --- a/tests/utils/bases/spider.py +++ b/tests/utils/bases/spider.py @@ -41,6 +41,16 @@ class TestSpiderBase(ABC): spider = self.spider_class() assert spider.name == global_object_name(self.spider_class) + def test_ignored(self): + """Base spiders shipped by Scrapy are ignored, their subclasses are + not.""" + + class Subclass(self.spider_class): + pass + + assert self.spider_class._is_ignored() + assert not Subclass._is_ignored() + def test_from_crawler_crawler_and_settings_population(self): crawler = get_crawler() spider = self.spider_class.from_crawler(crawler, "example.com") From 86f56805e2f3376bdbc73d4748577b7854168f74 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 31 Jul 2026 21:27:33 +0200 Subject: [PATCH 15/15] Solve issues --- scrapy/utils/spider.py | 15 +++++++-------- tox.ini | 4 +++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index f1904e8f0..42b43b7d0 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -61,14 +61,13 @@ def iter_spider_classes( :attr:`~scrapy.Spider.name` is also excluded. """ for obj in vars(module).values(): - if ( - inspect.isclass(obj) - and issubclass(obj, Spider) - and obj.__module__ == module.__name__ - and not obj._is_ignored() - and (not require_name or getattr(obj, "name", None)) - ): - yield obj + if not inspect.isclass(obj) or not issubclass(obj, Spider): + continue + if obj.__module__ != module.__name__ or obj._is_ignored(): + continue + if require_name and not getattr(obj, "name", None): + continue + yield obj @overload diff --git a/tox.ini b/tox.ini index edde83356..fa0bf0057 100644 --- a/tox.ini +++ b/tox.ini @@ -111,7 +111,9 @@ commands = pre-commit run {posargs:--all-files} [testenv:pylint] -basepython = python3 +# Version-dependent code and pylint suppressions require a fixed interpreter. +# Keep in sync with the pylint job in .github/workflows/checks.yml. +basepython = python3.14 deps = {[testenv:extra-deps]deps} pylint==4.0.6