ability to disable grpc and/or event consumer for event history service - #3279
ability to disable grpc and/or event consumer for event history service#3279maki5 wants to merge 6 commits into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 18 |
| Duplication | 7 |
🟢 Coverage 1.60% diff coverage · -0.02% coverage variation
Metric Results Coverage variation ✅ -0.02% coverage variation (-1.00%) Diff coverage ✅ 1.60% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (1fa8950) 84104 19334 22.99% Head commit (1d55c4d) 84151 (+47) 19326 (-8) 22.97% (-0.02%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3279) 125 2 1.60% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| } | ||
|
|
||
| { | ||
| if !cfg.Debug.Disabled { |
There was a problem hiding this comment.
Hm, while we could make the debug endpoint optional, I would leave that out for now. If we want to change how we expose metrics, allow readyniess checks etc. we should track that in a separate issue. It would totally make sense to reduce the number of debug servers we run, especially because we run them in a single binary anyway. However we would need to look into metrics collection and how we can expose the metrics of all services and the readyness / healthyness in a common debug server.
For now, do not make this configurable.
| // this need to be discussed, I put it here for now because I belive we need to ensure that the service is doing | ||
| // at least something, if we have events and grpc disabled that means that the service is doing actually nothing | ||
| // which can be a siletlly ignorred and cause issues in the future | ||
| if cfg.Events.Disabled && cfg.GRPC.Disabled { |
| if err != nil { | ||
| return err | ||
| } | ||
| } else { |
There was a problem hiding this comment.
This only sets up the nats consumer. Unfortunately, the actual events.ConsumeAll(...) call still lives in the NewEventHistoryService. You will have te extract that into a separate module, like in https://github.com/opencloud-eu/opencloud/pull/3241/changes#diff-654db365a382a61398b04610d91188cc8b6d777398bd1b7a25793977e33231e0 ... although I still need to fix the failing tests it shows the different services that are started by the server command.
| ch, err := events.ConsumeAll(consumer, "evhistory") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| eh := &EventHistoryService{ch: ch, store: store, cfg: cfg, log: log} | ||
| go eh.StoreEvents() | ||
| eh.ch = ch | ||
| go eh.StoreEvents() |
There was a problem hiding this comment.
This consumer will never be started if grpc has been disabled
…enthistory_event_handler_on_demand
2 participants
for event history service now it's possible to enable/disable from config