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+1] fix: do not catch system exceptions like KeyboardInterrupt #3726

Merged
merged 1 commit into from Apr 6, 2019

Conversation

@ankostis
Copy link
Contributor

@ankostis ankostis commented Apr 5, 2019

I noticed that occasionally my Ctrl+C is ignored. In general this happens when using plain except:. In this case, you probably do not want to catch system-exceptions like KeyboardInterrupt.

@Gallaecio Gallaecio changed the title fix: do not catch system exceptions like KeyboardInterrupt [MRG+1] fix: do not catch system exceptions like KeyboardInterrupt Apr 5, 2019
@Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Apr 5, 2019

Thanks!

I’m thinking we should probably use pytest-pylint to detect this kind of issues. I’ll add that to my personal to-do.

@codecov
Copy link

@codecov codecov bot commented Apr 5, 2019

Codecov Report

Merging #3726 into master will not change coverage.
The diff coverage is 80%.

@@           Coverage Diff           @@
##           master    #3726   +/-   ##
=======================================
  Coverage   84.89%   84.89%           
=======================================
  Files         168      168           
  Lines        9521     9521           
  Branches     1418     1418           
=======================================
  Hits         8083     8083           
  Misses       1184     1184           
  Partials      254      254
Impacted Files Coverage Δ
scrapy/contracts/__init__.py 81.3% <0%> (ø) ⬆️
scrapy/utils/misc.py 95.71% <100%> (ø) ⬆️
scrapy/utils/defer.py 96.49% <100%> (ø) ⬆️
scrapy/core/spidermw.py 97.53% <100%> (ø) ⬆️

1 similar comment
@codecov
Copy link

@codecov codecov bot commented Apr 5, 2019

Codecov Report

Merging #3726 into master will not change coverage.
The diff coverage is 80%.

@@           Coverage Diff           @@
##           master    #3726   +/-   ##
=======================================
  Coverage   84.89%   84.89%           
=======================================
  Files         168      168           
  Lines        9521     9521           
  Branches     1418     1418           
=======================================
  Hits         8083     8083           
  Misses       1184     1184           
  Partials      254      254
Impacted Files Coverage Δ
scrapy/contracts/__init__.py 81.3% <0%> (ø) ⬆️
scrapy/utils/misc.py 95.71% <100%> (ø) ⬆️
scrapy/utils/defer.py 96.49% <100%> (ø) ⬆️
scrapy/core/spidermw.py 97.53% <100%> (ø) ⬆️

@kmike
Copy link
Member

@kmike kmike commented Apr 6, 2019

Thanks @ankostis!

@kmike kmike merged commit f816375 into scrapy:master Apr 6, 2019
2 of 3 checks passed
@kmike kmike added this to the v1.7 milestone Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants