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

CrawlerProcess documentation #1190

Merged
merged 6 commits into from Apr 30, 2015
Merged

CrawlerProcess documentation #1190

merged 6 commits into from Apr 30, 2015

Conversation

@curita
Copy link
Member

@curita curita commented Apr 27, 2015

As discussed in #1146, CrawlerProcess is generally a better fit than CrawlerRunner for scripting (see #1001 for example), so I made a couple of changes to reflect that in the documentation.

Changes made in this PR:

  • Use Sphinx autodocs for CrawlerRunner so CrawlerProcess doesn't need to document again the inherited methods
  • Document CrawlerProcess
  • Add CrawlerProcess examples to "Run Scrapy from a script"
  • Favor CrawlerProcess over CrawlerRunner in the "Run Scrapy from a script" section
@curita curita added the docs label Apr 27, 2015
@curita curita added this to the Scrapy 1.0 milestone Apr 27, 2015
return defer.DeferredList([c.stop() for c in list(self.crawlers)])

@defer.inlineCallbacks
def join(self):
"""Wait for all managed crawlers to complete"""
"""
join()

This comment has been minimized.

@kmike

kmike Apr 27, 2015
Member

this comment looks strange

This comment has been minimized.

@curita

curita Apr 27, 2015
Author Member

The first line is there so Sphinx can get the right signature of join(), because of the decorator it's computed as join(*args, **kwargs) otherwise.

This comment has been minimized.

@kmike

kmike Apr 27, 2015
Member

you can write a signature in the docs itself, see http://sphinx-doc.org/ext/autodoc.html#directive-automethod

This comment has been minimized.

@curita

curita Apr 27, 2015
Author Member

Yes, but in that case I'd have to add it both to CrawlerRunner and CrawlerProcess (this isn't inherited), and I would have to exclude join() from :members: and :inherited-members: because it would get duplicated otherwise. Looks strange but I think I prefer keeping the signature where it is right now.

This comment has been minimized.

@kmike

kmike Apr 27, 2015
Member

Listing :members: explicitly sounds good to me - it allows us to add docstrings to methods we don't want to promote, and these classes don't have lots of methods, so it is not a lot of typing. IMHO the less tricks the better :)

"""
join()
Wait for all managed :attr:`crawlers` to complete.

This comment has been minimized.

@kmike

kmike Apr 27, 2015
Member

I think we should mention a Deferred is returned, because this method itself doesn't wait for all crawlers to complete.

This comment has been minimized.

@curita

curita Apr 27, 2015
Author Member

I agree with mentioning the Deferred, I just changed it.

@kmike
Copy link
Member

@kmike kmike commented Apr 27, 2015

👍

@curita curita force-pushed the curita:crawlerprocess-docs branch 2 times, most recently from 04ddb10 to faa5184 Apr 27, 2015
@curita curita force-pushed the curita:crawlerprocess-docs branch from faa5184 to c1634e4 Apr 30, 2015
@curita
Copy link
Member Author

@curita curita commented Apr 30, 2015

I'm merging this PR after rebasing master as discussed in the meeting. Changes to remove the signature of join() from the docstring are going to be addressed in another PR.

curita added a commit that referenced this pull request Apr 30, 2015
CrawlerProcess documentation
@curita curita merged commit 5b884d1 into scrapy:master Apr 30, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@curita curita deleted the curita:crawlerprocess-docs branch Apr 30, 2015
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

2 participants