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

DOC improved Architecture overview #1879

Merged
merged 2 commits into from Mar 31, 2016
Merged

DOC improved Architecture overview #1879

merged 2 commits into from Mar 31, 2016

Conversation

@kmike
Copy link
Member

@kmike kmike commented Mar 25, 2016

  • spiders don't have to work on specific domains;
  • explain what to use Downloader middleware for
    and what to use Spider middleware for;
  • Engine no longer locates spiders based on domains;
  • "Spider middleware output direction" step was missing - this one was bugging me since 2013 :)

See also: GH-1569.

This overview is still incomplete because Extensions and Scraper are not explained.

* spiders don't have to work on specific domains;
* explain what to use Downloader middleware for
  and what to use Spider middleware for;
* Engine no longer locates spiders based on domains;
* "Spider middleware output direction" step was missing.

See also: GH-1569.
@kmike kmike added the docs label Mar 25, 2016
@codecov-io
Copy link

@codecov-io codecov-io commented Mar 25, 2016

Current coverage is 83.18%

Merging #1879 into master will not affect coverage as of cd732c1

Powered by Codecov. Updated on successful CI builds.


9. The process repeats (from step 2) until there are no more requests from the
8. The process repeats (from step 2) until there are no more requests from the
Scheduler, and the Engine closes the domain.

This comment has been minimized.

@redapple

redapple Mar 25, 2016
Contributor

I admit I never read that in detail. Now I have :) It's clear.

except this "the Engine closes the domain.". what does it mean?

This comment has been minimized.

@kmike

kmike Mar 25, 2016
Author Member

A good catch; I think that's another artifact from the past.

@redapple
Copy link
Contributor

@redapple redapple commented Mar 25, 2016

I like the "Use ___ if you want to ___ or ___ or ____" style.

In a another PR perhaps, I think we should also add simplified sequence diagrams for the data flow, so as to put the numbered steps in context.

@kmike
Copy link
Member Author

@kmike kmike commented Mar 25, 2016

@redapple there is a picture for this data flow in docs (http://doc.scrapy.org/en/latest/topics/architecture.html).

@redapple
Copy link
Contributor

@redapple redapple commented Mar 25, 2016

I know @kmike :) but I still think it could be clearer. I'll suggest something concrete

@eliasdorneles
Copy link
Member

@eliasdorneles eliasdorneles commented Mar 30, 2016

Nice PR, LGTM!

@redapple redapple merged commit 9d8c368 into master Mar 31, 2016
2 checks passed
2 checks passed
codecov/patch coverage not affected
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kmike kmike deleted the scrapy-arch-docs branch Apr 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.