krakenjs / levee

A circuit-breaker pattern implementation with fallback support.
171 stars 28 forks source link

Question about `circuit.run` #2

Closed pvenkatakrishnan closed 9 years ago

pvenkatakrishnan commented 9 years ago

I want to be able to run circuitBreaker with extra options for the context. For example:

I want to be able to do

circuit = levee.createBreaker(wreck.get, circuitOptions);
circuit.run(uri, {header: <someHeaderInfo>, timeout: timeOutValue}, function (err, req, payload) {
  .......
  .......
});

This does not seem possible today as run only takes the context and callback as arguments. Am i missing something ? I went down the path of understanding if I can stash uri and the extra options information into context itself, but i quickly realized the execute function that actually invokes the operation in the breaker forces arguments to just be the context and callback.

If this is indeed a bug, I would like to take a stab at fixing it.

totherik commented 9 years ago

It's not a bug. I would prefer not to make the API take n optional parameters, but instead a single content object, which is unwrapped as necessary for implementations. e.g.

var circuit = levee.createBreaker(function (context, cb) {
    wreck.get(context.uri, context.options, cb);
}, circuitOptions);

circuit.run({ uri: '', options: {header: <someHeaderInfo>, timeout: timeOutValue}, function (err, req, payload) {
    //...
});

Internally, this prevents slicing, mixing and matching, and rebuilding arguments and provides for function invocations with consistent argument types. If this is indeed a non-optimal API, we can discuss updating.

pvenkatakrishnan commented 9 years ago

I see. Let me dabble with it a little bit more to see if I can handle all use-cases in the servicecore.

pvenkatakrishnan commented 9 years ago

This should work. Thanks!