In Cron:

        $this->queueFactory->get($queue_name)->createQueue();
        $queue_worker = $this->queueManager->createInstance($queue_name);

whee, what? That needs to be documented.

CommentFileSizeAuthor
#10 2359037-queue-worker-docs-10.patch1.91 KBjhodgdon
#6 interdiff.txt677 bytesjhodgdon
#6 2359037-queue-worker-docs-6.patch1.91 KBjhodgdon
#4 2359037-queue-worker-docs-4.patch1.93 KBjhodgdon
#3 2359037-queue-worker-docs.patch1.93 KBjhodgdon
q.patch663 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Active » Reviewed & tested by the community

Looks good.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Reviewed & tested by the community » Needs work

Can we clean up the writing here a bit?
the id => the ID

and sentence is a bit hard to parse by human readers...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Needs review
FileSize
2359037-queue-worker-docs.patch1.93 KB

How about this? Ended up rewriting most of the QueueWorker plugin docs... did not bother with an interdiff since both of the lines in the previous patch were changed in this new patch (plus more), so it is kind of pointless.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
FileSize
2359037-queue-worker-docs-4.patch1.93 KB

Amazingly, this patch still applies and still needs review after a year... uploading newly rolled patch to bring to attention of new test bot.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs review » Needs work

Looks great overall. One minor question:

+++ b/core/lib/Drupal/Core/Annotation/QueueWorker.php
@@ -10,18 +10,21 @@
+ * \Drupal\Core\Cron::processQueues() processes queues that use workers; they
+ * can also be processed outside of the cron process, or even in parallel.

Can we remove the "or" in that sentence? Parallel processing is only possible outside of the cron process, right?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Needs review
FileSize
2359037-queue-worker-docs-6.patch1.91 KB
interdiff.txt677 bytes

Good point. I think the parallel part is kind of irrelevant, so removed.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs review » Needs work

I just wanted to note that I looked around for a better example of a queue, preferably one that doesn't use workers, but I find the cron run to be the easiest to understand (conceptually).

Anyway:

  1. +++ b/core/lib/Drupal/Core/Annotation/QueueWorker.php
@@ -10,18 +10,21 @@
+ * Worker plugins are used by some queues for processing the individual items
+ * in the queue. In that case, the ID of the worker plugin needs to match
+ * the machine name of a queue, so that you can retrieve the queue back end
+ * by calling \Drupal\Core\Queue\QueueFactory::get($plugin_id).

    I missed it the previous time, but the 80 chars rule is not respected here.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Needs review

Um... dreditor doesn't agree with #7? I think it's under 80 characters, by a few...

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs review » Needs work

Under but not wrapped as close to, no?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Needs review
FileSize
2359037-queue-worker-docs-10.patch1.91 KB

Ah, right. Rewrapped that Worker plugins paragraph. Did not make an interdiff file. Only a rewrap.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. I think it's a vast improvement.

  • catch committed 9cb0e23 on 8.1.x 
    Issue #2359037 by jhodgdon, chx: QueueWorker plugin id is the queue name
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

  • catch committed 5b585c9 on 8.0.x 
    Issue #2359037 by jhodgdon, chx: QueueWorker plugin id is the queue name...

Status: Fixed » Closed (fixed)

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