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

Rely on Datadog API to validate the query #2771

Merged
merged 3 commits into from
Mar 22, 2022

Conversation

arapulido
Copy link
Contributor

@arapulido arapulido commented Mar 17, 2022

Instead of trying to validate all potential valid queries ahead of creating the scaledobject, rely on the Datadog API to do so and return the error returned by the API if that happens. Also, rely on the Datadog API to apply the default rollup time frame if one is not specified in the query.

This way, if there are any addition to the query language down the line, we won't need to change the scaler code, while simplifying it.

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements

Fixes #2761

@arapulido arapulido requested a review from a team as a code owner March 17, 2022 17:05
@JorTurFer
Copy link
Member

JorTurFer commented Mar 17, 2022

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

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for this improvement ❤️
Could you update the changelog?

@arapulido arapulido force-pushed the simplify_dd_query_parsing branch from 8b2cdc3 to 904f380 Compare March 21, 2022 10:49
@arapulido
Copy link
Contributor Author

LGTM! Thanks for this improvement ❤️ Could you update the changelog?

@JorTurFer thanks for the review! Updated the changelog

@JorTurFer
Copy link
Member

JorTurFer commented Mar 21, 2022

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

@JorTurFer JorTurFer self-requested a review March 21, 2022 10:51
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for your awesome contributions! ❤️

@JorTurFer
Copy link
Member

@arapulido
Could you rebase main please? There was an error with the linter that we fixed yesterday 🙏
Thanks!

Signed-off-by: Ara Pulido <ara.pulido@datadoghq.com>
Signed-off-by: Ara Pulido <ara.pulido@datadoghq.com>
Signed-off-by: Ara Pulido <ara.pulido@datadoghq.com>
@arapulido arapulido force-pushed the simplify_dd_query_parsing branch from 904f380 to 8bd04da Compare March 22, 2022 10:46
@arapulido
Copy link
Contributor Author

@arapulido Could you rebase main please? There was an error with the linter that we fixed yesterday 🙏 Thanks!

done!

@JorTurFer JorTurFer merged commit 5dbb6cc into kedacore:main Mar 22, 2022
RamCohen pushed a commit to RamCohen/keda that referenced this pull request Mar 23, 2022
Signed-off-by: Ram Cohen <ram.cohen@gmail.com>
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.

Datadog query with functions never returns healthy
2 participants