-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Add failed and success count stats to feedstorage backends #4850
Add failed and success count stats to feedstorage backends #4850
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4850 +/- ##
=======================================
Coverage 87.86% 87.87%
=======================================
Files 160 160
Lines 9749 9755 +6
Branches 1439 1437 -2
=======================================
+ Hits 8566 8572 +6
Misses 926 926
Partials 257 257
|
14b207b
to
44cc533
Compare
CI failed, im not sure why -> https://travis-ci.org/github/scrapy/scrapy/jobs/738154019#L182 can help me here please @Gallaecio |
@joaquingx it looks like a temporary problem, I think @Gallaecio should be able to restart the job, but I think you can too if you push a commit again |
71713fd
to
c5f06de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
This is great, thanks. I was wondering if you would consider changing the approach regarding this small bit: diff --git tests/test_feedexport.py tests/test_feedexport.py
index 1a77cec7..2ce8d7ff 100644
--- tests/test_feedexport.py
+++ tests/test_feedexport.py
@@ -8,6 +8,7 @@ import tempfile
import warnings
from abc import ABC, abstractmethod
from collections import defaultdict
+from contextlib import ExitStack
from io import BytesIO
from logging import getLogger
from pathlib import Path
@@ -782,8 +783,11 @@ class FeedExportTest(FeedExportTestBase):
},
}
crawler = get_crawler(ItemSpider, settings)
- with MockServer() as mockserver, \
- mock.patch("scrapy.extensions.feedexport.FileFeedStorage.store", side_effect=KeyError("foo")):
+ with ExitStack() as stack:
+ mockserver = stack.enter_context(MockServer())
+ stack.enter_context(
+ mock.patch("scrapy.extensions.feedexport.FileFeedStorage.store", side_effect=KeyError("foo"))
+ )
yield crawler.crawl(mockserver=mockserver)
self.assertIn("feedexport/failed_count/FileFeedStorage", crawler.stats.get_stats())
self.assertEqual(crawler.stats.get_value("feedexport/failed_count/FileFeedStorage"), 1) I took it from this SO answer. Please excuse my nitpicking, this is not a blocker in any sense, I just really don't like backslash break lines 😅 |
@elacuesta Hey, thanks, it would improve the code. Changes are done! |
Resolves #3947
Example:
if S3 fails to store, stats will be:
Ready to review 😄