ayrat555/fang

Simplifying api

prabirshrestha opened this issue · 4 comments

One thing I have found it confusing is that both producer and consumer uses AsyncRunnable which has methods such as cron and uniq. This makes it confusing on if the value is taken from producer or from consumer. Would be great if the apis were a bit more like faktory-rs.

You create a producer that has that enqueue jobs with all the metadata.

use faktory::{Producer, Job};
let mut p = Producer::connect(None).unwrap();
p.enqueue(Job::new("foobar", vec!["data1"])).unwrap();

and to consume you register the handler only.

use faktory::ConsumerBuilder;
use std::io;
let mut c = ConsumerBuilder::default();
c.register("foobar", |job| -> io::Result<()> {
    println!("{:?}", job);
    Ok(())
});
let mut c = c.connect(None).unwrap();
if let Err(e) = c.run(&["default"]) {
    println!("worker failed: {}", e);
}

One thing I like about faktory-rs is that it is few lines of code. To add other features such as schedule the following could be used.

p.enqueue(Job::new("foobar", args)
    .on_queue("queue1")
    .schedule_at(Utc::now());

Job that is enqueued should be a different struct than the job that was dequeued.

pxp9 commented

I do not think the current API is confusing or difficult.

The main objective of the current API is to make easy to define tasks.

#[typetag::serde(tag = "type")]
#[async_trait]
pub trait AsyncRunnable: Send + Sync {
    async fn run(&self, client: &mut dyn AsyncQueueable) -> Result<(), FangError>;

    fn task_type(&self) -> String {
        COMMON_TYPE.to_string()
    }

    fn uniq(&self) -> bool {
        false
    }

    fn cron(&self) -> Option<Scheduled> {
        None
    }
}

This is the default implementation that has AsyncRunnable. So a task by default will be with COMMON_TYPE in task_type field, tasks by default are not uniq and also are not scheduled.

The only method that is not optional is run, because 'Consumer' that is our Worker or AsyncWorker struct in some WorkerPool or AsyncWorkerPool struct needs to call run to execute the task, same for blocking module.

In order to enqueue a task, it is very easy if you create a 'Producer' that is some Queueable or AsyncQueueable struct , and call method insert_task.

image

Maybe you can check fang_examples.

@prabirshrestha thank you for your feedback

I think the API of the tasks can be improved. The only method used by a worker is run, all other methods are used by the "producer".

as @pxp9 pointed it's the simplest approach, but possibly not the best.

One of the options to improve it would be :

  • split runnable traits into enqueueable and runnable traits
  • use the approach that you suggested

my main inspiration for fang is sidekiq (ruby). Every option and perform method are in the same module (https://github.com/mperham/sidekiq/wiki/Advanced-Options#workers)

I think your suggestion is valuable for us to think about improving API. For now I think we should focus on better docs so it wouldn't be so confusing for users.

Splitting trait would definitely help bring clarity. This would also help avoid having Runnable and AsyncRunnable traits as adding message to the queue would be the same trait, it is the worker callback that is either async or sync.

You could still have the traits but also add additional builder apis. For example, I'm trying to build a simple task scheduler where one selects the cron schedule and executes a custom lua script. I never want my apps to be calling fang apis directly and always use a helper method to schedule it.

fn add_cron_task(name: &str, cron: &str, script: &string) {
  fang_connection.enqueue(EnqueueableBuilder::new(script)
    .cron(cron)
    .task_type("exec_lua")
    .uniq(false)
    .max_retries(3)
    .build());
}

add_cron_task("ping", "* * * * *", "print('ping')");
add_cron_task("check weather", "* * * * *", "print('weather')");

The other good part about this is I can package my library into smaller crates (adding message to the queue vs worker) as workers are usually a lot heavy.

I can also see folks wanting to collocate their queing and worker which can be done by providing a trait that does both with : Enqueueable + Runnable.

Currently we're using typetag to serialize a task when enqueuing it and to deserialize it when executing it. typetag serializes Runnable/AsyncRunnable trait objects.

We need to figure out another way of storing a task to be able to split traits. Because right now if we use Enqueueable to serialize a task, workers won't be able to execute it because tasks won't contain run method

@prabirshrestha if you have any suggestion, please share. or you can contribute to the project ;]