New operations admin APIs and revamped change-event listener on admin WebSocket #668
Conversation
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #668 +/- ##
===========================================
- Coverage 100.00% 99.96% -0.04%
===========================================
Files 316 320 +4
Lines 19437 19586 +149
===========================================
+ Hits 19437 19579 +142
- Misses 0 5 +5
- Partials 0 2 +2
Continue to review full report at Codecov.
|
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
|
I like the idea of the separation of APIs (no apostrophe). Perhaps maybe the name |
| Path: "operations/{opid}", | ||
| Method: http.MethodGet, | ||
| PathParams: []*oapispec.PathParam{ | ||
| {Name: "opid", Description: coremsgs.APIMessageTBD}, |
There was a problem hiding this comment.
I'll just add this to my list of things to write descriptions for 🙂
| @@ -424,7 +421,7 @@ func (sm *subscriptionManager) close() { | |||
| } | |||
| sm.mux.Unlock() | |||
| for _, conn := range conns { | |||
| sm.connnectionClosed(conn.ei, conn.id) | |||
| sm.connectionClosed(conn.ei, conn.id) | |||
| type ChangeEventListener interface { | ||
| // ChangeEvent delivers a change event - no ack for these | ||
| ChangeEvent(connID string, ce *fftypes.ChangeEvent) | ||
| } |
There was a problem hiding this comment.
So ChangeEventListeners are not a distinct concept anymore? ChangeEvents are just a thing that you get on the admin websocket now?
There was a problem hiding this comment.
Mixing together application level events, and these change events, on the same WebSocket with different payloads has been removed. It felt really confusing, and the danger of opening a firehose you can't cope with is now pretty self-evident.
So I guess I think of it as more distinct now, in that you have a whole separate WebSocket to listen to.
So it's got nothing to do with the events plugin at all - it's a system-level thing built into the core.
|
I agree with the |
The policy engine described in hyperledger/firefly-ethconnect#149 (comment) is starting to come together, with some evolved architecture.
The intention is for this to be a separate microservice, so it can be pluggable with different implementations easily, have a separate scaling model, and be closer to EthConnect in where it sits in the architecture than to the core.
However, the source of truth for the completion of operations is still FireFly. So this is more like an augmented component of FireFly, than it is a plugin/connector that performs a particular task on behalf of FireFly. So as such I believe it should connect in to FireFly to perform actions.
Currently (before this PR) we have some of things it would need - with gaps:
So this PR proposes we create a proper separation of concerns between the components that have super-access to update FireFly state, because they are part of it (the
adminAPI already exists for this), and the external API that applications use.GET/admin/api/v1/operationsroutes - including aPUTto update operation status (per same rules that apply to internal plugins like Data Exchange today)/admin/wswith extra options, and a simplified no-confirm model. It's best effort up to a buffer size, and then operation of the core is prioritized over avoiding any event loss, and always ephemeral. No guarantees.