Getjob options [NOHANG, WITHCOUNTERS]

[macobo]

Currently the getjob command doesn't those two options supported by upstream. I think these can be added much the same way addjob's options are hacked on, but not sure what the best option for doing that is.

I can work on a PR if we figure out a good way to do this. :)

[mp911de]

Thanks for the issue. +1 for GetJobArgs with a builder and adding a getjob/getjobs which accepts GetJobArgs, Queues. getjob and getjobs with the timeout options can be @Deprecated and then the API feels more consistent again.

PR's are always welcome.

[macobo]

I'll take a look - no promises but hope to have a PR out in the next couple of days. :+1:

[macobo]

Hrm. Started implementing the solution but ran into a blocker - namely I'm not sure what a good API for this will be. I'll jot down my ideas below, I'd be curious to hear your feedback.

For reference, the command syntax is GETJOB [NOHANG] [TIMEOUT <ms-timeout>] [COUNT <count>] [WITHCOUNTERS] FROM queue1 queue2 ... queueN

And the current getjob api (DisqueJobAsyncCommands) is:

RedisFuture<Job<K, V>> getjob(K queue);
RedisFuture<Job<K, V>> getjob(long timeout, TimeUnit timeUnit, K queue);

RedisFuture<List<Job<K, V>>> getjobs(K... queues);
RedisFuture<List<Job<K, V>>> getjobs(long timeout, TimeUnit timeUnit, long count, K... queues);

---

So an obvious improvement would be to add to those two options:

RedisFuture<Job<K, V>> getjob(GetJobArgs getJobArgs, K queue);

RedisFuture<List<Job<K, V>>> getjobs(long count, GetJobArgs getJobArgs, K... queues);

However, an annoying bit with the first is that the returned job can be NULL - would that be an acceptable trade-off given it's documented?

---

As for the WITHCOUNTERS flag, I think an extra field in the Job object might be enough which will be NULL if the flag was not set in GetJobArgs.

The problem with this is that it's sure to bite someone that it's not set and java doesn't have Option to make it clear it's nullable - is there any other way we can signal that this is only set if the WITHCOUNTERS flag is passed.

[mp911de]

Thanks for your input. The issue we're facing right now is related to strong typing. The Disque (and previously Redis) API returns data/data structures that depends on the command args (refer to http://redis.io/commands/georadius). Drawing all permutations of the command args as API causes a lot of methods. The lettuce client does this for certain commands (RedisSortedSetsConnection) which highly depend on the result type.

Viewing the API from a different perspective: When using the API, you are either happy with the simple approach getjob(K queue)/getjobs(long count, K... queues) or you head for the advanced call getjobs(GetJobsArgs getJobsArgs, K... queues). So maybe just adding a getjobs(GetJobsArgs getJobsArgs, K... queues) method for the initial support. I'm not decided and not fully sure about a particular solution.

About the WITHCOUNTERS options: I'm not sure. Having the counters in the Job data structure is one option (similar to GeoWithin). A JobWithCounters is another option (similar to ScoredValue it would be getjobsWithCounters). WITHCOUNTERS implies some potential to grow and return more fields than it has now. Maybe a Map<String, Number> within Job that is empty when the counters are not requested.

Does this makes sense?

Keep in mind, spinach is Java 1.6 compatible and should not use (yet) Java 1.8 on the API. I'm also still fighting with myself to get rid of Google Guava or not since it causes sometimes more trouble than value.

[mp911de]

Implemented with #15