There's no reason we need to schedule a job with job_scheduler for background tasks.

We can just use the Queue API directly.

CommentFileSizeAuthor
#28 interdiff-2630694-27-28.txt1.89 KBMegaChriz
#28 feeds-background-queue-2630694-28.patch25.05 KBMegaChriz
#27 interdiff-2630694-25-27.txt3.45 KBMegaChriz
#27 feeds-background-queue-2630694-27.patch24.89 KBMegaChriz
#25 interdiff-2630694-24-25.txt11.1 KBMegaChriz
#25 feeds-background-queue-2630694-25.patch24.71 KBMegaChriz
#24 interdiff-2630694-23-24.txt7.15 KBMegaChriz
#24 feeds-background-queue-2630694-24.patch14.99 KBMegaChriz
#23 interdiff-2630694-22-23.txt7.74 KBMegaChriz
#23 feeds-background-queue-2630694-23.patch9.93 KBMegaChriz
#22 interdiff-2630694-13-22.txt711 bytesMegaChriz
#22 feeds-background-queue-2630694-22.patch4.24 KBMegaChriz
#13 feeds-background-queue-2630694-13.patch4.23 KBtwistor
#11 feeds-background-queue-2630694-11.patch4.23 KBtwistor
#9 feeds-background-queue-2630694-9.patch3.3 KBtwistor
#6 feeds-background-queue-2630694-6.patch3.22 KBtwistor
#4 feeds-clear-queue-2630694-4.patch1.72 KBtwistor
#2 feeds-clear-queue-2630694-1.patch998 bytestwistor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

twistor created an issue. See original summary.

twistor’s picture

Status: Active » Needs review
FileSize
998 bytes
twistor’s picture

Issue summary: View changes
twistor’s picture

Wrong patch.

twistor’s picture

twistor’s picture

Title: Remove job_scheduler usage for clearing in background. » Run background jobs directly in queue.
FileSize
3.22 KB
twistor’s picture

Issue summary: View changes
twistor’s picture

twistor’s picture

Leaving the feeds_source_clear definition in case any existing jobs are running.

The last submitted patch, 2: feeds-clear-queue-2630694-1.patch, failed testing.

twistor’s picture

Status: Needs review » Needs work

The last submitted patch, 11: feeds-background-queue-2630694-11.patch, failed testing.

twistor’s picture

Status: Needs work » Needs review
FileSize
4.23 KB

Status: Needs review » Needs work

The last submitted patch, 13: feeds-background-queue-2630694-13.patch, failed testing.

The last submitted patch, 2: feeds-clear-queue-2630694-1.patch, failed testing.

The last submitted patch, 11: feeds-background-queue-2630694-11.patch, failed testing.

MegaChriz’s picture

The patch is hanging on the same error as the test posted in #2624344: Import via pushImport() keeps looping / never completes, which uncovered an endless loop. Now that I committed a fix for that issue, let's see if a retest helps...

MegaChriz’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 13: feeds-background-queue-2630694-13.patch, failed testing.

twistor’s picture

There is a different bug happening here.

I know how to fix it, but I realized that things could be better.

What we need are two schedule-type methods. Which I will call ensureSchedule() and schedule(). ensureSchedule() should be safe to call at any time, and won't modify an existing schedule, if it exists. schedule(), on the other hand, will forcefully schedule the feed.

cigotete’s picture

Hi, I am just curious about if this change still is in progress and will be implemented. Thanks in advance.

MegaChriz’s picture

Assigned: twistor » MegaChriz
Status: Needs work » Needs review
FileSize
4.24 KB
711 bytes

Reroll with one difference: with #2450365: Importer is not rescheduled for new sources when using attach to node introducing the ensureSchedule() method that was suggested in #20, that method is called instead when submitting the import form or the delete items form.

I think that also the background job should be queued right away instead of first importing the first chunk.

MegaChriz’s picture

Alright, this puts the job directly in the queue and *does not* import the first chunk.

Let's see if this has any side effects. I adjusted already some tests.

MegaChriz’s picture

I noticed some issues with scheduling an import and then run cron: the import button remained disabled and showed a negative progress value.

Also added a test for clearing a feed in background.

MegaChriz’s picture

I noticed that an import job got queued twice when scheduling it for running in the background.

This adds a method isQueued() to the FeedsSource class which checks if an import job is already queued. For this I committed #2868134: Show next time that the source will be imported first as that already contained code for querying the queue table. Now isQueued() only has to call getNextImportTimeDetails() to get the necessary information.
Also checked for the number of items in the queue table in the tests.

Status: Needs review » Needs work

The last submitted patch, 25: feeds-background-queue-2630694-25.patch, failed testing.

MegaChriz’s picture

Adjusted test FeedsSchedulerTestCase::testNextImportTimeWhenQueuedViaBackgroundJob() and moved startBackgroundJob() back to its original location in the FeedsSource class.

MegaChriz’s picture

Small changes. Now that we have the isQueued() method, the message "Run cron to continue the import." is only shown if that method returns TRUE.

If this is still passing tests, I'll commit it.

  • MegaChriz committed 63250e0 on 7.x-2.x
    Issue #2630694 by MegaChriz, twistor: Run background jobs directly in...
MegaChriz’s picture

Status: Needs review » Fixed

Alright! Committed #28.

Now onto the final piece of the "process in background" story: #2445477: Process in background not working with certain combination of settings.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.