add possiblity of input in seconds to TimeDeltaParameter #3125
Conversation
joooeey
requested review from
dlstadther,
Tarrasch and
a team
as code owners
| @@ -447,7 +447,7 @@ traceback_max_length | |||
| Parameters controlling the contents of batch notifications sent from the | |||
| scheduler | |||
|
|
|||
| email_interval | |||
| email_interval_minutes | |||
There was a problem hiding this comment.
this config variable should remain email_interval, that's the name which is searched for by the code
Line 17 in 857deff
There was a problem hiding this comment.
I made a change in that file. But looks like tox threw it out. I didn't know I have to commit before running tox. Please check the commit I'll make in a few minutes.
There was a problem hiding this comment.
Committed. I'm not sure if it solves the issue though. And I don't know how I'd go about writing a test for it. Do you have any tips?
EDIT: can I make the code lines linked to appear in line like you did?
There was a problem hiding this comment.
Cool; yeah, this is what i was expecting.
As for the link/code preview, i think it has to be a code link from master. I didn't do anything special other than past the link.
| @@ -447,7 +447,7 @@ traceback_max_length | |||
| Parameters controlling the contents of batch notifications sent from the | |||
| scheduler | |||
|
|
|||
| email_interval | |||
| email_interval_minutes | |||
There was a problem hiding this comment.
Cool; yeah, this is what i was expecting.
As for the link/code preview, i think it has to be a code link from master. I didn't do anything special other than past the link.
|
Apologies for massive delay. Stalebot comments on a bunch of Luigi PRs and that reminded me that I approved some PRs and never merged them. Going back through them now, updating them with master, and confirming that all their checks pass. |
Description
Allow input as number in seconds to
TimeDeltaParameterto makemax_keep_alive_idle_duationexcept the same format as most other config options (i.e. seconds). This means that anyTimeDeltaParameterused in aluigi.Taskcan also be given as a bare number and interpreted as seconds.Plus two small changes:
batch_notifier.email_intervalgets a new aliasbatch_notifier.email_interval_minutesto make it obvious that a different unit is used there.Motivation and Context
Closes #3123. Making duration config parameters as consistent as possible. (
batch_notifier.email_intervalis still given in minutes but renamed tobatch_notifier.email_interval_minutes).Have you tested this? If so, how?
I have included two new unit tests.