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

docs: Clarify there's one extension instance per crawler #5014

merged 1 commit into from Apr 1, 2021

docs: Clarify there's one extension instance per crawler #5014

merged 1 commit into from Apr 1, 2021


Copy link

@jpmckinney jpmckinney commented Feb 26, 2021

The previous documentation suggested there was a single instance of each extension. This is not the case. For example, when running multiple spiders in the same process, the LogStats extension still works correctly, and it doesn't track stats per spider.

Copy link

codecov bot commented Feb 27, 2021

Codecov Report

Merging #5014 (caecf4e) into master (f95ebd8) will decrease coverage by 0.16%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5014      +/-   ##
- Coverage   88.01%   87.84%   -0.17%     
  Files         158      158              
  Lines        9726     9726              
  Branches     1433     1433              
- Hits         8560     8544      -16     
- Misses        911      928      +17     
+ Partials      255      254       -1     
Impacted Files Coverage Δ
scrapy/ 75.30% <0.00%> (-22.23%) ⬇️
scrapy/core/downloader/ 92.48% <0.00%> (+1.50%) ⬆️

Copy link

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

Thanks, that is a good point!

However, I personally feel like this change could be reduced to appending “ per spider being run” to “instantiating a single instance of the extension class”, and things should be clear enough.

I also think that we should avoid mentioning the crawler at this stage, as the crawler object is something people reading these 2 sections may not yet know.

Copy link
Contributor Author

jpmckinney commented Mar 2, 2021

@Gallaecio I've now made the simpler change you suggest.

@wRAR wRAR closed this Mar 3, 2021
@wRAR wRAR reopened this Mar 3, 2021
@kmike kmike merged commit e0a2d2b into scrapy:master Apr 1, 2021
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

4 participants