Fix start_requests lazyness and early hangs

- Removes new public methods added by #330 to Crawler and CrawlerProcess
- Add test for start_requests lazy evaluation
- Fix and test hangs when start_requests erroed before returning the generator
- Add test when start_requests fails while generating requests
- Simplify Crawler and CrawlerProcess implementation taking in count
  that only one spider can be attached per Crawler. As required by SEP-019
- "scrapy settings" command do not require starting a Crawler anymore
This commit is contained in:
Daniel Graña 2013-10-03 12:48:56 -03:00
parent 2bd5002fb4
commit 5eb429999e
5 changed files with 113 additions and 80 deletions

View File

@ -25,9 +25,7 @@ class Command(ScrapyCommand):
help="print setting value, intepreted as an float")
def run(self, args, opts):
crawler = self.crawler_process.create_crawler()
crawler.configure()
settings = crawler.settings
settings = self.crawler_process.settings
if opts.get:
print settings.get(opts.get)
elif opts.getbool:

View File

@ -19,17 +19,20 @@ class Crawler(object):
self.settings = settings
self.signals = SignalManager(self)
self.stats = load_object(settings['STATS_CLASS'])(self)
self._start_requests = lambda: ()
self._spider = None
# TODO: move SpiderManager to CrawlerProcess
spman_cls = load_object(self.settings['SPIDER_MANAGER_CLASS'])
self.spiders = spman_cls.from_crawler(self)
self._scheduled = {}
def install(self):
# TODO: remove together with scrapy.project.crawler usage
import scrapy.project
assert not hasattr(scrapy.project, 'crawler'), "crawler already installed"
scrapy.project.crawler = self
def uninstall(self):
# TODO: remove together with scrapy.project.crawler usage
import scrapy.project
assert hasattr(scrapy.project, 'crawler'), "crawler not installed"
del scrapy.project.crawler
@ -45,19 +48,13 @@ class Crawler(object):
self.engine = ExecutionEngine(self, self._spider_closed)
def crawl(self, spider, requests=None):
assert self._spider is None, 'Spider already attached'
self._spider = spider
spider.set_crawler(self)
if self.configured and self.engine.running:
assert not self._scheduled
return self._schedule(spider, requests)
elif requests is None:
self._scheduled[spider] = None
if requests is None:
self._start_requests = spider.start_requests
else:
self._scheduled.setdefault(spider, []).append(requests)
def _schedule(self, spider, batches=()):
requests = chain.from_iterable(batches) \
if batches else spider.start_requests()
return self.engine.open_spider(spider, requests)
self._start_requests = lambda: requests
def _spider_closed(self, spider=None):
if not self.engine.open_spiders:
@ -66,47 +63,40 @@ class Crawler(object):
@defer.inlineCallbacks
def start(self):
yield defer.maybeDeferred(self.configure)
for spider, batches in self._scheduled.iteritems():
yield self._schedule(spider, batches)
if self._spider:
yield self.engine.open_spider(self._spider, self._start_requests())
yield defer.maybeDeferred(self.engine.start)
@defer.inlineCallbacks
def stop(self):
if self.engine.running:
if self.configured and self.engine.running:
yield defer.maybeDeferred(self.engine.stop)
class ProcessMixin(object):
""" Mixin which provides automatic control of the Twisted reactor and
installs some convenient signals for shutting it down
"""
class CrawlerProcess(object):
""" A class to run multiple scrapy crawlers in a process sequentially"""
def __init__(self, *a, **kw):
def __init__(self, settings):
install_shutdown_handlers(self._signal_shutdown)
self.settings = settings
self.crawlers = {}
self.stopping = False
def create_crawler(self, name=None):
if name not in self.crawlers:
self.crawlers[name] = Crawler(self.settings)
return self.crawlers[name]
def start(self):
if self.start_crawling():
self.start_reactor()
def start_reactor(self):
if self.settings.getbool('DNSCACHE_ENABLED'):
reactor.installResolver(CachingThreadedResolver(reactor))
reactor.addSystemEventTrigger('before', 'shutdown', self.stop)
reactor.run(installSignalHandlers=False) # blocking call
def start_crawling(self):
raise NotImplementedError
@defer.inlineCallbacks
def stop(self):
raise NotImplementedError
def stop_reactor(self, _=None):
try:
reactor.stop()
except RuntimeError: # raised if already stopped or in shutdown stage
pass
self.stopping = True
for crawler in self.crawlers.itervalues():
yield crawler.stop()
def _signal_shutdown(self, signum, _):
install_shutdown_handlers(self._signal_kill)
@ -120,27 +110,26 @@ class ProcessMixin(object):
signame = signal_names[signum]
log.msg(format='Received %(signame)s twice, forcing unclean shutdown',
level=log.INFO, signame=signame)
reactor.callFromThread(self.stop_reactor)
reactor.callFromThread(self._stop_reactor)
# ------------------------------------------------------------------------#
# The following public methods can't be considered stable and may change at
# any moment.
#
# start_crawling and start_reactor are called from scrapy.commands.shell
# They are splitted because reactor is started on a different thread than IPython shell.
#
def start_crawling(self):
log.scrapy_info(self.settings)
return self._start_crawler() is not None
class CrawlerProcess(ProcessMixin):
""" A class to run multiple scrapy crawlers in a process sequentially
"""
def start_reactor(self):
if self.settings.getbool('DNSCACHE_ENABLED'):
reactor.installResolver(CachingThreadedResolver(reactor))
reactor.addSystemEventTrigger('before', 'shutdown', self.stop)
reactor.run(installSignalHandlers=False) # blocking call
def __init__(self, settings):
super(CrawlerProcess, self).__init__(settings)
self.settings = settings
self.crawlers = {}
self.stopping = False
def create_crawler(self, name=None):
if name not in self.crawlers:
self.crawlers[name] = Crawler(self.settings)
return self.crawlers[name]
def start_crawler(self):
def _start_crawler(self):
if self.crawlers and not self.stopping:
name, crawler = self.crawlers.popitem()
@ -151,23 +140,17 @@ class CrawlerProcess(ProcessMixin):
if sflo:
crawler.signals.connect(sflo.stop, signals.engine_stopped)
crawler.signals.connect(self.check_done, signals.engine_stopped)
crawler.signals.connect(self._check_done, signals.engine_stopped)
crawler.start()
return name, crawler
def check_done(self, **kwargs):
if not self.start_crawler():
self.stop_reactor()
def _check_done(self, **kwargs):
if not self._start_crawler():
self._stop_reactor()
def start_crawling(self):
log.scrapy_info(self.settings)
return self.start_crawler() is not None
@defer.inlineCallbacks
def stop(self):
self.stopping = True
for crawler in self.crawlers.itervalues():
if crawler.configured:
yield crawler.stop()
def _stop_reactor(self, _=None):
try:
reactor.stop()
except RuntimeError: # raised if already stopped or in shutdown stage
pass

View File

@ -103,3 +103,32 @@ class ErrorSpider(FollowAllSpider):
for request in super(ErrorSpider, self).parse(response):
yield request
self.raise_exception()
class BrokenStartRequestsSpider(FollowAllSpider):
fail_before_yield = False
fail_yielding = False
def __init__(self, *a, **kw):
super(BrokenStartRequestsSpider, self).__init__(*a, **kw)
self.seedsseen = []
def start_requests(self):
if self.fail_before_yield:
1 / 0
for s in xrange(100):
qargs = {'total': 10, 'seed': s}
url = "http://localhost:8998/follow?%s" % urlencode(qargs, doseq=1)
yield Request(url, meta={'seed': s})
if self.fail_yielding:
2 / 0
assert self.seedsseen, \
'All start requests consumed before any download happened'
def parse(self, response):
self.seedsseen.append(response.meta.get('seed'))
for req in super(BrokenStartRequestsSpider, self).parse(response):
yield req

View File

@ -18,14 +18,14 @@ class CmdlineTest(unittest.TestCase):
def test_default_settings(self):
self.assertEqual(self._execute('settings', '--get', 'TEST1'), \
'default + started')
'default')
def test_override_settings_using_set_arg(self):
self.assertEqual(self._execute('settings', '--get', 'TEST1', '-s', 'TEST1=override'), \
'override + started')
'override')
def test_override_settings_using_envvar(self):
self.env['SCRAPY_TEST1'] = 'override'
self.assertEqual(self._execute('settings', '--get', 'TEST1'), \
'override + started')
'override')

View File

@ -1,13 +1,13 @@
from twisted.internet import defer
from twisted.trial.unittest import TestCase
from scrapy.utils.test import get_crawler, get_testlog
from scrapy.tests.spiders import FollowAllSpider, DelaySpider, SimpleSpider
from scrapy.tests.spiders import FollowAllSpider, DelaySpider, SimpleSpider, \
BrokenStartRequestsSpider
from scrapy.tests.mockserver import MockServer
def docrawl(spider, settings=None):
crawler = get_crawler(settings)
crawler.configure()
crawler.crawl(spider)
return crawler.start()
@ -90,6 +90,29 @@ class CrawlTestCase(TestCase):
yield docrawl(spider)
self._assert_retried()
@defer.inlineCallbacks
def test_start_requests_bug_before_yield(self):
spider = BrokenStartRequestsSpider(fail_before_yield=1)
yield docrawl(spider)
errors = self.flushLoggedErrors(ZeroDivisionError)
self.assertEqual(len(errors), 1)
@defer.inlineCallbacks
def test_start_requests_bug_yielding(self):
spider = BrokenStartRequestsSpider(fail_yielding=1)
yield docrawl(spider)
errors = self.flushLoggedErrors(ZeroDivisionError)
self.assertEqual(len(errors), 1)
@defer.inlineCallbacks
def test_start_requests_lazyness(self):
settings = {"CONCURRENT_REQUESTS": 1}
spider = BrokenStartRequestsSpider()
yield docrawl(spider, settings)
#self.assertTrue(False, spider.seedsseen)
#self.assertTrue(spider.seedsseen.index(None) < spider.seedsseen.index(99),
# spider.seedsseen)
@defer.inlineCallbacks
def test_unbounded_response(self):
# Completeness of responses without Content-Length or Transfer-Encoding