new complexScalingLogic struct for SO definition#1189
new complexScalingLogic struct for SO definition#1189gauron99 wants to merge 4 commits intokedacore:mainfrom
Conversation
✅ Deploy Preview for keda ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Thank you for your contribution! 🙏 We will review your PR as soon as possible.
While you are waiting, make sure to:
Learn more about:
|
gauron99
force-pushed
the
scaling-logic-docu
branch
from
cfd6205 to
7b6040f
Compare
JorTurFer
left a comment
JorTurFer
left a comment
There was a problem hiding this comment.
I ❤️ this feature!
Maybe we should add an example for each case, as this is an advanced feature, some examples could help with the adoption
| fallback: # Optional. Section to specify ec-fallback options | ||
| replicas: 3 # Mandatory if fallback section is included | ||
| failureThreshold: 3 # Mandatory if fallback section is included |
There was a problem hiding this comment.
Do we need a second fallback? I mean, we already have a fallback section and duplicate it could be confusing
There was a problem hiding this comment.
If this fallback is for the external server, I think that it's wrong indented
There was a problem hiding this comment.
the fallback is for external servers, if one of them fails, the fallback is applied, works the same like normal triggers. I was thinking if triggers have error, normal fallback is applied and this logic can still work, therefore if any of the server fail, that would be the fallback for it. But i agree, this could be simplified. I could simply call the same fallback function as the standard one if an error occurs in external servers
There was a problem hiding this comment.
ive got rid of the grpc server fallback entirely and everything is handled with the default fallback as it exists currently
| - type: Percent | ||
| value: 100 | ||
| periodSeconds: 15 | ||
| complexScalingLogic: # Optional. Section to specify complex scaling logic |
There was a problem hiding this comment.
complexScalingLogic sounds weird, doesn't? WDYT about something like modifiers ?
| complexScalingLogic: # Optional. Section to specify complex scaling logic | ||
| formula: {formula-for-fetched-metrics} # Optional. Formula for calculation | ||
| target: {target-value-to-scale-on} # Optional. If metrics are anyhow composed together | ||
| externalCalculators: # Optional. Section for user defined grpc calculation servers |
There was a problem hiding this comment.
As above, something like externalModifiers?
There was a problem hiding this comment.
externalModifiers does sound pretty good. combining these two would then be
modifiers:
externalModifiers:
- name: ...
what about just leaving the servers as external as they would be already in modifiers field?
There was a problem hiding this comment.
Ive opened kedacore/keda#4583 for review. Im guessing this will be discussed there
yes Im definitelly going to add examples, Im still drafting the actual PR as well (i shouldve probably put this as draft as well, thats my bad), i thought of more cases to test and want to cleanup unnecessary code before putting it up for review. Im happy to change the structures before that so we can discuss here edit: added some examples for each case |
gauron99
force-pushed
the
scaling-logic-docu
branch
from
92539ac to
a7e33c5
Compare
Signed-off-by: gauron99 <fridrich.david19@gmail.com>
Signed-off-by: gauron99 <fridrich.david19@gmail.com>
Signed-off-by: gauron99 <fridrich.david19@gmail.com>
gauron99
force-pushed
the
scaling-logic-docu
branch
from
f0d5db8 to
dcd98e4
Compare
|
@gauron99 this can be closed, right? |
|
yes, sorry i completely forgot about this one @zroubalik |
3 participants
This PR is opened for kedacore/keda#4583 new feature