[D7] Queue API

Fixed the issues found by the bot.
The last 3 are false-positives.

Hi,

Thanks for the review! :]

Answer of 1:
Are you referring to the following lines?

    $max = &$context['sandbox']['max'];
    $progress = &$context['sandbox']['progress'];
    $errors = &$context['results'][self::RESULT_KEY]['errors'];

If so, the idea here is that I am assigning a reference to some sub-element of the passed context parameter, to simplify the access to the value and improve readability in the following code.

If there is no & in that code, it will assign the value with the PHP's copy on write and on the first change of the value, PHP will make a copy of the value and change the copy. This will result in no change in the $context parameter.

If you are not referring to this piece of code, please explain in detail.
I am marking this as "works as designed" and no need to change anything.

Answer 2:
Deleted the submit binding as proposed.

Answer 3:
Changed the usage to Drupal's JSON utility method.

Answer 4:
Well it just looks like :).
The class actually ends there and the last method in the file is there jut as it was intended to be part of the class, but the core Drupal APIs does not allow it to be, so a normal static function is implemented there to keep things together as they are meant to be by design.

Answer 5:
Added a new line.

BR,
Nikolay Dobromirov.

First, it allows easy processing of queues in object oriented manner.

I see it as an improvement over the queue api in general, as for some complex queue processing, you will have to define a router method and then depending on it, some processing methods. For some of them define some helpers and everything else, resulting in many functions. With this coding approach everything is compacted in a single class with a single purpose in a separate file. This is lowering the clutter in the *.module file, as most queue processing operations are implemented that way, with batch it is optional to use other file for this, but i've seen cases where this was not used. This approach is forcing the developer to move the code outside.

The above can be stated also for some complex batch operation as it will spare you the batch definition and processing method and allow you to focus on the real work, not the pluming.

This module was designed more as a queue processor and the batch processing comes as a bonus :).
When the code for the queue is implemented, and a client comes and say: "Yeah but I want to process this data with a batch as I do not want to wait for the cron executions", you have the code already written and nothing more is needed, except a form submit handler to start the processing.

This is a simple abstraction over both APIs to allow code re-usage.

Hi there,

Queue UI is just providing the interface to manage queues. Even if stated in the module description that the queue UI processes queus on batch, there is no code in that module to provide this out of the box. Someone will have to write the code for processing through the normal batch/queue APIs or through the utilities provided by this module.

The two modules are not competing, they are solving different problems.

This module aims to unify the approach for such implementations.

BR,
Nikolay Dobromirov.

Hi

I had a look at the sandbox, and even...

I have missed the patch above when I was looking into the queue_ui.
As it is not in the dev version yet this means that this functionality was not present in queue_ui and is still not in even when stated as available :).

I don't yet understand what problems this module is trying to solve.

It is allowing you to have less code in the .module file, by moving it in the class and just proxy the processing there.
Also it is abstracting the routing functionality for processing multiple and different queue items in a single queue. This will spare you the need to write one ugly if / else / switch function that will manage the queue's routing (@see QueueApiController::dispatch($item)).

You are right that this might not add much over the queue_ui + the patch,
but still it was a nice attempt for me to see how OOP friendly the batch and queue apis in Drupal 7 are - not much :].
Also this is just a developer's tool, no additional UI is added to Drupal's interface, what if I want the queue_ui without the ui :).

BR,
Nikolay Dobromirov.

Hi,

I don't actually understand that. How do you put queue items into another queue, and why? I've used queue API a lot, and I've never had to do this routing switch you mention! Can you explain the use case?

The idea is to split one complex processing task in many small ones.
This will allow you to fit in Batch API time limitations per queue-item processing and still have a real queue to work with.
The result is that within the same queue, you will get many different items, that require different processing implementation.
So, your actual queue processor callback will just do routing/proxy to the correct implementation for the given queue-item.
This is what I was referring to as routing in the comment above.
Let me give you an example:
Imagine some third party integration and your cron is responsible for synchronization from the remote system to the local one.

Flow is:

1. get a list of remote items and iterate over it
2. 1. Request each item to extract specific data (url) not available in the list.
   2. Download a file pointed by the URL
   3. do a heavy operation 1 on the downloaded file
   4. do a heavy operation 2 on the result of operation 1
   5. ... this can go on...

Standard option 1:
You can implement this easy on cron with one method with a loop in it and be happy with it, but you can hit limitations of any sort :) with naive implementation like this, depending on processing and service.

Standard option 2:
Or just create a cron that will fill in a queue and the queue processor will do all the operations for a single queue-item in a single method. This is better than the first but this mega method might be heavy and still hit queue processing limits. And there are some issues in core, need to find the correct reference, but as a summary core queue processing does not cope well with slow processing of a single queue item. This can result in locked queues resulting in even slower processing etc.

How this module solves it:
So the idea is to put everything in a queue for delayed processing and every operation will be a queue-item in the queue. When an operation finishes it is generating the next queue item and this continues until you reach the last queue-item that does not generate new items in the queue after being processed, effectively completes the processing.

My assumption is that any single operation can not be more time-consuming than 5-10 seconds. And this allows seamlessly to process such queues with the Batch API without the risk of hitting time limits.

I hope things are a bit more clear now. :)

The problem I have with your module is that this feature is all mixed in with your more advanced ideas -- or maybe it's not mixed in, but it needs more documentation as I can't unpick them.

As explain above this is just delayed processing through a queue. The result with all this is that it was fairly easy to add Bath API support on-top of all that was already there. Maybe a better README will help, but I am a bit short on time at the moment.

BR,
Nikolay Dobromirov.

Hi,

Thanks for the feedback, but I have some comments about it :)

However, I think your solution for this is overkill. Here's how I would do it: I would implement hook_cron_queue_info() and declare the following queues...

Yes but for complex operations this can go ugly and hard for debugging and development. If you have 10+ operations, resulting in 10+ queues will not be that intuitive. The thing that you are describing was my initial idea but when I had to implement it 2-3 times in a single project... it became insane for maintenance.

One other reason for implementing this was to streamline processing of such queues, lower the clutter in the module file and ease development.

With your approach, it will result in more worker callbacks that will be placed in the module file, increasing the code in it.
If changes are needed (new operations) you will have to tweak on 3-4 places at least (http://en.wikipedia.org/wiki/Shotgun_surgery).

With this approach, is easier for development as adding new operations is happening only in the class. Once everything is set up (meaning queue definition, hook cron and queue worker proxy, submit handlers for batch processing), all the development is within the controller. Adding actions, processing, changes etc.

Finally, it has the advantage of being asynchronous OOTB

Just by being processed in the queue it is already asynchronous. By using atomic operations that share nothing (the idea behind an action by definition - get input, process, create input for next action, send a message in the queue), there is no problem to run several processing workers on a single queue to achieve parallel processing or just increase the time limit of the queue, to be processed longer by Drupal.

... That may be what your code's doing... but so far I've found it too opaque to figure out, sorry :(

It is processing it just as a standard queue. While there are items in the queue, process one at a time. When a next operation is needed it is just added in the end of the queue and processed accordingly, when it's time comes.

Most of the logic is within 3 methods in the QueueApiController:
dispatch: gets an item in the queue and based on some metadata stored in the item, invokes the action to process it.
addAction: get the next action name and data array to pass on to it as input parameters. Based on the input, it prepares a queue-item with the needed metadata and adds it to the queue, where dispatch will take on again some time later.

failed: This method is used to say - this action failed, performs some logging and error messages preparation (if needed). It also manages the retry policy of the controller. If an action fails for some reason, it might not fail next time (until a defined limit is reached). Default behavior is to not use retries.

Everything is in the base controller. User of the API will have to just define his queue, pointing to his controller and implement the processing actions. There is no need to think about routing and when things are processed, as everything is handled by the base controller.
If some custom queue initialization is needed, you cat override the init, isProcessing or any relevant for the developer method, as you are extending the base controller. In most of the cases this is not needed.

BR,
Nikolay Dobromirov.

Just to give some execution example to make processing internals more clear:

For the example:
Operation LIST, that returns a list of items for processing.
Operation A that processes a list item.
Operation B that takes data from A and finished the processing.
You have 3 items in the source data.

After init the init the queue will be:
LIST

When it is executed, it is expected to start the pressing of each of the items, so the queue after it will be:
A1, A2, A3

After A1 is processed the queue will be:
A2, A3, B1.

After A2:
A3, B1, B2

After all A operations are processed it will be:
B1, B2, B3.

Next iteration:
B2, B3.

Next iteration:
B3.

Next iteration:
Empty queue.

Iteration here is the processing of a single queue item, so depending on queue configurations, the number of iterations may differ.

The last 3 issues from the pear review script are are noted in comment #4 as false positives.

The first two for the hook... the text is different, but the parser says otherwise, nothing to do about this.
The last issue is coming from the fact that the module file is empty, when fixed, others pop up. The only way to solve this is to add some code to the module file, but by design the idea was to have an empty module file.

BR,
Nikolay Dobromirov.