docs: ADR for publishing a RabbitMQ settings update message #4654
Conversation
shepilov
force-pushed
the
adr_common_setting_rabbitmq_publish
branch
from
2898820 to
b519590
Compare
| This approach has several issues: | ||
|
|
||
| - **Synchronous blocking**: HTTP calls block the settings update operation | ||
| - **Version conflict complexity**: The code checks remote versions before updates, creating complex conflict resolution logic |
There was a problem hiding this comment.
This was done by purpose. Because we need to have a source of truth.
I'm not sure we're dealing with that correctly from the common-settings app side right now.
If we have 2 app sending a message to update CS, we'll have an issue on CS side. And this cozy-stack today is the entry point of CS, the check was done there.
So if we remove this check, we need to be sure that CS can handle this stuff. cc @rezk2ll
There was a problem hiding this comment.
Yes, the version control is done in CS for incoming requests.
but IMO, the CS app can retire and the stack will handle it. so in reality, we will have only one app sending the message updates.
There was a problem hiding this comment.
linagora/twake-workplace-common-settings
| - **Version conflict complexity**: The code checks remote versions before updates, creating complex conflict resolution logic | ||
| - **Tight coupling**: Direct HTTP dependency on the common settings app | ||
| - **Failure propagation**: HTTP failures can affect the main settings update flow | ||
| - **No longer needed**: The common settings app is no longer actively updating settings, making version conflict checking obsolete |
There was a problem hiding this comment.
I don't understand this point, can you elaborate?
There was a problem hiding this comment.
The common settings app is being deprecated, so the stack no longer needs to support conflict resolution
There was a problem hiding this comment.
What are you calling "common settings app"?
There was a problem hiding this comment.
linagora/twake-workplace-common-settings
There was a problem hiding this comment.
Oh I didn't even know that there was a front app for this API. This front app is not used anywhere.
The version's check on stack was not done because of that, it was done to be sure that we're sending the up to date data. And be sure that the user updated the latest version.
There was a problem hiding this comment.
So I still don't understand. We will alwaus need this backend app right?
There was a problem hiding this comment.
if the stack handles the message sending part, we no longer need this app.
The CS app was created because that stack could not send RabbitMQ messages and also connect to different RabbitMQ servers on a different infra.
Both of those issues are solved .
The other argument to keep the CS app was the on-prem installations. But I believe now it's mandatory to ship the stack on-prem if you want common settings. so really, no need to keep the app
There was a problem hiding this comment.
But we still need CS app to consume the message, no?
There was a problem hiding this comment.
no, the other apps will consume the message directly no need for CS to relay the message.
CS app was exposing an API to the stack -> the stack sends the settings -> the app stores the settings and checks the version etc -> then sends a rabbitmq message to everyone.
The stack is already a copy of the source of the truth and is now capable of sending the RabbitMQ messages. so we just need to add the version control check and replace the API call with a RabbitMQ message.
There was a problem hiding this comment.
But today we have a DB for CS, no? I liked the idea of having an external DB for that. Don't we have a "script" reading this DB to send event in order to fully align all the listeners (in case of something not working? Like what we had few weeks ago?) If we don't have this external DB anymore, and we only store in CouchDB, it'll be a mess to do.
If we want to do that, then the cozy-stack still need to checks the version and else, so I don't understand how we can simplify or remove this check?
|
@Crash-- so, do you have any comments, or should I mark it as decided, merge, and plan for implementation? |
3 participants
No description provided.