Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Mentioned here: #705872: HTTPFetcher autodiscovery, it seems a little odd to keep it as one class. It essentially is a two mode class, one is 'file mode' and the other is 'url mode'. I purpose:
1.) FeedsImportBatch becomes abstract
2.) all methods that do an (if ($this->url) { Do X} elseif ($this->file_path) {do Y} become abstract methods
3.) create two classes FeedsImportBatchUrl and FeedsImportBatchFile
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 708228-6_break_up.patch | 8.12 KB | alex_b |
| #1 | feeds_708228-1.patch | 5.56 KB | Scott Reynolds |
Comments
Comment #1
Scott Reynolds commentedAttach is a patch. There is a strange field of the FeedsBatchImporter $this->file and its only used in getURL(). I ported it over as is into the FeedsBatchImportFile class.
FeedsBatchImportUrl probably needs
For semantics and to keep that from being a 'public' property. Just didn't feel like re rolling.
And should consider doing a try/catch around $this->import. Maybe in that case $this->raw gets set to FALSE. That way when FeedsBatchImportURL won't cause multiple HTTP requests to a bad URL.
Comment #2
fereira commentedWhat are the chances that this patch will get committed?
To me, it seems like to make FeedsImportBatch as an abstract class, because as it is now it assumes that the feed is coming from either or URL or a File. In my case, I am consuming a feed from another module (sparql) and could see creating an FeedsImportBatchFunction class which did an override of the import function such that it called a function (perhaps via module_invoke()) from another module to set the "raw" content.
Comment #3
alex_b commented#2: you would rather create a FeedsImportSparql class than trying to make this generic.
I really would like to get this patch committed. Need to carve out some time for review though. Recent commit of Batch API might collide with this patch btw. Does it still apply?
Comment #4
Scott Reynolds commentedyes this will need a reroll with that batch api commit.
Comment #5
alex_b commentedI'm going to take a shot at this now.
Comment #6
alex_b commentedOk, this is passing all tests now, will commit this soon.
- I used slightly different names for the import batch classes for file and http fetcher (FeedsHTTPBatch and FeedsFileBatch).
- Moved FeedHTTPBatch into FeedsHTTPFetcher and FeedsFileBatch into FeedsFileFetcher.
Comment #7
alex_b commentedCommitted. http://drupal.org/cvs?commit=330434
Thank you.