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

Datadog: isActive() should react on num > 0 #2798

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

zroubalik
Copy link
Member

@zroubalik zroubalik commented Mar 22, 2022

Signed-off-by: Zbynek Roubalik zroubalik@gmail.com

I confused @arapulido on #2694 to include a wrong implementation of IsActive() function, fixing this here.

Checklist

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

Relates #2657

Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
@zroubalik zroubalik requested a review from a team as a code owner March 22, 2022 20:54
@zroubalik zroubalik changed the title Datadog: isActive should react on num > 0 Datadog: isActive() should react on num > 0 Mar 22, 2022
@zroubalik
Copy link
Member Author

zroubalik commented Mar 22, 2022

/run-e2e datadog*
Update: You can check the progres here

@arapulido
Copy link
Contributor

Mmm, I'm very confused about this. A metric can have a positive value, or a negative, so checking this doesn't give much information.

What's the goal?

I tested the code before this, and I was able to make to 0.

Can we wait until tomorrow morning before merging this, please?

I would like to do more testing and report

@zroubalik
Copy link
Member Author

zroubalik commented Mar 22, 2022

@arapulido this is just aligning the functionality with all the existing scalers, so the experience is consistent. isActive() should return true if there is any activity on the scaler. We are discussing on making this configurable (ie adding a new property for the activation threshold) -> #2800

@arapulido
Copy link
Contributor

@arapulido this is just aligning the functionality with all the existing scalers, so the experience is consistent. isActive() should return true if there is any activity on the scaler. We are discussing on making this configurable (ie adding a new property for the activation threshold) -> #2800

OK, as discussed on Slack, happy to be consistent with the rest of scalers.

@zroubalik zroubalik merged commit f8f86bd into kedacore:main Mar 23, 2022
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