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

make dupefilter support create from spider #83

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Conversation

zoyanhui
Copy link

@zoyanhui zoyanhui commented Jan 5, 2017

redis dupefilter is based on spider, it may suppport from spider.

@codecov-io
Copy link

codecov-io commented Jan 5, 2017

Current coverage is 96.57% (diff: 100%)

Merging #83 into master will increase coverage by 0.30%

@@             master        #83   diff @@
==========================================
  Files            17         17          
  Lines           696        700     +4   
  Methods           0          0          
  Messages          0          0          
  Branches         55         55          
==========================================
+ Hits            670        676     +6   
+ Misses           10          8     -2   
  Partials         16         16          

Powered by Codecov. Last update 500039b...384996d

@rmax rmax self-assigned this Feb 13, 2017
@rmax
Copy link
Owner

rmax commented Feb 13, 2017

Thanks for your PR. I will think about this as it will break custom dupefilter classes.

@tyong920
Copy link

@zoyanhui I think this PR is NOT needed. If i'm right, you might want to make it possible to set specific settings for scrapy_redis in spider. To do that, you could just set them in custom_settings in your spider.
A simple

custom_settings = {
        ... ...
        'SCHEDULER_DUPEFILTER_KEY' = '... ...',
        ...

@rmax rmax merged commit 0403645 into rmax:master Oct 23, 2017
@Germey Germey mentioned this pull request Mar 27, 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.

5 participants