Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
There's no reason we need to schedule a job with job_scheduler for background tasks.
We can just use the Queue API directly.
Comment | File | Size | Author |
---|---|---|---|
#28 | interdiff-2630694-27-28.txt | 1.89 KB | MegaChriz |
#28 | feeds-background-queue-2630694-28.patch | 25.05 KB | MegaChriz |
#27 | interdiff-2630694-25-27.txt | 3.45 KB | MegaChriz |
#27 | feeds-background-queue-2630694-27.patch | 24.89 KB | MegaChriz |
#25 | interdiff-2630694-24-25.txt | 11.1 KB | MegaChriz |
Comments
Comment #2
twistor CreditAttribution: twistor as a volunteer commentedComment #3
twistor CreditAttribution: twistor as a volunteer commentedComment #4
twistor CreditAttribution: twistor as a volunteer commentedWrong patch.
Comment #5
twistor CreditAttribution: twistor as a volunteer commentedComment #6
twistor CreditAttribution: twistor as a volunteer commentedComment #7
twistor CreditAttribution: twistor as a volunteer commentedComment #8
twistor CreditAttribution: twistor as a volunteer commentedComment #9
twistor CreditAttribution: twistor as a volunteer commentedLeaving the feeds_source_clear definition in case any existing jobs are running.
Comment #11
twistor CreditAttribution: twistor as a volunteer commentedComment #13
twistor CreditAttribution: twistor as a volunteer commentedComment #17
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedThe 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...
Comment #18
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedComment #20
twistor CreditAttribution: twistor as a volunteer commentedThere 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.
Comment #21
cigotete CreditAttribution: cigotete commentedHi, I am just curious about if this change still is in progress and will be implemented. Thanks in advance.
Comment #22
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedReroll 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.
Comment #23
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedAlright, 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.
Comment #24
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedI 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.
Comment #25
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedI 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. NowisQueued()
only has to callgetNextImportTimeDetails()
to get the necessary information.Also checked for the number of items in the queue table in the tests.
Comment #27
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedAdjusted test
FeedsSchedulerTestCase::testNextImportTimeWhenQueuedViaBackgroundJob()
and movedstartBackgroundJob()
back to its original location in the FeedsSource class.Comment #28
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedSmall 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.
Comment #30
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedAlright! Committed #28.
Now onto the final piece of the "process in background" story: #2445477: Process in background not working with certain combination of settings.