topliceanu / schedule-drone

A reliable, fault-tolerant, persistent event scheduler for node.js
5 stars 4 forks source link

scheduleAndStore Saves the data but doesn't schedule CRON #6

Closed imomin closed 9 years ago

imomin commented 9 years ago

I am playing with your cron module. I've noticed the scheduleAndStore saves the data in the database but it doesn't fire event. I don't know coffeescript but I think may be adding schedule(dataOrCronString, eventName, eventPayload) method on line 68 should resolve the issue. eg. store.save data, (err, storedEvent) => if err callback err schedule '* * * * * 1', 'my-cron-event', params

Following is the javascript equivalent.

return store.save(data, function(err, storedEvent) { if(err) return callback(err); PersistentScheduler.super.schedule(timestamp, event, payload); return callback(null, storedEvent); });

topliceanu commented 9 years ago

Hi @imomin,

I'm not exactly sure what you need to do: you need to run a function immediately after a job is scheduled successfully (1)? OR you need to schedule a job to run after one millisecond * * * * * 1 (2).

If (1), note that you can pass a callback to schedule which will run just after the the job is persisted on disk. Here's the API from the readme PersistenceScheduler.schedule(when:String|Date, event:String, data:Object, callback:Function)

If (2), schedule-drone executes jobs by polling the database every minute, so your job might execute with a maximum delay of ~1 minute. This is for performance and practical reasons: if you need to schedule a job after 1 ms, just use setTimeout(fn, 1), if you need to schedule a job in six months, use schedule-drone.

Also, I'm working on v1.0.0 of this module, should be out soon. Check out the issue, it's open for comments.

As per coffeescript, why don't you try it out, it's quite fun, even thou, I guess, by this point, you're better off investing time in ES6 rather then coffeescript.

imomin commented 9 years ago

If you run below example, you will see the scheduleAndStore will not fire the event.

var drone, scheduler, moment;

drone = require('schedule-drone');
moment = require('moment');

drone.setConfig({
  persistence: {
    type: 'mongodb',
    connectionString: 'mongodb://localhost:27017/scheduled-events',
    options: {}
  }
});

scheduler = drone.daemon();
var date = moment().add('seconds', 5).toDate()
scheduler.scheduleAndStore(date, 'my-one-time-event', {"prop1":date,"prop2":"persistence"}, function(err,resp){
    console.log("done");
});

scheduler.schedule(date, 'my-one-time-event', {"prop1":date,"prop2":"non persistence"});

scheduler.on('my-one-time-event', function(params) {
    console.log(JSON.stringify(params));
});
topliceanu commented 9 years ago

I can confirm this issue withing a test. Working on a fix!

topliceanu commented 9 years ago

The correct usage of the module is displayed in this test case

Below I've modified your example to make it work:

var drone, scheduler, moment;

drone = require('schedule-drone');
moment = require('moment');

drone.setConfig({
  persistence: {
    type: 'mongodb',
    connectionString: 'mongodb://localhost:27017/scheduled-events',
    options: {}
  }
});

scheduler = drone.daemon();
var date = moment().add('seconds', 5).toDate()
scheduler.schedule(date, 'my-one-time-event', {"prop1":date,"prop2":"persistence"}, function(err,resp){
    console.log("done");
});

// !!!!! Here is the change, use the .schedule() method as per the README!
scheduler.schedule(date, 'my-one-time-event', {"prop1":date,"prop2":"non persistence"});

scheduler.on('my-one-time-event', function(params) {
    console.log(JSON.stringify(params));
});

If it still doesn't work, please submit a pull request with a failing test case. Thank you!

topliceanu commented 9 years ago

The method scheduleAndStore is not in the public api and thus it should not be used. I'm not going to work on this anymore because I'm working on a rewrite of the module and I'm focusing on that. Please help out by suggesting an API which feels natural to you