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

GSoC Per-spider settings #854

Closed
wants to merge 23 commits into from
Closed

GSoC Per-spider settings #854

wants to merge 23 commits into from

Conversation

@curita
Copy link
Member

@curita curita commented Aug 13, 2014

Per-spider settings task of the 2014 GSoC, implemented on top of the #816 pull request.

This changes are based on the SEP#19.

@curita curita mentioned this pull request Aug 13, 2014
30 of 30 tasks complete
@curita
Copy link
Member Author

@curita curita commented Aug 14, 2014

@dangra I've fixed both issues and rebased the new changes in the api-cleanup pull request on this branch.

@dangra

This comment has been minimized.

Copy link

@dangra dangra commented on tests/test_settings/__init__.py in 70f2010 Aug 14, 2014

sorry for been late, but I think RuntimeError is better than TypeError in this case, the latter is much more common mistake. @kmike what do you think?

This comment has been minimized.

Copy link

@kmike kmike replied Aug 14, 2014

I suggested TypeError because this is what immutable containers raise, e.g.

In [1]: (3,2)[0] = 1
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-cab3fa073914> in <module>()
----> 1 (3,2)[0] = 1

TypeError: 'tuple' object does not support item assignment

This comment has been minimized.

Copy link

@dangra dangra replied Aug 14, 2014

def spiders(self):
if not hasattr(self, '_spiders'):
warnings.warn("Crawler.spiders is deprecated, use "
"CrawlerRunner.spiders or instantiate "

This comment has been minimized.

@kmike

kmike Aug 14, 2014
Member

It seems that CrawlerRunner.spiders is not documented. I'm not sure I like that CrawlerSpider knows about SpiderManager (and it can be used without it, as shown in your new example).

@dangra
Copy link
Member

@dangra dangra commented Sep 2, 2014

Merged by 5daa147

@dangra dangra closed this Sep 2, 2014
@curita curita deleted the curita:per-spider-settings branch Sep 6, 2014
@raintan

This comment has been minimized.

Copy link

@raintan raintan commented on docs/topics/practices.rst in 3547ca6 Nov 14, 2015

This comment should be removed or changed to: # 'testspider' is the name...

This comment has been minimized.

Copy link
Member Author

@curita curita replied Nov 16, 2015

Not sure if I'm misunderstanding what you're saying @raintan, but testspiders is the name of the project (which you can found here: https://github.com/scrapinghub/testspiders, should be linked in the docs as well), and followall is the name of the spider inside that project that we're using (You can check the code in the line below).

This comment has been minimized.

Copy link

@raintan raintan replied Nov 16, 2015

In the latest version of the docs, followall isn't part of the code example anymore.
see: http://doc.scrapy.org/en/1.0/topics/practices.html right after "What follows is a working example of how to do that, using the testspiders project as example."
(I'm new here, so I'm not sure if this was the proper place to put this comment.)

This comment has been minimized.

Copy link
Member Author

@curita curita replied Nov 16, 2015

Oh, I see what you meant, thanks for the notice!

This issue seems to be fixed already (that's why these docs are correct), but the latest available version of scrapy docs is this one: http://doc.scrapy.org/en/master/topics/practices.html (I know, rather confusing that is not latest). We'll see to backport the fix to 1.0 accordantly.

Generally opening an issue for any kind of bugs is the way to go, it's more flexible and github offers more features for them (can be referenced easier, we can modify their status to track the work done on them, milestones can be assigned to each of them, etc), so feel free to open an issue for your next bug report :)

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.