Refactor of Beanstalkd

[bakura10]

Ping @juriansluiman, ready for review.

Once it's ok I'll write the test (or if you want to do it so I can work sooner on SQS :D).

The only thing missing is the watch/ignore/use thing. I don't know where I should put it. In the queue itself ? Or in the SlmQueueBeanstalkd\Service\PheanstalkService ?

[bakura10]

If we have a PheanstalkService (I'll likely have a SqsService too) for things like getting the names of all queues... that kind of thing, do you think that the Tube should accept our PheanstalkService or a Pheanstalk_Pheanstalk object ?

[juriansluiman]

Re: features of a service: I would put it in the queue implementation. You have now a tube implements queue and you let the beanstalk queue implement the tube interface. That's (as in, separate overloading interface) what I'd do too.

Re: pheanstalk: at first sight I'd like to encapsule pheanstalk to make it more clean and with the 0.1.0 I didn't have to work with the pheanstalk in my controller directly. Now with the additional abstraction, I'd say the service accepts pheanstalk directly (that is, the "queue" in the current implementation).

[juriansluiman]

I am not sure what is happening. I know Travis did first PRs only for repos which were sponsored (that's why zf2 was early at checking PRs). I was off the radar for a while, so I am not sure if Travis has changed this. I have put my key in all three repos, is there anything else I have to do?

[bakura10]

Mmhh it's strange... In ZfrRest I have it for the oldest PRs while the last one does not have Travis status in PR... Maybe it has been temporarily deactivated ? But yeah, the only thing to do is activate Travis on the "Service Hooks" (and not forget to check "Active" : http://cl.ly/image/1t3T3H2h101T)

[juriansluiman]

I think I screwed up :angry: I have enabled it for SlmQueue and SlmQueueSqs but not SlmQueueBeanstalkd. Now I have...

[bakura10]

Ha, thanks. Anyway, does it look good to you ? Do you feel like writing tests ? :)

[Fgruntjes]

Hi there,

Looking good guys, like the extra layer of abstraction to be able to use some other queue. Two minor points though. If I am missing something please correct me however having a QueuePluginManager seems like a bit over-engineering to me. How often do you really want to use different queue implementations in an application?

Second but this is more of an enhancement. Extend the QueueInterface or add a separate interface. This separate interface could dictate the queue has more features, much like the storage adapters in Zend\Cache are implemented. That gives a queue the option to implement a method that returns the result from a job. Gearman for example gives you the option to run 10 jobs and wait for those jobs to return. This way you can easily implement a distributed and more scalable application while still use the response in your actions.

Now I think of it an implementation with extra interfaces for different options can easily be added in your current implementation.

[bakura10]

You're right, I'm not sure people will use different queue systems at the same time. To me, there are two advantages of QueuePluginManager:

• First, you can use the service manager. This means that if a given queue (for instance, newsletter), need special config, you can provide your own factory.
• Second, what I did in the beginning was simply a queues config that was parsed (and the queue were hence created) whenever the service was asked. The problem is that, for instance, for Amazon SQS, you need to retrieve the queue URL from the name. This means that if you have 3 queues (let's say, email, newsletter, picture), it would have done 3 GET requests to get the queue url of all these queues), even if you use only one.

But I agree this may be overkill (because most queuing systems have method to get ALL the queues in one call).

I'm not sure to have understand the second enhancement. Basically you're telling that instead of having Tube implements TubeInterface, you have Tube implements TubeInterface, QueueInterface ?

Btw, if you are using Gearman, feel free to provide an integration of it =).

Questions to both of you that arise this morning:

1. Should we serialize metadata too (https://github.com/juriansluiman/SlmQueue/blob/master/src/SlmQueue/Job/AbstractJob.php#L48) ? From what I've seen in both Beanstalkd and SQS, metadata (identifier, receipt handle...) are set once the job is pushed, or when a job is retrieved, therefore when the job is pushed, there are typically no metadata.
2. Currently, because the data is converted to JSON, if someone set an object as a content, it will fail (well, it will be serialized to json, so people will "expect" to have an object back when it will be deserialized while they will get an array.

My quesiton is: should we enforce that content is ALWAYS an array ? Or should we do this instead:

$data = array(
            'class'   => get_called_class(),
            'content' => serialize($this->getContent())
        );

        return json_encode($data);

[juriansluiman]

@Fraak: having the queue plugin manager has to be proven over time. The fact you have the service manager to instantiate queues is a big +1 for me. However, if it must be necessarily done via a plugin manager, I am not sure about that. We need to come up with a method to configure queues easily with module config files and somehow be able to load the defined queue via the SM.

For your second suggestion, the cache storage interfaces are about capabilities. While beanstalk adds capabilities to the existing queue interface, those capabilities are not really sharable to other queues. You could actually say you rely on the queue interface and if you need beanstalkd specific features, you type hint for the beanstalkd implementation (removing its interface completely). It's an abstraction not really required for this use case. However, if the capabilities of the queues are possible to abstract (like cache has), then I'd certainly follow your suggestion.

[Fgruntjes]

The QueuePluginManager is indeed useful if you have different queues of the same type but different tubes. Have not thought of this yet. And yes having the service manager instantiate queues is an absolute must. However you don't really need a plugin manager for that. Another option would be to implement an abstract service factory that could instantiate the different tubes. However I am no fan of that solution. So yes I agree stick with the QueuePluginManager.

It is indeed about the capabilities of the different queue implementations. However I could think of some possible capabilities that could be abstracted:

• Peeking at job(s)
• Queue multiple jobs at once
• Queue job(s) and wait for the jobs to finish
• Get info like average wait time for a job

As to your other questions:

1. I would not suggest serializing the metadata or the content by default. Even if there is metadata before the job is pushed this metadata is used by the specific queue implementation. The implementation of the QueueInterface would use that metadata to create its Queue specific job object.
2. Serializing or not serializing the content by default does not really make a difference to me. So therefor I would not serialize it or maybe use is_object().

[juriansluiman]

@bakura10 To me, JSON is a storage / transporting format. I would like to have no JSON parsing/encoding whatever in userland code at all. So in a controller/service:

$job = new Job\NewsletterSubscription($email);
$queue->push($job, array('priority' => PRIO_LOW));

Then continuing this thought, there is also no need for any JSON serialization in SlmQueue, but only in the specific implementation. Then you can make SlmQueue\Queue\AbstractQueue::createJob() look like:

protected function createJob($name, $content = null, array $metadata = null)

Then beanstalkd and SQS might serialize data with JSON, but you can use serialize() or igbinary if your message contents can be binary and must be as small as possible. I think this makes it far more clear what the abstract worker does and how specific implementation of message contents is persisted.

For beanstalkd, there is only the need to get the id back for a job (as metadata). If you want to release/kick/bury the job, the id must be set. Thus, the metadata must be set back into the job when you pop. This argues more for specific queue implementation to handle the serialization of (meta)data.

Also, you can then let the content of the message be any value. I should not enforce arrays, as an XML string or whatever must be possible as well.If you have a good serialization method (json/php/igbinary) then you can serialize booleans/integers/strings/arrays/objects back and forth. Just let the userland code set a message contents that is serializable (so no Doctrine proxy classes for instance).

[bakura10]

I like this idea for createJob jurians. Wait, I'm gonna give it a try.

Btw, I've written the tests for SlmQueueBeanstalkd. Anything else that would need coverage?

[bakura10]

Jurians, is this what you were thinking about ?

https://github.com/juriansluiman/SlmQueue/blob/master/src/SlmQueue/Queue/AbstractQueue.php#L57-L64 https://github.com/bakura10/SlmQueueBeanstalkd/blob/c52a55df351487d49aadbf2624b8a65d6e91d89a/src/SlmQueueBeanstalkd/Queue/Tube.php#L70

[bakura10]

Ok, all the tests pass. I will begin to work on Sqs.

On the mean time, @Fraak could you test a little bit Beanstalkd to check if it works as expected ? Then we'll be able to tag :).

[juriansluiman]

Looks better @bakura10 with the new signature

[bakura10]

Code convention help: in ZF 2 for config options we use underscore_separated. However for Job metadata, should it be receiptHandle or receipt_handle ?