scrapinghub / exporters

Exporters is an extensible export pipeline library that supports filter, transform and several sources and destinations
BSD 3-Clause "New" or "Revised" License
40 stars 10 forks source link

Add Multiple filters #325

Open raphapassini opened 8 years ago

raphapassini commented 8 years ago

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?

raphapassini commented 8 years ago

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)

raphapassini commented 8 years ago

What has changed in the last commit?

raphapassini commented 8 years ago

@eliasdorneles please, take a look in my last update when you have time :)

tsrdatatech commented 7 years ago

@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!