Option to keep the ttl while releasing the task

[qdrin]

Adds the option 'touch_ttl' that is assotiated with delay option in the release method of fifottl. Allows keep the task ttl unchanged when it necessary. Might help with retries.

[Totktonada]

I would name a new option in a way to make its default value false: say, keep_ttl. It looks more natural for me.

I guess the code hunk may be written in a simpler way (didn't tested):

task = self.space:update(id, {
    { '=', i_status, state.DELAYED },
    { '=', i_next_event, util.event_time(opts.delay) },
    not opts.keep_ttl and { '+', i_ttl, util.time(opts.delay) } or nil,
})

[qdrin]

Thanks for the fine advise. Renamed

[Totktonada]

It would be good to support it in 'utubettl' driver too.

[qdrin]

Added in new commit. Thanks for Your idea

[Totktonada]

We usually cover all new functionality with a test. Aren't you mind to add one? They're in the t/ directory.

[qdrin]

Covered in new commit. Sorry for that

[rybakit]

IMHO, introducing a new option that solely depends on another option is really not the best solution to the problem. I'm afraid it will be a source of confusion for users, because the API allows the "associated" option to be passed alone and, when allowed, is likely to be misused. So I expect to see in the future something like

t:release(15, {keep_ttl=true})

which makes no sense (however, it gives a false impression that it somehow affects the task ttl). Moreover, when #125 gets implemented, we might get

t:release(15, {delay=10, keep_ttl=true, ttl=10})

which is a valid combination of options, although it looks very weird and confusing.

As an alternative, I would consider adding a new standalone option, something like delay_with_ttl or delay_wo_ttl (or any other name that works best), so at least it can be used alone. Also, we might think of forbidding passing both delay and delay_with_ttl or at least clearly define the precedence of these options.

Another alternative is to introduce a new api method.

[rybakit]

After thinking more about the issue, I believe it can be solved in the scope of #125. If we had the ability to pass more options to release() we could do

t:release(15, {delay=10, ttl=-1})

where ttl=-1 indicates that we don't want to update ttl.

[qdrin]

IMHO, introducing a new option that solely depends on another option is really not the best solution to the problem. I'm afraid it will be a source of confusion for users, because the API allows the "associated" option to be passed alone and, when allowed, is likely to be misused. So I expect to see in the future something like

t:release(15, {keep_ttl=true})

which makes no sense (however, it gives a false impression that it somehow affects the task ttl). Moreover, when #125 gets implemented, we might get

t:release(15, {delay=10, keep_ttl=true, ttl=10})

which is a valid combination of options, although it looks very weird and confusing.

As an alternative, I would consider adding a new standalone option, something like delay_with_ttl or delay_wo_ttl (or any other name that works best), so at least it can be used alone. Also, we might think of forbidding passing both delay and delay_with_ttl or at least clearly define the precedence of these options.

Another alternative is to introduce a new api method.

Perhaps the option is not the best solution. But in the current implementation it doesn't seem confusive as the only case with ttl update is the release.
I agree the #125 is fine and covers much more cases. So it's implementation seems to be more complicated and i suppose deserves the particular driver. Meenwhile my purpose was to give some new uses to current one with minimum efforts.

[LeonidVas]

HI! Thank you for the patch.
I have several questions:

• Why can't you just use the touch method for this purpose?
• Could you to describe few use case of the feature?

[qdrin]

IMHO, introducing a new option that solely depends on another option is really not the best solution to the problem. I'm afraid it will be a source of confusion for users, because the API allows the "associated" option to be passed alone and, when allowed, is likely to be misused. So I expect to see in the future something like

t:release(15, {keep_ttl=true})

which makes no sense (however, it gives a false impression that it somehow affects the task ttl). Moreover, when #125 gets implemented, we might get

t:release(15, {delay=10, keep_ttl=true, ttl=10})

which is a valid combination of options, although it looks very weird and confusing.

As an alternative, I would consider adding a new standalone option, something like delay_with_ttl or delay_wo_ttl (or any other name that works best), so at least it can be used alone. Also, we might think of forbidding passing both delay and delay_with_ttl or at least clearly define the precedence of these options.

Another alternative is to introduce a new api method.

Perhaps the option is not the best solution. But in the current implementation it doesn't seem confusive as the only case with ttl update is the release.
I agree the #125 is fine and covers much more cases. So it's implementation seems to be more complicated and i suppose deserves the particular driver. Meenwhile my purpose was to give some new uses to current one with minimum efforts.

* Could you to describe few use case of the feature?

My case is to retry failed tasks (http requests) every 30 seconds for 6-minutes period. So I need ttl to be constant. Shure if touch() could decrease ttl it would solve the issue along with "classic" release()

[qdrin]

Sorry it seems i close the request accidently. Let's me reopen

[LeonidVas]

My case is to retry failed tasks (http requests) every 30 seconds for 6-minutes period. So I need ttl to be constant. Shure if touch() could decrease ttl it would solve the issue along with "classic" release()

Is it close to ttr = 30 with ttl = 360. Did I understand your case correctly?
Or maybe it's like ttl = 360 and release with delay = 30.

[LeonidVas]

Well, thanks for the PR, but it looks like we haven't decided yet whether such functionality is needed in the module and how exactly the API of this functionality should look like. I think it would be good to resolve these questions in discussion #125, and then start implementing the feature.