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

[MRG] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure or warning #2632

Merged
merged 4 commits into from Mar 9, 2017

Conversation

@redapple
Copy link
Contributor

commented Mar 7, 2017

Related to #2433 (comment)

@codecov-io

This comment has been minimized.

Copy link

commented Mar 9, 2017

Codecov Report

Merging #2632 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2632      +/-   ##
==========================================
+ Coverage   84.17%   84.18%   +<.01%     
==========================================
  Files         162      162              
  Lines        9088     9093       +5     
  Branches     1347     1348       +1     
==========================================
+ Hits         7650     7655       +5     
  Misses       1177     1177              
  Partials      261      261
Impacted Files Coverage Δ
scrapy/commands/runspider.py 73.52% <100%> (+0.39%)
scrapy/commands/list.py 80% <100%> (ø)
scrapy/settings/default_settings.py 98.63% <100%> (ø)
scrapy/commands/version.py 91.11% <100%> (ø)
scrapy/commands/settings.py 72.72% <100%> (ø)
scrapy/commands/startproject.py 100% <100%> (ø)
scrapy/spiderloader.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3f8f3d...9628a73. Read the comment docs.

@redapple redapple changed the title [WIP] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure and warning Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure and warning Mar 9, 2017

@redapple

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2017

@vshlapakov , would this change work for you?

@redapple redapple added this to the v1.4 milestone Mar 9, 2017

@@ -250,6 +250,7 @@
SCHEDULER_PRIORITY_QUEUE = 'queuelib.PriorityQueue'

SPIDER_LOADER_CLASS = 'scrapy.spiderloader.SpiderLoader'
SPIDER_LOADER_WARN_ONLY = False

This comment has been minimized.

Copy link
@kmike

kmike Mar 9, 2017

Member

Could you please add docs for this option? Or are you waiting for @vshlapakov's feedback?

This comment has been minimized.

Copy link
@redapple

redapple Mar 9, 2017

Author Contributor

@kmike , docs updated.

@redapple

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2017

Sorry, I'll add some doc.
But yes, I was also asking if this solution would work (name of option still up of course)

@redapple redapple changed the title Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure and warning [WIP] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure and warning Mar 9, 2017

@vshlapakov

This comment has been minimized.

Copy link

commented Mar 9, 2017

@redapple The changes looks good to me, thanks Paul! cc @chekunkov

@redapple redapple changed the title [WIP] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure and warning [WIP] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure or warning Mar 9, 2017

@chekunkov

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

LGTM

@redapple

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2017

phew, f2ac24e was the whole point of this... ;)

@redapple redapple changed the title [WIP] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure or warning [MRG] Add SPIDER_LOADER_WARN_ONLY to toggle between spiderloader failure or warning Mar 9, 2017

@kmike kmike merged commit 9c69e90 into scrapy:master Mar 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kmike

This comment has been minimized.

Copy link
Member

commented Mar 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.