exporters
exporters copied to clipboard
Add Multiple filters
First of all thank you for the review @eliasdorneles, I think it makes a lot of sense. I'll work on a new implementation, did you have time to check the tests? Do you think they cover enough what we expected this new Filter to do?
Hello @eliasdorneles please take a look at my last commit - https://github.com/scrapinghub/exporters/pull/325/commits/98f3f698d4306974a95b6deb1bce7e846b901205, I changed the implementation in the way you suggest in your last comment (https://github.com/scrapinghub/exporters/pull/325#discussion-diff-72542313)
What has changed in the last commit?
- Changed the implementation to support nested filters
- Throw a
ValueError
when try to load invalid filters. - Update tests to cover nested filters
@eliasdorneles please, take a look in my last update when you have time :)
@raphapassini After reviewing this a bit there are some changes still needed in order for this to work. The code looks fine. I am a little confused on some of the tests. Are we testing for different types of filters together here such as key_value_filters and pythonexp_filter rather than 2 or 3 that are the same?
Here are some items which will need to implemented for this to actually work:
You will need to add the MultipleFilter to this section https://github.com/raphapassini/exporters/blob/737785b86b2fa2d8e66a97aa4eb1d0baadd721a1/exporters/filters/init.py
You will also need to add some code to handle processing the filters section from the config in order for it to be used. Check https://github.com/raphapassini/exporters/blob/737785b86b2fa2d8e66a97aa4eb1d0baadd721a1/exporters/exporter_config.py#L23 how it is done for the filter section. There will be a few other spots in that file that will probably require some changes in order for this module to get loaded. This function https://github.com/raphapassini/exporters/blob/737785b86b2fa2d8e66a97aa4eb1d0baadd721a1/exporters/exporter_config.py#L48 will need to be modified to handle the list of filters rather than just one.
Once these are done then I think we can do some real tests to see if it works with real configurations.
Thanks!