i saw this: https://drupal.org/node/833066
and i realize that you can also (i think) touch feeds_process_limit

...but why not simply expose this setting on the feeds admin via the UI? should people really be mucking about in the settings file just to do something like:

a) limit total number of items per import (all feeds)
b) limit total number of items imported by one feed item (maybe this is exposed alongside the Link field when creating a feed?)

just my own opinion, but i'd love to see it as a feature! "Limit per batch" (all feed items) or "Allow to Limit by Feed Item" and then expose the setting in the individual feed items like "don't download more than (Dropdown) items"

CommentFileSizeAuthor
#6 feeds-expose_process_limit-2124281-6.patch3.84 KBufku
#3 feeds-expose_process_limit-2124281-3.patch3.97 KBufku
#1 feeds-expose_process_limit-2124281-1.patch1.26 KBufku
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ufku’s picture

ufku CreditAttribution: ufku commented
Version: 7.x-2.0-alpha8 » 7.x-2.x-dev
Issue summary: View changes
Status: Active » Needs review
FileSize
feeds-expose_process_limit-2124281-1.patch1.26 KB

The patch exposes the limit in Processor settings. The default value is still variable_get('feeds_process_limit', FEEDS_PROCESS_LIMIT)

Status: Needs review » Needs work

The last submitted patch, 1: feeds-expose_process_limit-2124281-1.patch, failed testing.

ufku’s picture

ufku CreditAttribution: ufku commented
FileSize
feeds-expose_process_limit-2124281-3.patch3.97 KB

Fixed FeedsEntityProcessor::configDefaults() and updated tests to use the exposed limit.

ufku’s picture

ufku CreditAttribution: ufku commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: feeds-expose_process_limit-2124281-3.patch, failed testing.

ufku’s picture

ufku CreditAttribution: ufku commented
Status: Needs work » Needs review
FileSize
feeds-expose_process_limit-2124281-6.patch3.84 KB

It seems FeedsEntityProcessor::configDefaults() should not call parent::configDefaults()

MegaChriz’s picture

MegaChriz CreditAttribution: MegaChriz commented
Related issues: +#1909974: Limit number of feed items to import

@ufku
FeedsEntityProcessor::configDefaults() can not call parent::configDefaults(), because FeedsProcessor::configDefaults() calls ::bundle() and the FeedsEntityProcessor's implementation of the bundle() method needs to know the importer's config in order to determine the bundle, so calling the parent's configDefaults() method would trigger an infinite loop:

  1. FeedsEntityProcessor::bundle()
  2. FeedsEntityProcessor::entityType()
  3. FeedsPlugin::pluginDefinition()
  4. feeds_importer()
  5. FeedsConfigurable::instance()
  6. FeedsConfigurable::__construct()
  7. FeedsEntityProcessor::configDefaults()
anou’s picture

anou
Primary language French
Location Collonges au Mont d'Or
CreditAttribution: anou as a volunteer commented

Sorry to tell you this but patch from #6 doesn't work anymore... I'm also trying to solve this feature request.

Now FeedsEntityProcessor.inc has been removed from Feeds module and is a stand alone module : Feeds entity processor

MegaChriz’s picture

MegaChriz CreditAttribution: MegaChriz as a volunteer commented
Status: Needs review » Needs work

Yes, the patch from #6 needs a reroll. More specifically, the following line should be removed from the patch:

+++ b/plugins/FeedsEntityProcessor.inc
@@ -135,6 +135,7 @@ class FeedsEntityProcessor extends FeedsProcessor {
+      'limit' => variable_get('feeds_process_limit', FEEDS_PROCESS_LIMIT),

Also note that some parsers ignore this setting. And the patch will only have effect on the CSV parser. Parsers provided by contrib modules will continue to use "feeds_process_limit" setting (or ignore the setting at all, as already said). As such, I think the setting should be moved to the CSV parser.

MegaChriz’s picture

MegaChriz CreditAttribution: MegaChriz as a volunteer commented

Oops, spoke to soon, I saw a comment in the patch mentioning the CSV parser, but it wasn't actually in the CSV parser.

That means the patch will need a reroll, but the description of the setting should also say that some parsers will ignore it.

sbydrupal’s picture

sbydrupal CreditAttribution: sbydrupal commented

Can you please comment on #10 at https://www.drupal.org/node/1909974 whether it is possible to
solve ? I don't want to replicate same comment in this thread. Thanks