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

Include loading settings in Running multiple spiders in the same precess section #5070

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

ric2b
Copy link
Contributor

@ric2b ric2b commented Mar 29, 2021

The example in the documentation doesn't take into account how to load the project's settings.

I assume most people want to load the settings when running multiple spiders concurrently. If they don't, it's at least more straightforward to remove that functionality than to notice the settings are being ignored and then find out how to fix it.

I had to find it in a third-party example: https://botproxy.net/docs/how-to/scrapy-crawl-multiple-spiders-sharing-same-items-pipeline-and-settings-but-with-separa/

…cess` section

The example in the documentation doesn't take into account the project settings
@Gallaecio
Copy link
Member

Please, switch the target of this pull request to the master branch.

As for the examples themselves, as they seem intended to feature the alternative to scrapy crawl to run several spiders, I agree that it makes sense to make this change a part of the example itself. Also, we should probably reproduce the same change in the examples right below, which are meant to be based on the modified example.

@ric2b ric2b changed the base branch from 2.4 to master April 6, 2021 19:18
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #5070 (8b46aed) into master (63becd1) will increase coverage by 0.20%.
The diff coverage is 91.09%.

❗ Current head 8b46aed differs from pull request most recent head 8603f9d. Consider uploading reports for the commit 8603f9d to get more accurate results

@@            Coverage Diff             @@
##           master    #5070      +/-   ##
==========================================
+ Coverage   87.86%   88.07%   +0.20%     
==========================================
  Files         160      162       +2     
  Lines        9750    10345     +595     
  Branches     1439     1504      +65     
==========================================
+ Hits         8567     9111     +544     
- Misses        926      965      +39     
- Partials      257      269      +12     
Impacted Files Coverage Δ
scrapy/commands/crawl.py 60.00% <0.00%> (ø)
scrapy/core/downloader/middleware.py 96.36% <ø> (ø)
scrapy/core/scheduler.py 93.20% <ø> (+3.20%) ⬆️
scrapy/utils/log.py 89.24% <ø> (ø)
scrapy/utils/project.py 80.00% <ø> (+3.33%) ⬆️
scrapy/contracts/__init__.py 83.59% <61.70%> (+0.12%) ⬆️
scrapy/pipelines/images.py 90.35% <80.00%> (-1.47%) ⬇️
scrapy/core/http2/protocol.py 83.41% <83.41%> (ø)
scrapy/core/downloader/contextfactory.py 87.03% <84.61%> (-2.97%) ⬇️
scrapy/core/http2/stream.py 91.37% <91.37%> (ø)
... and 44 more

@ric2b
Copy link
Contributor Author

ric2b commented Apr 6, 2021

Done :)

Btw, just noticed that how to load the settings is mentioned in the previous section. But someone that is just looking for how to run multiple spiders concurrently (like I was) will easily miss it.

@wRAR
Copy link
Member

wRAR commented Apr 15, 2021

Thank you!

@wRAR wRAR merged commit 5b78a64 into scrapy:master Apr 15, 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