Problem/Motivation
We have a Queue topic in Drupal 8:
https://api.drupal.org/api/drupal/core%21core.api.php/group/queue/8.4.x
(if this link is broken, go to Topics in the sidebar and find the Queue topic).
It currently says nothing about how to get a queue to be processed automatically in Cron.
Also, the plugin annotation class and hook related to this (QueueWorker annotation class and hook_queue_info_alter() ) are not listed as @ingroup for this topic, so they do not show up in the Functions and Classes lists on the topic page.
Proposed resolution
a) Add a section to the Queue topic about how to make a queue processed automatically in Cron. The main points:
- A queue can be set up to be processed in Cron by defining a QueueWorker plugin.
- This plugin needs to have \Drupal\Core\Annotation\QueueWorker annotation on it
- See the docs for that annotation class for details.
b) Make sure that the QueueWorker annotation class, Drupal\Core\Queue\QueueWorkerInterface, Drupal\Core\Queue\QueueWorkerBase and hook_queue_info_alter() both have lines in their doc blocks saying:
@ingroup queue
c) Make the Queue topic and hook_cron() have @see references to each other:
@see queue [in the hook]
@see hook_cron() [in the Queue topic]
Remaining tasks
Make a patch.
User interface changes
None.
API changes
None.
Beta eval
This is just API docs, so is unfrozen in Beta.
Comment | File | Size | Author |
---|---|---|---|
#38 | interdiff_2264943_37-38.txt | 2.15 KB | _pratik_ |
#38 | 2264943-38.patch | 3.18 KB | _pratik_ |
| |||
#37 | 2264943-37.patch | 2.94 KB | Nikhil_110 |
#36 | 2264943-nr-bot.txt | 153 bytes | needs-review-queue-bot |
#25 | interdiff-2264943-17-25.txt | 3.15 KB | FeyP |
Comments
Comment #1
jhodgdonIt's not just linking... It seems like the Queue Operations topic needs to mention that before you can create a queue, you also need to declare it using hook_queue_info()? I guess, anyway?
Hm... hook_queue_info() says:
And then later on, under Return value, it says:
So I think in the Queue Operations topic, we should say something about when you do and don't need to use hook_queue_info(), and what the consequences are. And definitely hook_queue_info(), not to mention the basic Queue classes and interfaces, should have @ingroup queue added to their doc blocks.
Comment #2
joachim CreditAttribution: joachim commented> before you can create a queue, you also need to declare it using hook_queue_info
No, that's not the case. It's only if you want cron to run your queue for you.
Comment #3
gauravkhambhala CreditAttribution: gauravkhambhala commentedComment #4
gauravkhambhala CreditAttribution: gauravkhambhala commentedThe first link says "Sorry, api/drupal/core!includes!common.inc/group/queue/8 cannot be found". Could you pont out right link?
Comment #5
joachim CreditAttribution: joachim commentedIt's been moved: https://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...
Comment #6
gauravkhambhala CreditAttribution: gauravkhambhala commentedAdded @ingroup queue to the functions. Please review the patch.
Also added @see links to hook_cron function. Let me know if that was required, otherwise I will remove that change and upload the patch again.
Comment #7
jhodgdonI do not think a @see in the hook_cron() documentation to callback_queue_worker() is really a good idea -- that seems like something only used by hook_queue_info() right?
By the way, you can also do
@see queue
and it will make a link to the Queue topic.
So... This patch is mostly good, but... I think we should do a bit more:
- Probably the Queue topic needs an @see to hook_cron()
- Probably hook_cron() needs an @see to "queue"... and probably also to hook_queue_info(), but not hook_queue_info_alter() or the callback function?
- Probably hook_queue_info() should have an @see to hook_cron()... oh, it already does. That's good. :)
Comment #8
joachim CreditAttribution: joachim commentedI don't think you need the @see to hook_queue_info() at the top of the patch, as it's in the doc text directly above it. As for the @see hook_queue_info_alter(), that's not needed either, as you can discover that exists when you go read about hook_queue_info().
Comment #9
er.pushpinderrana CreditAttribution: er.pushpinderrana commentedMoving forward to this, please review updated patch.
Comment #10
jhodgdonOK...
I still think we need this from #7:
- Probably the Queue topic needs an @see to hook_cron()
Actually, the Queue topic doesn't even talk about hook_queue_info in the text. Should it?
Comment #11
joachim CreditAttribution: joachim commentedI don't think the Queue topic should @see to hook_cron(). It should mention hook_queue_info() in the text though. At that point I suppose it could mention hook_cron(), but it's not really relevant, as it's not hook_cron() that works on queues declared by hook_queue_info() -- it's the base cron handling in system module.
Comment #12
jhodgdonWell I don't think it says anything at all about cron now. Shouldn't it?
Comment #13
joachim CreditAttribution: joachim commentedThe gist to convey is "if you want a queue to be worked on automatically by Drupal during cron runs, you need to declare it with hook_queue_info()". So yes, probably.
Comment #14
jmarkel CreditAttribution: jmarkel as a volunteer commentedUpdated tags - needs re-roll and an updated issue summary.
I am removing the Novice tag from this issue because it appears to be a significant effort for anyone with no familiarity with the Queue system. It could be done by a novice but may take more time to read up on and digest the Drupal queueing system..
I’m using this documentation as a source: https://www.drupal.org/core-mentoring/novice-tasks#avoid
Comment #15
jmarkel CreditAttribution: jmarkel as a volunteer commentedComment #16
Weilinggu CreditAttribution: Weilinggu commentedI am at Drupalconla. I am working on this issue right now.
Comment #17
Weilinggu CreditAttribution: Weilinggu commentedhook_cron and hook_queue_info_alter are moved to core.api.php. Link has been added. Uploaded the patch here.
Can't find callback_queue_worker and hook_queue_info using grep in D8.
Comment #18
Weilinggu CreditAttribution: Weilinggu commentedComment #19
jhodgdonThanks for the new patch! You're right, things have changed in Drupal 8 since this issue was first filed.
So. It appears that in Drupal 8, instead of having hook_queue_info(), we now have a QueueWorker plugin annotation. So the way you say "This queue needs to run in Cron" is you define a class and give it QueueWorker annotation.
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Annotation!QueueW...
The Queue topic still doesn't say anything about this, and it should:
https://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...
Updating the issue summary. We just need a bit more added to the patch, which is good as far as it goes.
Comment #20
joachim CreditAttribution: joachim at Torchbox commented> So the way you say "This queue needs to run in Cron" is you define a class and give it QueueWorker annotation.
You probably also want to have it inherit from Drupal\Core\Queue\QueueWorkerBase, and if not, implement \Drupal\Core\Queue\QueueWorkerInterface.
Comment #21
jhodgdonDefinitely, and the base class and interface should also have an @ingroup so they appear in this topic.
Comment #25
FeyP CreditAttribution: FeyP as a volunteer and at werk21 commentedI made some changes to the issue summary:
QueueWorkerInterface
andQueueWorkerBase
per comment 21.The previous patch basically implemented some parts of the changes proposed in b) and c). Attached is a new patch against 8.4.x. Changes from previous patch:
core.api.php
moved to/core/core.api.php
core.api.php
@ingroup queue
toDrupal\Core\QueueWorker\QueueWorkerInterface
andDrupal\Component\Annotation\Plugin
to complete b).@see hook_cron()
to Queue API topic to complete c).Comment #29
apadernoComment #36
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #37
Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commentedAttached patch against drupal 10.1.x
Comment #38
_pratik_ CreditAttribution: _pratik_ as a volunteer and at Specbee for Drupal India Association commentedComment #39
joachim CreditAttribution: joachim as a volunteer commentedLGTM. Thanks!
Comment #40
larowlanUpdating credits
Comment #41
longwaveCommitted and pushed 4b781a54fa to 10.1.x and 98c56f0a20 to 10.0.x and d92acc38f2 to 9.5.x. Thanks!
Comment #45
luenemann