Skip to content

Instantly share code, notes, and snippets.

@nvie
Last active April 27, 2018 12:53
Show Gist options
  • Save nvie/4707624 to your computer and use it in GitHub Desktop.
Save nvie/4707624 to your computer and use it in GitHub Desktop.
Which API is the nicest for the new RQ with concurrency?
# Alt 1: very explicit, but requires knowledge on what type of workers exist
from rq.workers.gevent import GeventWorker
w = GeventWorker(4)
# Alt 2: A bit like an "Abstract Factory", no need for knowing the implementing
# class names, backward compatible, and kind='forking' by default
from rq import Worker
w = Worker(4, kind='gevent')
# Alt 3: I don't think (2) works without really nasty tricks. This one is as
# simple as (2), but uses a convenient class method, so we can return an instance
# of the implementing subclass instead. Switching worker impls is as easy as
# changing a single string.
from rq import Worker
w = Worker.create('gevent', 4)
# Alt 4: A helper function that wraps (1)
from rq import make_worker
w = make_worker('gevent', 4) # will return a rq.workers.gevent.GeventWorker
# with max 4 processes
@samueljohn
Copy link

Both, Alt 1 or Alt 3 look pythonic to me, and don't rule out each other, IMO.

For alt 1, I would suggest a shorter import statement:

from rq.workers import GeventWorker
w = GeventWorker(4)

for alt 3:

from rq import Worker
w = Worker(4, backend='gevent')
# or
w2 = Worker(1) # takes what is available

@deferraz
Copy link

deferraz commented Feb 4, 2013

3

@ghickman
Copy link

ghickman commented Feb 4, 2013

1 feels the most pythonic.

@nvie
Copy link
Author

nvie commented Feb 4, 2013

Well, basically, (3) is a bit of sugar on top of (1).

(1) will definitely be there, and be possible. (3) only makes it shorter and easier to remember. Under the hood, (3) will do (1).

I'm dismissing (2), I agree it's confusing.

@samueljohn
Copy link

oh, after sending my comment the naming changed ...

for Worker(4, kind='gevent') one could write a __new__ that can return another type than the class itself.

@saghul
Copy link

saghul commented Feb 4, 2013

1

@AndreaCrotti
Copy link

I'm not sure what the 4 is for just from reading any of the alternatives..
Why not just calling it generically
ConcurrentWorker and keep the kind option, so that at least looking at the instantiation line I know exactly what that thing is, and what the 4 might stand for?
So I guess I'm for alternative 5 which does not exist :D

@nvie
Copy link
Author

nvie commented Feb 4, 2013

@AndreaCrotti 4 is the number of concurrent processes. Every worker will be a ConcurrentWorker once 0.4 gets released, there won't be non-concurrent workers anymore (unless you give it num_processes=1, which will be the default).

@nvie
Copy link
Author

nvie commented Feb 4, 2013

@samueljohn I think that approach is confusing in Python. Instantiating a class like that should return an instance of that class, not some other class—that's a bit of black magic. I'm not going for option 2 for this reason.

@nvie
Copy link
Author

nvie commented Feb 4, 2013

I've added another alternative. What about (4)?

@AndreaCrotti
Copy link

@nvie I knew that 4 is the number of concurrent processes, but if I just look at the code it's not really clear.
Maybe you could force passing a keyword argument for num_processes to make it always readable.

Otherwise I would keep separate the standard Worker from something like ConcurrentWorker (which then might default to num_processes=2 for example), unless it really make sense that the default implementation should be concurrent.

Otherwise I would vote for option #2..
And anyway it's probably better if you leave the old alternatives instead of replacing them or old answers will not make much sense..

@saulshanabrook
Copy link

1 is the most explicit. You call a factory on a class and it returns an instance of that class. Whats the use case for needing to be able to type of worker on the fly, instead of importing?

@nvie
Copy link
Author

nvie commented Feb 5, 2013

@saulshanabrook: The main use case is that implementing custom worker scripts would be easier. The default rqworker script would look something like this:

import sys
from rq import make_worker

...

if __name__ == '__main__':
    args = parse_args()
    worker = make_worker(args.concurrency_backend, args.num_processes)
    worker.work()

Where the concurrency_backend and num_processes arguments could be command line flags, like for example:

$ rqworker -c gevent -n 4

Implementing custom worker scripts would be just as easy and can use the default rqworker script as a reference implementation.

@kramer65
Copy link

kramer65 commented Oct 8, 2016

I'm really interested in concurrency. Did any of this get implemented?

@MrJohnsson77
Copy link

I'm all curious what the final decision was on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment