I have several import nodes from the same Feed Importer configuration. I want to override the import_period per node, because sometimes I want to disable fetching for a node or set a different period when the feed should be fetched.

Proposed solution: add a import_period override to FeedsHTTPFetcher.

CommentFileSizeAuthor
#7 interdiff.txt663 bytesklausi
#7 feeds-period-1984982-7.patch2.1 KBklausi
#4 feeds-period-1984982-4.patch2.15 KBklausi
#3 feeds-period-1984982-3.patch2.25 KBklausi
#3 feeds-period-1984982-3-interdiff.txt686 bytesklausi
#1 feeds-period-1984982-1.patch2.21 KBklausi

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Active » Needs review
StatusFileSize
new feeds-period-1984982-1.patch2.21 KB

Patch attached.

Status: Needs review » Needs work

The last submitted patch, feeds-period-1984982-1.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review
StatusFileSize
new feeds-period-1984982-3-interdiff.txt686 bytes
new feeds-period-1984982-3.patch2.25 KB

Added an isset() for the new setting to avoid PHP notices for old configurations.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes
StatusFileSize
new feeds-period-1984982-4.patch2.15 KB

Now that we use queue scheduling directly in scheduleImport() of FeedsSource we need to be careful to only invoke schedule if the batch is complete.

twistor’s picture

twistor commented
Status: Needs review » Needs work

Hmm... I'm not sure about this. I've been avoiding adding more options at this point.

+++ b/plugins/FeedsHTTPFetcher.inc
@@ -237,6 +255,14 @@ class FeedsHTTPFetcher extends FeedsFetcher {
+    // Apply overridden import period and re-schedule job if necessary.
+    $progress = $source->progressImporting();
+    $config = $source->getConfigFor($this);
+    if (isset($config['import_period']) && $config['import_period'] !== 'importer'
+      && $progress === FEEDS_BATCH_COMPLETE
+    ) {
+      $source->schedule();

If we can only re-schedule when the batch is complete, then form should be disabled if the progress is running. However, it should always be safe to call $source->schedule(), so does this means we introduced a bug in scheduleImport()?

Still not sure, but at a minimum, this would have to be optional. I don't want everyone to get a new form option in everybody's face. This could be done in a contrib module.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

It is not safe to call $source->schedule() now, because we add items to the queue if a batch is in progress. What happened to me in this case is that every import step triggers a source safe which had triggered a schedule() with this patch. So I got 32.000 queue items for one import node with this patch, meaning that the import would run amok and execute all the time with cron.

So maybe we should consider the earlier patch version in #1231332: periodic import imports only one file per cron, which only adds to the queue in the queue worker.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
StatusFileSize
new feeds-period-1984982-7.patch2.1 KB
new interdiff.txt663 bytes

Just updating the patch here because FEEDS_BATCH_COMPLETE is now a float.

bluegeek9’s picture

bluegeek9 commented
Status: Needs work » Closed (outdated)
//www.flaticon.com/free-icons/thank-you Thank you for your contribution!

Unfortunately, Drupal 7 is End of Life and no longer supported. We strongly encourage you to upgrade to a supported version of Drupal.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.