scrapinghub / spidermon

Scrapy Extension for monitoring spiders execution.
https://spidermon.readthedocs.io
BSD 3-Clause "New" or "Revised" License
528 stars 96 forks source link

Add UnwantedHTTPCodesFamilyMonitor Scrapy Monitor #311

Closed shafiq-muhammad closed 2 years ago

shafiq-muhammad commented 2 years ago

Added UnwantedHTTPCodesFamilyMonitor monitor to spidermon

codecov[bot] commented 2 years ago

Codecov Report

Merging #311 (d7655d1) into master (3c606ad) will increase coverage by 0.07%. The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #311      +/-   ##
==========================================
+ Coverage   74.04%   74.11%   +0.07%     
==========================================
  Files          68       68              
  Lines        3028     3044      +16     
  Branches      465      470       +5     
==========================================
+ Hits         2242     2256      +14     
- Misses        723      724       +1     
- Partials       63       64       +1     
Impacted Files Coverage Δ
spidermon/contrib/scrapy/monitors.py 97.22% <87.50%> (-0.95%) :arrow_down:

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 3c606ad...d7655d1. Read the comment docs.

rennerocha commented 2 years ago

This spider is closely related to the existing UnwantedHTTPCodesMonitor . I will look with more care, maybe we can use it instead of creating a new one.

curita commented 2 years ago

We have talked with Renne about this and it looks like it might be better to stick with the functionality of the existing one, as declaring the specific allowed http error codes is more explicit than declaring whole groups of allowed error codes.

@rennerocha Maybe it would be better to redefine and/or close the idea from https://github.com/scrapinghub/spidermon/issues/302 (which this PR implements) to get more clarity on the feature request.

rennerocha commented 2 years ago

@shafiq-muhammad thanks for the PR. Sorry for the delay to review it. I checked how the proposed monitor works and it has the same goal as the UnwantedHTTPCodesMonitor monitor that already exists in Spidermon. The main difference is that the existing monitor ask explicitly which are the codes that we want to monitor.

Explicit is better than implicit (as Zen of Python tells), so I think that asking for the threshold for specific status codes are more useful as a built-in monitor. If a use case demands another kind of thresholds, I think it will work better as a custom project-specific monitor.