Update Kafka Scaler to check for ActivationLagThreshold below 0 #4035
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vicky Ledsom <victorialedsom@gmail.com>
Signed-off-by: Vicky Ledsom <victorialedsom@gmail.com>
Signed-off-by: Vicky Ledsom <victorialedsom@gmail.com>
JorTurFer
reviewed
Dec 22, 2022
There was a problem hiding this comment.
Thanks for the improvement!
Could you review the tests? TestGetBrokers is failing.
BTW, you can maintain the previous behaviour just not setting activationLagThreshold, the default value is 0 but it isn't checked (the value is only checked in case of set activationLagThreshold), so in case of not providing, KEDA uses 0
| @@ -40,6 +40,8 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| - **Kafka Scaler:** Don't error when activationLagThreshold is default | |||
There was a problem hiding this comment.
Could you move this to ### Improvements section?
|
fixed in : #4143 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Vicky Ledsom victorialedsom@gmail.com
We were experiencing issues when not providing
ActivationLagThreshold, the value is then defaulted to 0 but the logic was checking for a value of less or equal to 0. Which would always fail withmust be positive numberwhen using the default. If the plan to keep the default as 0 then this logic needs changing. Hence this PR.Checklist