Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Job info in handler #25

Closed
gtamas opened this issue Jan 11, 2014 · 8 comments
Closed

Job info in handler #25

gtamas opened this issue Jan 11, 2014 · 8 comments

Comments

@gtamas
Copy link

gtamas commented Jan 11, 2014

This is not really an issue, but more like a feature idea.

So the handler's work() method has access to the payload, but it doesn't know anything about the job's details. Using this info, the work method could make better decisions about how to handle the job.

Example:

The jobstat - among other things - includes the number of tries, buries and time-left. These could be used to implement some kinda functionality that buries / deletes jobs after X tries and runs each retry with decreasing frequency.

But this is only one use-case. I guess having access to the job data (and the beans client) would make the handler more powerful.

So something like this would be cool:

.work(jobid, payload, callback)

or

.work(stats-job, payload, callback)

In the first case, the handler should have access to the beanstalkd client (created by worker).

In second case, by "jobs-stat" I meant the most important info about the job and its history, not necessarily the whole stats-job() response.

What do you think? :)

@bryanlarsen
Copy link

In our fork (Exocortex/fivebeans) we add the stats to the payload as _stats, if payload is an object. There are a bunch of other hacks in there that we should PR. But this one and some timeout/failure detection we've added are pretty hacky -- probably trying too hard to avoid changing the API.

@ceejbot
Copy link
Owner

ceejbot commented Jan 16, 2014

I'm totally okay with changing the API where it's sensible, or even handing over ownership of the project if you're up for it! I don't have this in production at the moment, so I don't have the incentive to drive it forward the way somebody who does have it in production has.

@imjoshholloway
Copy link

+1 on adding a key to the payload (something like $job) maybe?

@cmawhorter
Copy link

+1 and making a change but payload is payload imo and I don't think this belongs there.

Howsabout changing this to be this:

switch(handler.work.length) {
  default: // do what is done now
  case 3: // use signature (id, payload, callback) or whatever
}

That's less than ideal though. If API changes are on the table I'd like to see the handler implementation reworked.

@cmawhorter
Copy link

FWIW-- I implemented my suggestion in a patch and it is working for me. All tests pass too.

var fivebeans = require('fivebeans');
var _fivebeansWorkerCallHandler = fivebeans.worker.prototype.callHandler;
fivebeans.worker.prototype.callHandler = function(handler, jobID, jobdata)
{
  switch (handler.work.length) {
    case 3:
      var patchedHandler = {
        work: function(payload, callback) {
          return handler.work(jobID, payload, callback);
        }
      };
      return _fivebeansWorkerCallHandler.call(this, patchedHandler, jobID, jobdata);
    default:
      return _fivebeansWorkerCallHandler.apply(this, arguments);
  }
};

@ceejbot
Copy link
Owner

ceejbot commented Jun 14, 2015

Cool, ty for this! Will advise when it's released.

ceejbot added a commit that referenced this issue Jun 14, 2015
#25 (comment)

Tweaked the jscsrc; this is not yet in use.
@ceejbot
Copy link
Owner

ceejbot commented Jun 14, 2015

Published on npm as v1.3.0, including version bumps for dependencies.

@ceejbot ceejbot closed this as completed Jun 14, 2015
@cmawhorter
Copy link

Wow, awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants