Announce message notifications via fedmsg #253
Conversation
|
For Hubs I'll just listen to all context for all users, I think, so it doesn't matter. However outside of Hubs it feels like the most common use case would be to subscribe to all contexts for a user, so Do you have examples of messages with and without the simple formatter? |
|
The un-formatted message would just be a fedmsg with |
|
Let's keep the original message then, thanks! |
jeremycline
force-pushed
the
fedmsg-notifier
branch
from
703e15e
to
951e556
Compare
Send a notification of the notification we're about to send via fedmsg. Make sure the consumer does not try to process that notification. Signed-off-by: Jeremy Cline <jeremy@jcline.org>
jeremycline
force-pushed
the
fedmsg-notifier
branch
from
951e556
to
f5da020
Compare
Codecov Report
@@ Coverage Diff @@
## develop #253 +/- ##
===========================================
+ Coverage 55.23% 55.32% +0.08%
===========================================
Files 72 72
Lines 3760 3767 +7
Branches 495 496 +1
===========================================
+ Hits 2077 2084 +7
+ Misses 1606 1605 -1
- Partials 77 78 +1
Continue to review full report at Codecov.
|
jeremycline
changed the title
| if 'fmn.notification' in topic: | ||
| # This is a notification of a notification we've sent. Processing it | ||
| # would lead to a positive feedback loop. | ||
| return |
There was a problem hiding this comment.
Is this unit-tested? I think it should be, it's pretty important.
There was a problem hiding this comment.
Definitely agree. Sorry for the slow response, I got caught up in a bunch of other stuff. After I get the next round of FMN bugfixes in and deployed, I'll write tests for this and get it in 2.1.
Send a notification of the notification we're about to send via fedmsg.
Make sure the consumer does not try to process that notification.
@abompard I thought I'd just put this up and we could talk about the details of what you need on the PR. I was thinking about implementing a simple formatter for the message so it would arrive on your end as human-readable, but I thought you might want to control that so maybe it makes sense to just resend the original fedmsg. Also, I don't know whether the message topic should be "notification.{user}.{context}" or "notification.{context}.{user}" - the first lets you subscribe to all contexts for a user, or all contexts for all users, while the second lets you subscribe to all users for a context, or all contexts for all users.