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

feat: Adding global kedaAutoscaling section #7392

Merged
merged 7 commits into from
Feb 19, 2024

Conversation

beatkind
Copy link
Contributor

@beatkind beatkind commented Feb 15, 2024

What this PR does

This PR adds a global kedaAutoscalingsection with the option to configure the prometheusAddress, pollingInterval& customHeaders.

Which issue(s) this PR fixes or relates to

Related to #7368

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@beatkind beatkind requested a review from a team as a code owner February 15, 2024 12:58
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 💪 I'm not sure about the per-component overriding, I can't think of a use case for it.

not sure how the few incorrect manifests slipped in 🤔 I checked with the original PR and everything else seems correct

@beatkind
Copy link
Contributor Author

Thanks for the PR 💪 I'm not sure about the per-component overriding, I can't think of a use case for it.

not sure how the few incorrect manifests slipped in 🤔 I checked with the original PR and everything else seems correct

@dimitarvdimitrov I decided to remove the per component config, as you stated there will probably never be a realistic use case. And even if there would be in some edge case, this is a feature that does not need to be shipped to the majority of users. Further details of my thought process: #7392 (comment)

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

nicely done! I left only one more suggestion with some docs, otherwise LGTM

operations/helm/charts/mimir-distributed/values.yaml Outdated Show resolved Hide resolved
@dimitarvdimitrov dimitarvdimitrov enabled auto-merge (squash) February 19, 2024 13:10
@dimitarvdimitrov dimitarvdimitrov merged commit dcce59d into grafana:main Feb 19, 2024
30 checks passed
@beatkind beatkind deleted the support-keda-prometheusurl branch February 19, 2024 13:26
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.

2 participants