Comments

MegaChriz created an issue. See original summary.

megachriz’s picture

Status: Active » Needs review
StatusFileSize
new2.14 KB
megachriz’s picture

Noticed some issues:

  • The patch showed only the correct time for importers with machine names consisting of four characters (I happen to have tested before with only the default Feeds importers: 'node', 'user' and 'feed').
  • Besides FeedsSource::scheduleImport(), JobScheduler::dispatch() can queue a feeds import job as well, which will result that the job data in the queue is different.
  • job_scheduler_cron() only dispatches jobs where 'schedule' = 0.

Here is a new patch with tests. Also extended the information about the next import time (whether Job scheduler or the Drupal queue handles it). Discovered that when the importer ID is in 'feeds_reschedule', then the next import time from job scheduler isn't accurate anymore, so I figured it should read 'to be rescheduled' then. If there is a job that has a value for 'schedule', but there is no queue item, I suspect that the job is stuck, so also reflected that.

It probably could use some more automated tests.

megachriz’s picture

Created tests for the following cases:

  • Queued via background job.
  • Queued via Job scheduler.

And also catched a bug in the previous patch. An import task queued via Job Scheduler wasn't correctly picked up.

megachriz’s picture

I do not have PostgreSQL installed locally, but I read on https://www.postgresql.org/docs/9.3/static/functions-matching.html that PostgreSQL uses the operator "SIMILAR TO" for regular expressions. Not sure if that works in the exact same way as "REGEXP" in MySQL.

Anyway, catching a PDOException in getNextImportTimeDetails() to at least guarantee a working import page for non-standard database types.

Maybe the query for the queue table could be converted to use "LIKE". We'll see.

megachriz’s picture

StatusFileSize
new8.25 KB
new1.34 KB
+++ b/includes/FeedsSource.inc
@@ -666,6 +666,93 @@ class FeedsSource extends FeedsConfigurable {
+        ->condition('data', 's:4:"type";s:[0-9]*:"' . db_like($this->id) . '";s:2:"id";(i|s:[0-9]*):"?' . db_like($this->feed_nid) . '"?', $regex_operator)

I figured since we know what the length of the job type and job ID is, we don't need regexes at all. We could just calculate the lengths and use a 'LIKE' query. This way the query should work with PostgreSQL.

New patch. Interdiff against patch in #4 instead (is more useful because everything added by the patch in #5 is disregarded).

megachriz’s picture

StatusFileSize
new8.22 KB
new1002 bytes
+++ b/includes/FeedsSource.inc
@@ -666,6 +666,92 @@ class FeedsSource extends FeedsConfigurable {
+    $serialized_job_type = format_string('s:4:"type";s:!length:"!type";', array(
+      '!length' => strlen($this->id),
+      '!type' => db_like($this->id),
+    ));
+    $serialized_job_id_as_string = format_string('s:2:"id";s:!length:"!id";', array(
+      '!length' => strlen($this->feed_nid),
+      '!id' => db_like($this->feed_nid),
+    ));
+    $serialized_job_id_as_integer = format_string('s:2:"id";i:!id;', array(
+      '!id' => db_like($this->feed_nid),
+    ));

Since the output of these isn't for HTML display, we can just use strtr() instead of format_string().

megachriz’s picture

Okay, for PostgreSQL it seems to be impossible to construct the right query to perform a search in the data column of the queue table. This is because the Drupal database layer adds '::text' to bytea columns, which results into the data column becoming unreadable in conditions. See DatabaseConnection_pgsql::prepareQuery().

So, instead we need a workaround for PostgreSQL. And that is: select the first 10 items in the queue and hope that the job we looking for is in there. Else report that the job may be still in the queue, but we are not sure.

And that's it for PostgreSQL.

vakulrai’s picture

StatusFileSize
new10.65 KB

I have successfully applied the patch provided by #8 and get (Next import: Mon, 05/01/2017 - 15:19 (via Job scheduler)) the next import time after running cron.

please find the attachment.

megachriz’s picture

Thanks for trying out the patch, vakulrai. The image shows an expected result.

While working simultaneously on #2630694: Run background jobs directly in queue. and #2445477: Process in background not working with certain combination of settings, I figured that a condition on the 'expired' column should also be set when searching in the queue table. When expired is 0, the task is not picked up yet. When it has a value, the task is picked up and hopefully still in progress. If not, then the import page saying "possibly stuck" is correct.

If this still passes tests, I will commit it, so I can continue with the other two issues.

megachriz’s picture

Corrected column names in query. "expired" should have been "expire".

  • MegaChriz committed f36dbcb on 7.x-2.x
    Issue #2868134 by MegaChriz: Show next time that the source will be...
megachriz’s picture

Status: Needs review » Fixed

Committed #11.

Status: Fixed » Closed (fixed)

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