-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
prometheus-xmpp-alerts: 0.4.2 -> 0.5.1; apply RFC 42 #100274
Conversation
Result of 1 package built:
|
The fix is apparently in this commit: jelmer/prometheus-xmpp-alerts@8b4c2f7 but I see no harm in just bumping to the newest commit. It includes a commit by @andir ;-) I saw. This looks fine by me. I am just not sure if there is some other convention for the version name that we should use. I would like a second opinion on that last point, but ping me, if you don‘t get any more reactions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please report this issue upstream and have them issue a fixed release.
@mweinelt The issue is already fixed upstream. (As I pointed to in the commit I linked.) Do you want to wait with this PR until they have made the next release? |
I'm saying let's apply the patch and wait for upstream to do a new release. Let's not start doing releases downstream. |
Fair enough. |
You want at least 0f54da76ecd200887e7f267113b7c6a9cfae43d4 & 500a316a83b5d72c490cb62c3e7f5aa5efc7f5bf from my fork in order to actually be able to use the amtool integration of this. I filed a PR for those just now: jelmer/prometheus-xmpp-alerts#19 |
While we are waiting for upstream and/or the required fixes you can probably start adding a nixos module based on https://github.com/andir/infra/blob/e078bda5b9dafbfff2216a8e9131ce180196c362/config/servers/mon/xmpp-alerts.nix. |
Please update to the latest release: jelmer/prometheus-xmpp-alerts@1620e9e |
@hax404 ping |
b1ed827
to
e74c258
Compare
e74c258
to
cc94465
Compare
cc94465
to
0261c3f
Compare
0261c3f
to
2cac7d6
Compare
LGTM, but other people in this thread are more familiar with these parts of nixpkgs. |
2cac7d6
to
daff272
Compare
daff272
to
0eaae2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package looks fine, the module can be migrated further to RFC 42 still, also in a backward compatible way.
0eaae2b
to
03c0925
Compare
Motivation for this change
Fix 500s that come back when the alertmanager sends alerts
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)