Remove deprecated spider parameter

[Laerte]

Closes #5588

[codecov]

Codecov Report

Merging #5589 (fa4ed08) into master (92be5ba) will increase coverage by 0.16%.
The diff coverage is 80.00%.

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

@@            Coverage Diff             @@
##           master    #5589      +/-   ##
==========================================
+ Coverage   88.66%   88.83%   +0.16%     
==========================================
  Files         162      162              
  Lines       10995    10996       +1     
  Branches     1799     1800       +1     
==========================================
+ Hits         9749     9768      +19     
+ Misses        966      948      -18     
  Partials      280      280              

Impacted Files	Coverage Δ
scrapy/core/engine.py	84.01% <80.00%> (+0.05%)	⬆️
scrapy/robotstxt.py	97.53% <0.00%> (+22.22%)	⬆️

[Laerte]

The error in CI is not related with the change made.

[kmike]

If I'm not mistaken, this parameter is deprecated in #5090; it was released with Scrapy 2.6.0 (https://docs.scrapy.org/en/latest/news.html#scrapy-2-6-0-2022-03-01), which happened 2022-03-01. According to https://docs.scrapy.org/en/latest/versioning.html#deprecation-policy, we promise at least one year before the deprecated code is removed, so unfortunately we can't remove it now.

[Gallaecio]

Instead, we need to capture and silence that warning when triggered by our own code.

[Laerte]

@Gallaecio I will update the code to this approach.

[Gallaecio]

I would like to first understand why it is happening in the first place, though. Maybe there is a better way.

[Gallaecio]

OK, so I think we need to change download so that:

• spider = self.spider never happens.
• if spider is None changes to if self.spider is None.

Hopefully that solves the issue without undesired side effects.

[Laerte]

@Gallaecio Hope i getted it the idea.

[Gallaecio]

Looks good to me, thanks! 🚀

How do you feel about test coverage? Do you think you can add at least a test to verify that the exception is not logged with default settings? If you do not see it clearly, I can try and have a look later.

[Laerte]

Looks good to me, thanks! 🚀

How do you feel about test coverage? Do you think you can add at least a test to verify that the exception is not logged with default settings? If you do not see it clearly, I can try and have a look later.

I'm on vacation, if you can add tests i will be glad, thanks!

[hernandezj1]

Thanks for this fix. It worked great!

[kmike]

Thanks @Laerte for keeping the PR up-to-date, this helps a lot!