timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.05k stars 157 forks source link

Suggestions for improved subscribe API #54

Closed bradleyayers closed 6 years ago

bradleyayers commented 6 years ago

I think the subscribe API should be simplified, which would make it easier for users to understand and for you to maintain. The changes I'd vote for are:

An example of where I found the existing API confusing:

https://github.com/timgit/pg-boss/blob/master/docs/usage.md#expired-job

The payload is the same job object that the subscriber's handler function receives with id, name and data properties.

When I read this I thought it might mean that it includes the done callback too. But from looking at the code it looks like it doesn't, so technically you might consider it not the same job object that the subscriber's handler function receives.

It would also be great to be able to do something like throw a RetryJobError and have the job automatically retried later with exponential back-off.

timgit commented 6 years ago

I'm going to nuke how expired is handled in 3.0. It's one of the oldest parts of the api that I'm not happy with. I added state subs after that, and I now want to consolidate completion subs (success, fail, expire) into just onComplete().

Forcing an async handler, though? I don't really know if that should be a requirement or not. Seems like overkill.

bradleyayers commented 6 years ago

I'm going to nuke how expired is handled in 3.0. It's one of the oldest parts of the api that I'm not happy with. I added state subs after that, and I now want to consolidate completion subs (success, fail, expire) into just onComplete().

Consolidation sounds good, presumably the state would be available within onComplete so the user can respond to success/fail/expire in a single place?

Forcing an async handler, though? I don't really know if that should be a requirement or not. Seems like overkill.

It's the modern natural way to support async operations (now that async/await are natively supported). Though is supporting older versions of node is important, then callbacks are the way to go.

timgit commented 6 years ago

Consolidation sounds good, presumably the state would be available within onComplete so the user can respond to success/fail/expire in a single place?

👍

It's the modern natural way to support async operations (now that async/await are natively supported). Though is supporting older versions of node is important, then callbacks are the way to go.

job.done() does currently return a promise. Maybe I misunderstood your original request on this. Ultimately, it's still a convenience to use this instead of complete() or fail().