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

chore: add pint #958

Merged
merged 1 commit into from
Jul 18, 2024
Merged

chore: add pint #958

merged 1 commit into from
Jul 18, 2024

Conversation

rexagod
Copy link
Collaborator

@rexagod rexagod commented Jul 17, 2024

Continues #838.

@rexagod rexagod force-pushed the pint branch 5 times, most recently from 126b238 to 79f3322 Compare July 17, 2024 22:40
@rexagod
Copy link
Collaborator Author

rexagod commented Jul 17, 2024

Test run: https://github.com/kubernetes-monitoring/kubernetes-mixin/actions/runs/9982477680/job/27588267056?pr=958#step:4:56. LMK if this looks good and I'll send a patch fixing it.

@povilasv
Copy link
Contributor

LGTM, but some merge conflicts / failing ci 👍

@rexagod rexagod force-pushed the pint branch 3 times, most recently from 0b7cdb8 to fca4313 Compare July 18, 2024 08:59
@rexagod rexagod marked this pull request as draft July 18, 2024 09:04
@rexagod rexagod marked this pull request as ready for review July 18, 2024 11:43
@@ -143,7 +143,7 @@
{
expr: |||
(
max without (revision) (
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why this change?

Copy link
Collaborator Author

@rexagod rexagod Jul 18, 2024

Choose a reason for hiding this comment

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

Pint suggested a refactor here.

┌[rexagod@nebuchadnezzar] [/dev/ttys000] [pint ⚡] [2]
└[~/repositories/oss/kubernetes-mixin]> make pint-lint 

prometheus_alerts.yaml:106-123 Warning: Aggregation using `without()` can be fragile when used inside binary expression because both sides must have identical sets of labels to produce any results, adding or removing labels to metrics used here can easily break the query, consider aggregating using `by()` to ensure consistent labels. (promql/fragile)
 106 |     "expr": |
 107 |       (
 108 |         max without (revision) (
 109 |           kube_statefulset_status_current_revision{job="kube-state-metrics"}
 110 |             unless
 111 |           kube_statefulset_status_update_revision{job="kube-state-metrics"}
 112 |         )
 113 |           *
 114 |         (
 115 |           kube_statefulset_replicas{job="kube-state-metrics"}
 116 |             !=
 117 |           kube_statefulset_status_replicas_updated{job="kube-state-metrics"}
 118 |         )
 119 |       )  and (
 120 |         changes(kube_statefulset_status_replicas_updated{job="kube-state-metrics"}[5m])
 121 |           ==
 122 |         0
 123 |       )
make: *** [pint-lint] Error 1

@povilasv povilasv merged commit e4a4969 into kubernetes-monitoring:master Jul 18, 2024
9 checks passed
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