Install the reactor after reading spider settings

[wRAR]

Closes: #4485

[codecov]

Codecov Report

Merging #5352 (3577c00) into master (c5ab580) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5352   +/-   ##
=======================================
  Coverage   88.56%   88.57%           
=======================================
  Files         163      163           
  Lines       10624    10631    +7     
  Branches     1809     1812    +3     
=======================================
+ Hits         9409     9416    +7     
  Misses        939      939           
  Partials      276      276           

Impacted Files	Coverage Δ
scrapy/commands/shell.py	92.85% <ø> (ø)
scrapy/crawler.py	88.64% <100.00%> (+0.38%)	⬆️
scrapy/utils/log.py	89.47% <100.00%> (+0.11%)	⬆️

[pawelmhm]

works fine, tested this in my project. Minor thing, you could probably simplify this to something like this to avoid repeatedly calling self.settings.get('TWISTED_REACTOR')

reactor_class = self.settings.get("TWISTED_REACTOR")
if init_reactor:
    if reactor_class:
        install_reactor(reactor_class, self.settings["ASYNCIO_EVENT_LOOP"])
... etc

I also wonder if default value for init_reactor should be False? Are there any legitimate cases when you don't want to init reactor if you have it in settings?

[wRAR]

Thanks, I'll implement this simplification. As for the flag, it was the default behavior before this change, and the use case to not init the reactor is CrawlerRunner.

[Gallaecio]

Looks good to me, but I leave the conflict resolution to you :)

[AlvinSartorTrityum]

When is the next version containing this fix supposed to be released? Thanks!

[wRAR]

@AlvinSartorTrityum we don't currently have a date for the next release.

[JSPromisel]

Any updates on when this will be released? Would be very useful to have.

[elacuesta]

This was released as part of 2.6