keda icon indicating copy to clipboard operation
keda copied to clipboard

add new a filed InitialDelayCooldownPeriod delay the effective time…

Open RonaldFletcher opened this issue 1 year ago • 5 comments

… of CooldownPeriod

·

Add a new field InitialDelayCooldownPeriod The purpose of adding a new field InitialDelayCooldownPeriod is to delay the effective time of the CooldownPeriod, so that when the service is started for the first time, it needs to wait for the time of InitialDelayCooldownPeriod to expire before destroying the service.

Checklist

Fixes https://github.com/kedacore/keda/issues/5008

Relates to #

RonaldFletcher avatar Feb 04 '24 09:02 RonaldFletcher

Hi, @JorTurFer is this PR getting merged anytime soon? Our current workaround for handling the delay at the time of deployment is causing challenges. This feature will be very helpful for many of our use cases.

shubham-kaushal avatar Feb 26 '24 15:02 shubham-kaushal

Hi, @JorTurFer is this PR getting merged anytime soon? Our current workaround for handling the delay at the time of deployment is causing challenges. This feature will be very helpful for many of our use cases.

I hope so, but the feature isn't ready to merge yet because test coverage is required. IDK the state of it, @xrwang8 ?

JorTurFer avatar Feb 26 '24 15:02 JorTurFer

I've added e2e testing. @JorTurFer

RonaldFletcher avatar Feb 28 '24 07:02 RonaldFletcher

What's wrong with that now @JorTurFer

RonaldFletcher avatar Mar 04 '24 11:03 RonaldFletcher

Hi @JorTurFer , could you plz. check this PR. E2E test has been added by @xrwang8

shubham-kaushal avatar Mar 05 '24 11:03 shubham-kaushal

/run-e2e initial_delay_cooldownperiod Update: You can check the progress here

JorTurFer avatar Mar 12 '24 21:03 JorTurFer

Hi @JorTurFer , could you plz. check this PR. E2E test has been added by @xrwang8

Sorry, I've been quite disconnected :( I have left some comments inline. Apart from them ,we need to fix DCO check for merging the PR, you can find the steps to fix it in the check itself image

JorTurFer avatar Mar 12 '24 21:03 JorTurFer

I have simplified e2e @JorTurFer

RonaldFletcher avatar Mar 13 '24 09:03 RonaldFletcher

/run-e2e internal Update: You can check the progress here

JorTurFer avatar Mar 15 '24 23:03 JorTurFer

Could you open a PR against docs repo too, adding this new parameter to the ScaledObject docs?

JorTurFer avatar Mar 15 '24 23:03 JorTurFer

Could you open a PR against docs repo too, adding this new parameter to the ScaledObject docs?

ok

RonaldFletcher avatar Mar 17 '24 06:03 RonaldFletcher

I have modified the document https://github.com/kedacore/keda-docs/pull/1337 @JorTurFer

RonaldFletcher avatar Mar 18 '24 01:03 RonaldFletcher

I have modified the document kedacore/keda-docs#1337 @JorTurFer

Let me check them, sorry for the delay... last week was KubeCon EU

JorTurFer avatar Mar 25 '24 22:03 JorTurFer

LGTM! Could you add a record in changelog.md file too? I think that #New section is the best place for this feature

i hava already done @JorTurFer

RonaldFletcher avatar Mar 26 '24 01:03 RonaldFletcher

/run-e2e internal Update: You can check the progress here

JorTurFer avatar Mar 26 '24 18:03 JorTurFer

@zroubalik PTAL

JorTurFer avatar Mar 26 '24 19:03 JorTurFer

What is the current status of this PR? @JorTurFer

RonaldFletcher avatar Apr 02 '24 13:04 RonaldFletcher

What is the current status of this PR?

It's ready to merge IMHO, but I'd like to read @zroubalik thoughts too :)

JorTurFer avatar Apr 03 '24 20:04 JorTurFer

Hey! @zroubalik , could you please take a look at this. This will save a bit of development for us. Thank you. cc: @JorTurFer

aj-ya avatar Apr 08 '24 12:04 aj-ya

@zroubalik done

RonaldFletcher avatar Apr 11 '24 01:04 RonaldFletcher

/run-e2e internals Update: You can check the progress here

zroubalik avatar Apr 11 '24 12:04 zroubalik

@JorTurFer can it be merged ?

RonaldFletcher avatar Apr 12 '24 01:04 RonaldFletcher