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 support for custom exporter class #6273
base: master
Are you sure you want to change the base?
add support for custom exporter class #6273
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6273 +/- ##
==========================================
+ Coverage 88.67% 88.88% +0.20%
==========================================
Files 161 161
Lines 11788 11971 +183
Branches 1912 1929 +17
==========================================
+ Hits 10453 10640 +187
+ Misses 1007 980 -27
- Partials 328 351 +23 |
Hello, I like the idea of using the Feed Exporters rather than Feed Storages to do the actual exporting. You don't actually need to write any additional code to support this right now, this should be similiar to your solution: FEED = {
"stdout://": {
"format": "StreamingAPI"
}
}
FEED_EXPORTERS = {
"StreamingAPI": "StreamingAPIExporter"
} The methods However, there are several downsides that we would need to figure out:
|
Wouldn't this be something that the developer needs to take care of on their customized exporter code? If we're allowing the customization of
Couldn't all this be managed by the custom exporter class itself? If we assume that there's only a limited amount of exceptions that are covered by the engine, wouldn't this be something managed by the code itself? I think those questions can be answered better if we have an example of a customized exporter that matches those requirements. @VMRuiz what about the Airtable exporter we've previously discussed? |
70e95cc
to
e208f82
Compare
Reopening this since it closed when I reset the branch, because the changes were not necessary. I tried @VMRuiz proposal for custom exporters and runs just fine.
The failing tests confuse me, because from the error they seem to be related to |
Turns out without changing any Scrapy code I get the same behavior by calling |
…om the custom exporter side
@Gallaecio @wRAR , Do you think using deferToThread is appropriate here, or could there be performance issues if there are too many calls? |
I suggest using |
Opening this PR to discuss the implementation of adding support for custom exporter classes. This comes from the idea of adding stream exporters that don't rely on local storage. The usage I had in mind would be something like:
The batching would work the same, since a new instance would be created for every new slot and the custom exporter can handle the queuing, etc.
After this, reusable exporters for widely used services can be implemented.
Missing tests and docs.