Deprecate StreamMicrogridDispatches RPC#351
Conversation
Marenz
requested review from
stefan-brus-frequenz
and removed request for
a team
github-actions
Bot
added
part:docs
|
What are we gaining by removing streaming from this API? It is a breaking change, what's the improvement here? |
|
Streaming is controller-only and no public client uses it - Kuiper proxy and the etrading actor are both CRUD-only. Removing it just keeps the public API honest. But it's a breaking change for no functional gain, so I'm happy to keep it in the public API too - worth discussing whether the tidier surface is worth the break. |
|
We can deprecate it to make it clear the intention is to remove it in the future. |
Marenz
force-pushed
the
split-hostdispatch
branch
from
7d11f0e to
2c1d9d2
Compare
Marenz
changed the title
| > [!NOTE] | ||
| > Real-time dispatch streaming for controllers is moving to the dedicated | ||
| > [HostDispatch API](https://github.com/frequenz-io/frequenz-api-hostdispatch) | ||
| > (private, controller-only). The `StreamMicrogridDispatches` RPC in this API is | ||
| > deprecated and will be removed in a future major release. New controller-facing | ||
| > integrations should use `HostDispatchService`. |
There was a problem hiding this comment.
This note makes no sense as it is...
| and `StreamMicrogridDispatchesResponse` messages are now deprecated. Real-time | ||
| dispatch streaming for controllers is moving to the dedicated | ||
| `HostDispatchService` in the | ||
| [HostDispatch API](https://github.com/frequenz-io/frequenz-api-hostdispatch). |
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
Well, actually the mention to the internal repo is everywhere. It makes no sense in a private repo, and we don't even know if we are going this way, so I would just remove all mentions to it and just mention the API will be split for now without any further precision.
Mark the controller-facing stream RPC and its request/response messages as deprecated instead of adding or removing service surface. This keeps the change non-breaking while documenting that this part of the API will move out in a future Dispatch API split. Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
Marenz
force-pushed
the
split-hostdispatch
branch
from
2c1d9d2 to
0700309
Compare
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
We should only do this once we know exactly where are we moving it, so I would wait on this too for now.
2 participants
Deprecate the controller-facing
StreamMicrogridDispatchesRPC and its request/response messages instead of removing them.This keeps the change non-breaking:
StreamMicrogridDispatchesRequest/StreamMicrogridDispatchesResponsemessages, and the streaming event import are all retained.option deprecated = trueplus generic doc notes.No specific target repository or replacement service is named here; the exact split shape is left unspecified for now.