Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Add a note on scalers which support float target value #783

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

JorTurFer
Copy link
Member

Signed-off-by: Jorge Turrado jorge_turrado@hotmail.es

After merging the PR to support mili unit in core, there are some scalers that support float as target value (because the scaler logic already supported float64 and it made sense).

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO)

Related kedacore/keda#3135

@netlify
Copy link

netlify bot commented Jun 14, 2022

Deploy Preview for keda ready!

Name Link
🔨 Latest commit 6af7818
🔍 Latest deploy log https://app.netlify.com/sites/keda/deploys/62ab8f02a642230009edb882
😎 Deploy Preview https://deploy-preview-783--keda.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an end-user, how much do I care about this? If I do, might need some more background on what this means maybe? Scaler typically does the heavy lifting on this and in some scalers this does not make sense - For example, I won't be scaling for 30.5 messages.

Maybe it's better to update the scalers where this is useful?

@JorTurFer
Copy link
Member Author

JorTurFer commented Jun 16, 2022

As an end-user, how much do I care about this? If I do, might need some more background on what this means maybe? Scaler typically does the heavy lifting on this and in some scalers this does not make sense - For example, I won't be scaling for 30.5 messages.

Maybe it's better to update the scalers where this is useful?

you are right, on it specifying the parameters which accepts floats

BTW, all the scalers in this PR support float 😄

@JorTurFer JorTurFer force-pushed the mark-float-scalers branch from 9fd4dfa to 9e7b507 Compare June 16, 2022 18:51
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer force-pushed the mark-float-scalers branch from 9e7b507 to 6af7818 Compare June 16, 2022 20:13
@tomkerkhove
Copy link
Member

They do support it, but does it make sense? :D Anyway it's up to them but for SB it's less relevant

@JorTurFer
Copy link
Member Author

JorTurFer commented Jun 17, 2022

Yes, the scalers inclued in this PR support float because it's relevant.
I mean, you can get a rate from 0 to 1 using Azure Monitor, or database scalers, etc.
The only case where could not be relevant is RabbitMQ using queues, but there is a remark explaining that this only applies in case of message rate.
ie, service bus scaler doesn't support float because doesn't make sense

@JorTurFer
Copy link
Member Author

@zroubalik , does this match with your expectations too?

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks for this!

@JorTurFer JorTurFer merged commit de11b10 into kedacore:main Jun 20, 2022
@JorTurFer JorTurFer deleted the mark-float-scalers branch June 20, 2022 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants