One of the things I wanted to add for 2.0 is a built-in CLI tool that would make it easy for people to run jobs without them having to build an actual CLI job runner. I've just pushed to the cli_tool branch my proposal for this.

What's your opinion @Revisor?

TO-DO

• [ ] Add command line option to run a single job (wait for one, run, then exit) for atomic runs
• [ ] Add tests
• [ ] Add a section in the CLI tool documentation mentioning supervisord and memmon

Add support for NOHANG to GETJOB.

In the latest version of Disque, GETJOB can be called with the NOHANG option, which allows it to return immediately if no jobs are available. This PR adds support for that workflow.

GETJOB returns at least the queue, the ID and the body:

Jobs are returned as a three elements array containing the queue name, the Job ID, and the job body itself.

What's the point of a job without a body?

Case in point: Disque\Queue\Marshal\JobMarshaler::marshal() expects a JobInterface type, but it actually requires a Job type and even throws an exception, because it needs the Job::getBody() method.

And subquestions:

• Should we agree that there is no point of a Job without an ID and a body, shouldn't they be provided as soon as in the constructor so that the object can't be instantiated in an invalid state?
• In case the ID and the body are provided in the constructor, wouldn't it be preferrable to remove their setters and make Job immutable? What would be the use case for changing the Job ID or the body on the fly, after it has been created?

When switching nodes, the prioritizer can recommend to stay on the current node. The current node however could have lost its connection in the meantime. In that case we will try and reconnect, or if not possible, connect to the next best possible node.

The check whether the current node has disconnected has been moved so that it is checked every time we execute any command, not just after the GETJOB command.

I tried to write a test for this situation, but ended up with a brittle monster of a test depending on the order of method calls among several mocks. :-/

I have no idea on how to test this. I've come to this problem where a one queue processor adds to another queue and thus a new disque client is made. to push a job to another queue. And somewhat the connection in original disque client is dropped.

In #12 you mentioned that moving the connection code out of the Manager was something you had wanted to do.

Is there something else you would like to change and take advantage of the upcoming 2.0? If so, can you create issues for it?

There is always something to improve, of course, but what are the high priorities in your opinion?

Since I'm now immersed in the library, I would be glad to help you.

This is admittedly a small issue that doesn't concern the functionality, but rather best practices.

Should Queue::pull() really throw an exception if there is no job to do? From all I've read, exceptions should be used sparingly and only for truly exceptional cases. If there is another clean way to relay the negative result, it should be favored over throwing an exception.

According to this exception typology, the exception thrown here would be an example of a vexing exception. It is not exceptional to not have a job ready.

What if the method just returned null?

What's your opinion about this in general? There might be more cases like this, but this is the place I'm working on right now and I don't want to nitpick needlessly.

This script runs fine:

$client = new \Disque\Client([
    new \Disque\Connection\Credentials('127.0.0.1', 7711),
]);

$job = $client->queue('something')->pull(1);

var_dump($job);

// hello called after pull
var_dump($client->hello());

But if i try to call hello first, it throws a ConnectionException every time.

$client = new \Disque\Client([
    new \Disque\Connection\Credentials('127.0.0.1', 7711),
]);

// hello called right after client created, throws exception here
var_dump($client->hello());

$job = $client->queue('something')->pull(1);

var_dump($job);

Any ideas? I'm trying to write a wrapper and I'd like to be able to test the connection when I create it. I thought using hello or info would be a great way to do that, but both seem to have this issue.

While working on #8 and refactoring the Disque\Connection\Manager, I realized that, no, we cannot just set a connection object instead of a class name, because we need to create a new connection for each node separately.

We have in my opinion two options:

1. Leave it as it is now - Manager gets a class name that conforms to the ConnectionInterface and creates its own connections as needed
2. Or the Manager gets a ConnectionBuilder (name may change) and every time it needs a new connection, it asks the builder for one.

The status quo has these advantages:

1. It's already there and it works.
2. It's simple - just set a class name and go.

Refactoring into a ConnectionBuilder has these advantages:

1. We can use type hinting instead of the "condition + exception" combo: Set a ConnectionBuilder instead of a class name, and further methods would require a ConnectionInterface
2. The Manager is already doing too much. ConnectionBuilder could make the Manager simpler by taking over all the steps needed for the actual connection:

• Manager::buildConnection() - the instantiation of a new connection object
• Manager::doConnect() - the actual connection
• Manager::getNodeConnection() - the initial HELLO

(There are multiple steps, that's why I suggest the name Builder instead of Factory)

The Manager's role would then be to manage node connections as a group, choose the best node, disconnect the inactive ones etc. That's a complicated enough task, even without having to handhold single connections.

This is more an internal refactoring, not much would change for the library user (unless they want to use a different connection object).

How does it sound to you?

If installed with Composer, disque-php GetJob command fails due to changed Job ID prefix. It is fixed in current version on Github, so Packagist version needs to be updated.

Hi! I've been looking into this library and there's quite a difference between the last published stable version on packagist (1.3) and the latest one here. Any feature missing so you can release 2.0 ? Are you waiting for disque to release 1.0 to release your version ?

The PING command is useful for checking for underlying timeouts, connection issues, dead server nodes, etc. PING appears to be implemented the same as Redis's PING (but I've asked upstream whether it's likely to be supported long-term.)

It works like this:

127.0.0.1:7711> PING
PONG
127.0.0.1:7711> PING something
"something"

This means that the PING command takes one optional string argument. It appears that neither ARGUMENTS_TYPE_STRING nor ARGUMENTS_TYPE_EMPTY quite covers this type of command.

Builds for HHVM are currently failing.

Presumably should be a major version to drop support, so maybe consider just no longer testing on HHVM for now and officially dropping support later.

Nobody is using it (happy to be surprised if that's not the case), and it's still not compatible with 7.0 features (let alone 7.1, and 7.2 is now in alpha)

The method Socket::getType() (https://github.com/mariano/disque-php/blob/39bc697/src/Connection/Socket.php#L192L207) is notated via PHPDoc as @return string A single char - that's not the case; it's actually ?string due to a small logic bug.

The problem is: if $this->socket is at EOF, the initial while condition is already false, and fgetc is never called. This makes the value of $type strictly equal to null, not false or the empty string.

From there, the check at the end of the method ($type === false || $type === '') will fail to match, and the correct exception is not thrown (should probably be "Nothing received while reading from client" - this usually happens during timeout).

This results in the user getting a ResponseException (when there was no response!) instead of a ConnectionException. The message is confusing too: "Don't know how to handle a response of type".

It's not well documented, but the HELLO response is not expected to always reply with an IP address for every node.

Specifically:

• Shut down any running Disque instance
• Clear any existing nodes.conf
• Start a single-node cluster of Disque (must never have had CLUSTER MEET called on it)
• Connect and call HELLO

In this case, the Disque server does not have an IP address yet (even if you use the bind option), and the HELLO response looks like this:

127.0.0.1:7711> hello
1) (integer) 1
2) "b27f65e00cb76bb4de71dab8148f3a09d93251cd"
3) 1) "b27f65e00cb76bb4de71dab8148f3a09d93251cd"
   2) ""
   3) "7711"
   4) "1"

In this situation, the client will still consider this a valid candidate node, and will load it into the connection managers ->nodes member during revealClusterFromHello()/revealNodeFromHello(). From there, it may be selected by the priorityStrategy and put into use (because even though it doesn't have a host, it does have a low priority), causing an error.

Instead, we should filter out HELLO responses that don't have a host set (we wouldn't be able to connect to them anyway)

I'd like to release 3.0 soon. On the list of to-do I see:

• [ ] Issue #25
• [ ] Issue #28
• [ ] Issue #43
• [ ] Review and add official documentation for Disqontrol (see issue #35)

I'd also like to make php7 the minimum requirement, and consequently move to full strict typing.

What are your feelings retarding this roadmap and particularly the new minimum php version to be supported?