Use a custom DropItem subclass #126
Conversation
…wer the message severity
Gallaecio
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #126 +/- ##
=======================================
Coverage ? 97.58%
=======================================
Files ? 63
Lines ? 2364
Branches ? 0
=======================================
Hits ? 2307
Misses ? 57
Partials ? 0
|
zyte_common_items/pipelines.py
Outdated
| @@ -56,6 +61,11 @@ def process_item(self, item, spider): | |||
| return ae.downgrade(item) | |||
|
|
|||
|
|
|||
| class InfoDropItem(DropItem): | |||
| """DropItem subclass for items that should be dropped with an INFO message | |||
| (instead of the default WARNING message).""" | |||
There was a problem hiding this comment.
If this class is defined here, we should define the log formatter here as well (not in zyte-spider-templates-project).
There was a problem hiding this comment.
Am I correct to assume that we should also enable the new formatter in the library add-on, and not in the project?
There was a problem hiding this comment.
Yes, it seems we should do it here.
There was a problem hiding this comment.
I meant the zyte-spider-templates library, given there is no add-on here. Or are you thinking we should add an add-on to zyte-common-items?
There was a problem hiding this comment.
You're adding it in #124 already :)
There was a problem hiding this comment.
As for the settings, it seems it'd be better to keep them in the project - as we discussed elsewhere, it might be simpler for addons not to configure other libraries. But I don't have a stong opinion on this.
| (instead of the default WARNING message).""" | ||
|
|
||
|
|
||
| class ZyteLogFormatter(LogFormatter): |
There was a problem hiding this comment.
Alternatively, we could allow to configure the severity level at Scrapy level via settings given that this is not the first time this has come up.
There was a problem hiding this comment.
@kmike mentioned somewhere that maybe we should allow components to set their desired log level in the exception, e.g. raise DropItem(severity=INFO).
Maybe we could have a setting to implement the overall default log level, allow components to set a custom value through severity, and let components choose whether or not they respect the setting, force a given value, or provide their own setting to override their own request droppings.
There was a problem hiding this comment.
To enable custom logging to cleanly lower the message severity.