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

Use BaseStatMonitor to implement built-in downloader exception monitor #334

Conversation

rennerocha
Copy link
Collaborator

@rennerocha rennerocha commented Feb 8, 2022

All built-in monitors should use BaseStatMonitor when possible. So this PR will continue converting them. Also improving the documentation with a bit more details about the monitor.

@rennerocha rennerocha added this to the 1.17.0 milestone Feb 8, 2022
@codecov
Copy link

codecov bot commented Feb 8, 2022

Codecov Report

Merging #334 (fa51494) into master (088f801) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #334      +/-   ##
==========================================
- Coverage   74.11%   74.06%   -0.05%     
==========================================
  Files          68       68              
  Lines        3036     3031       -5     
  Branches      466      465       -1     
==========================================
- Hits         2250     2245       -5     
  Misses        723      723              
  Partials       63       63              
Impacted Files Coverage Δ
spidermon/contrib/scrapy/monitors.py 98.20% <100.00%> (-0.06%) ⬇️

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 088f801...fa51494. Read the comment docs.

Copy link
Contributor

@andersonberg andersonberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, good docstrings

@rennerocha rennerocha force-pushed the downloader-exception-new-base-stat-class branch from a136b12 to 8308246 Compare February 10, 2022 13:40
As `downloader/exception_count` statistic is not always available (it
happens only if there is some exception), we should skip the monitor if
the stat is not there.
@rennerocha rennerocha force-pushed the downloader-exception-new-base-stat-class branch from 8308246 to fa51494 Compare February 10, 2022 14:26
@rennerocha rennerocha merged commit 99c1d81 into scrapinghub:master Feb 10, 2022
@rennerocha rennerocha deleted the downloader-exception-new-base-stat-class branch February 10, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants