Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call asyncio.get_event_loop when installing the asyncio reactor #4872

Merged
merged 5 commits into from Nov 10, 2020

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Nov 5, 2020

Fixes #4855. See #4855 (comment) for the rationale of this change.

  • Code changes
  • Tests
  • Docs

In the docs, I intentionally only mentioned set_event_loop and omitted the call to get_event_loop when the default event loop is used. I've grown more convinced that this is "harmless", in the sense that get_event_loop will call set_event_loop only if it hasn't been called, and if that's the case, the user most likely does not care about the loop being used.

@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #4872 (b20cfef) into master (c292957) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4872      +/-   ##
==========================================
+ Coverage   87.84%   87.86%   +0.02%     
==========================================
  Files         160      160              
  Lines        9749     9750       +1     
  Branches     1439     1439              
==========================================
+ Hits         8564     8567       +3     
+ Misses        927      926       -1     
+ Partials      258      257       -1     
Impacted Files Coverage Δ
scrapy/utils/reactor.py 81.66% <100.00%> (+0.31%) ⬆️
scrapy/core/downloader/__init__.py 92.48% <0.00%> (+1.50%) ⬆️

Copy link
Member

@kmike kmike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks right, thanks @elacuesta!

@kmike kmike merged commit 27b07c6 into scrapy:master Nov 10, 2020
@kmike
Copy link
Member

kmike commented Nov 10, 2020

Thanks @elacuesta!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return Deferred object from open_spider in pipeline blocks spider
2 participants