search

FirebaseExtended / firebase-queue

MIT License
787 stars 108 forks source link

Feature request: Add a "Does not Inherit" clause in docs for spec #19

Closed uofmmike closed 9 years ago

uofmmike commented 9 years ago

Just a thought here, if its possible-- add a "does not inherit clause in the docs for the specs.

Ran into some trouble, i assume that if defaults were not specificed ,

Say my spec was:

 "spec_1": {   
    "timeout": 10000
  }

I assumed it would translate to:

  "spec_1": {
    "start_state": null,
    "in_progress_state": "in_progress",
    "finished_state": null,
    "error_state": "error",
    "timeout": 300000, // 5 minutes
    "retries": 0 // don't retry
  }

But the defaults (as far as i could tell) are note appended to the object that i create. Just a thought, could be completely off base, but thought I would at least raise the question!

Worker is great, thank you for the great product!

cbraynor commented 9 years ago

Right, I agree this is a little broken and inconsistent, but this was tricky because the queue uses a null timeout to switch it to an at-most-once mode (never time a job out) and in Firebase an undefined attribute is the same as a null attribute.

Was this suggestion to just make the docs a little clearer, or to change the behavior a little?

We can't fix the behavior in any 1.x release because it's too big a breaking change, but in 2.0.0 we could switch to using 0 for special-casing a task that doesn't timeout. I don't think that alone is worth a bump in major versions, but perhaps the addition of delayed, scheduled and repeating tasks along with it would be worthy of the bump...

As I use this more, I tend to be very explicit with my task specifications to avoid unexpected behavior, which suggests to me things aren't quite right. The biggest inconsistency other than the timeout is that in_progress_state doesn't have a default (if you specify a specId in the queue options). The reasoning behind this was that it's the attribute that defines the job that's running and what to look for to time out other jobs - having two task specs the same in_progress_state would cause very weird behavior, particularly around job timeouts and resetting jobs to a potentially wrong start_state.

I could perhaps be convinced that we could make a non-breaking change to get that to default to "in_progress" just like the error_state does to "error", but then timeout is the one attribute that doesn't work like the others.