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

Engine: deprecations and type hints #5090

Merged
merged 27 commits into from
Apr 20, 2021
Merged

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Apr 8, 2021

As discussed earlier @wRAR

Type hints adapted from #3559

Deprecated methods:

  • ExecutionEngine.open_spiders: I didn't check the history, but I suspect this probably comes from the time when multiple spiders could run in a single crawl. That hasn't been the case for a long time, so it's not necessary to have a plurally-named property with only one element.
  • ExecutionEngine.has_capacity: same as the above, the engine only handles one spider slot, which can be checked directly. Not called from outside the engine.
  • ExecutionEngine.schedule: Not called from outside the engine, deprecated as per Requests scheduled when idle never go through the spider middlewares #542 (comment)

Also, the spider argument is deprecated for the spider_is_idle, crawl and download methods, the spider instance saved in the ExecutionEngine.spider after calling open_spider is used instead. I would also like to deprecate passing a spider argument to close_spider, but that would be more complicated because of the order of the positional arguments, let's leave it for a future PR.

@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #5090 (f6a95b0) into master (5b78a64) will decrease coverage by 0.06%.
The diff coverage is 81.59%.

❗ Current head f6a95b0 differs from pull request most recent head ab29cbc. Consider uploading reports for the commit ab29cbc to get more accurate results

@@            Coverage Diff             @@
##           master    #5090      +/-   ##
==========================================
- Coverage   88.25%   88.19%   -0.07%     
==========================================
  Files         162      162              
  Lines       10374    10392      +18     
  Branches     1506     1510       +4     
==========================================
+ Hits         9156     9165       +9     
- Misses        948      952       +4     
- Partials      270      275       +5     
Impacted Files Coverage Δ
scrapy/extensions/memusage.py 50.00% <0.00%> (+1.08%) ⬆️
scrapy/utils/engine.py 52.63% <ø> (ø)
scrapy/core/engine.py 84.12% <82.16%> (-3.38%) ⬇️
scrapy/core/scraper.py 87.86% <100.00%> (ø)
scrapy/downloadermiddlewares/robotstxt.py 100.00% <100.00%> (ø)
scrapy/pipelines/media.py 97.16% <100.00%> (ø)
scrapy/shell.py 67.96% <100.00%> (ø)

scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
elacuesta and others added 2 commits April 8, 2021 11:52
Co-authored-by: Adrián Chaves <adrian@chaves.io>
@elacuesta elacuesta changed the title Engine type hints Engine: deprecate open_spiders property, type hints Apr 8, 2021
@elacuesta elacuesta changed the title Engine: deprecate open_spiders property, type hints Engine: deprecate open_spiders and has_capacity, add type hints Apr 8, 2021
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
@elacuesta elacuesta changed the title Engine: deprecate open_spiders and has_capacity, add type hints Engine: deprecations and type hints Apr 9, 2021
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Show resolved Hide resolved
scrapy/core/engine.py Outdated Show resolved Hide resolved
scrapy/core/engine.py Show resolved Hide resolved
@elacuesta elacuesta merged commit 7e23677 into scrapy:master Apr 20, 2021
@elacuesta elacuesta deleted the engine-types branch April 20, 2021 11:46
marlenachatzigrigoriou pushed a commit to marlenachatzigrigoriou/scrapy that referenced this pull request Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants