chore(dot): transform telemetry into an injectable interface
#2113
Conversation
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
…ssamer into eclesio/net-race-conditions
dot/peerset/peerset.go
Outdated
| @@ -666,15 +666,14 @@ func (ps *PeerSet) disconnect(setIdx int, reason DropReason, peers ...peer.ID) e | |||
| } | |||
|
|
|||
| // start handles all the action for the peerSet. | |||
| func (ps *PeerSet) start(aq chan action, stopCh chan struct{}) { | |||
| func (ps *PeerSet) start(ctx context.Context, aq chan action) { | |||
There was a problem hiding this comment.
Rename aq to actionQueue please; also maybe restrict to a read only / write only channel if you can
There was a problem hiding this comment.
the PeerSet.actionQueue already is a <-chan action (read only channel)
There was a problem hiding this comment.
So you could restrict the argument to be actionQueue chan<- action too 😉
dot/telemetry/mailer.go
Outdated
| } | ||
| } | ||
|
|
||
| func msgToJSON(message Message) ([]byte, error) { |
There was a problem hiding this comment.
Add a MarshalJSON() method to the Message interface and implement them for all message types. You wouldn't have to do a marshal, then an unmarshal into a map[string]interface{}, and then another marshal.
There was a problem hiding this comment.
@timwu20 implemented!
dot/telemetry/telemetry.go
Outdated
| @@ -31,4 +31,5 @@ type Client interface { | |||
| // Message interface for Message functions | |||
| type Message interface { | |||
| messageType() string | |||
There was a problem hiding this comment.
Does this need to be private still?
There was a problem hiding this comment.
no, changed to Type()
Changes
EXPECTstelemetry.Telemetryinterfacetelemetry.Telemetryinterface as argumentdot.createStateServiceindot.dot.createStateServiceanddot.startStateService. This is necessary because thestate.Start()function needs the telemetry and the telemetry needs the state initialization of the database, so I decoupled the database initialization from the state service start function.Tests
Issues
t.Parallelto all tests #2105t.Parallel()and remove race conditions #2111Primary Reviewer