[FS-1845] Deploy RabbitMQ on k8s for CI#3236
Conversation
akshaymankar
changed the title
zebot
added
the
ok-to-test
elland
force-pushed
the
backend-notifications-begins
branch
from
23b7a63 to
9cbefca
Compare
services/backend-notification-pusher/src/Wire/BackendNotificationPusher/Options.hs
Outdated
Show resolved
Hide resolved
|
I'm a bit skeptical about making this a separate service. There is a lot of overlap between the code in federator and in notification-pusher (the requests that are currently going to federator will be pushed to the queue in some form by federator, and then picked up by notification-pusher, is that the plan? So the forwarding is currently done by federator, and will be done by notification-pusher in the future?). And I don't think scalability will become a problem if we do everything in one scalable service rather than several. I am all for separating concerns, but I would prefer doing that using module boundaries rather than microservices. |
akshaymankar
force-pushed
the
backend-notifications-begins
branch
from
337cba4 to
6fa2603
Compare
The idea is that this service will make calls to federator, which will do its thing as usual. The new service will not duplicate code in federator. The idea of why this service should be separate is simple: This is not an HTTP service, it is a background worker. There was a plan to make federator's internal service into haskell library and make federated calls directly from brig, galley and cargohold which follows the reasoning of not making another HTTP calls instead of a function call, but I am not sure why it was not followed. Even if we do that plan, I would say this new service should be running as a separate process as it not being an http server is significant enough change to warrant different scaling and process management. |
Also: Refactor RabbitMQ -> RabbitMq
The happy path cannot be tested with just mock federation anymore, it needs to be tested end 2 end, as we cannot easily mock the backend notification pusher. The sad path with remote being down is not relevant anymore as it will be dealt with by the backend notification pusher.
elland
force-pushed
the
backend-notifications-begins
branch
from
7f4f7a6 to
48d1327
Compare
This also renames tag 'federator' to 'federation' on the wire-sever helm chart.
They need to spin up brig
mdimjasevic
left a comment
mdimjasevic
left a comment
There was a problem hiding this comment.
It's still a draft PR, but given that I was asked to review, I'd expect another change log apart from the one mentioning renaming in Helm charts.
elland
force-pushed
the
backend-notifications-begins
branch
from
a6334da to
f6f43ab
Compare
elland
force-pushed
the
backend-notifications-begins
branch
from
c157d5b to
e9a5812
Compare
elland
changed the title
6 participants
The scope changed from the discussions. The work will now be split into two PRs, this one deals with CI infrastructure only, the next one incorporates it into Brig.