Skip to content

Commit

Permalink
Merge pull request #613 from nkryuchkov/fix/todo
Browse files Browse the repository at this point in the history
Fix some TODO's
  • Loading branch information
jdknives authored Nov 23, 2020
2 parents 1d37a97 + 764e217 commit e729863
Show file tree
Hide file tree
Showing 19 changed files with 227 additions and 74 deletions.
2 changes: 1 addition & 1 deletion cmd/setup-node/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func prepareMetrics(log logrus.FieldLogger) setupmetrics.Metrics {
r.Use(middleware.Logger)
r.Use(middleware.Recoverer)

// TODO: The following should be replaced by promutil.AddMetricsHandle
// TODO(evanlinjin): The following should be replaced by promutil.AddMetricsHandle
reg := prometheus.NewPedanticRegistry()
reg.MustRegister(prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{}))
reg.MustRegister(prometheus.NewGoCollector())
Expand Down
1 change: 0 additions & 1 deletion cmd/skywire-cli/commands/visor/transports.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ var (

func init() {
const (
// TODO: add stcpr implementation
typeFlagUsage = "type of transport to add; if unspecified, cli will attempt to establish a transport " +
"in the following order: stcp, stcpr, sudph, dmsg"
publicFlagUsage = "whether to make the transport public"
Expand Down
2 changes: 1 addition & 1 deletion cmd/skywire-visor/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ var rootCmd = &cobra.Command{
log.WithError(err).Infof("Restarted skywire-visor service")
}

// Detach child from parent. TODO: This may be unnecessary.
// Detach child from parent.
if _, err := syscall.Setsid(); err != nil {
log.WithError(err).Errorf("Failed to call setsid()")
}
Expand Down
1 change: 0 additions & 1 deletion pkg/restart/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func CaptureContext() *Context {

cmd := exec.Command(shellCommand, shellArgs...) // nolint:gosec

// TODO(nkryuchkov): use syscall.Dup to duplicate descriptors
cmd.Stdout = os.Stdout
cmd.Stdin = os.Stdin
cmd.Stderr = os.Stderr
Expand Down
2 changes: 1 addition & 1 deletion pkg/restart/restart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestContext_Start(t *testing.T) {
cmd := "bad_command"
cc.cmd = exec.Command(cmd) // nolint:gosec

// TODO(nkryuchkov): Add error text for Windows
// TODO: Add error text for Windows
possibleErrors := []string{
`exec: "bad_command": executable file not found in $PATH`,
}
Expand Down
32 changes: 16 additions & 16 deletions pkg/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func (r *router) saveRouteGroupRules(rules routing.EdgeRules, nsConf noise.Confi
r.logger.Errorf("Error closing already existing noise route group: %v", err)
}

r.logger.Infoln("Successfully closed old noise route group")
r.logger.Debugf("Successfully closed old noise route group")
}

if rg.encrypt {
Expand Down Expand Up @@ -513,15 +513,15 @@ func (r *router) handleDataHandshakePacket(ctx context.Context, packet routing.P
desc := rule.RouteDescriptor()
nrg, ok := r.noiseRouteGroup(desc)

r.logger.Infof("Handling packet with descriptor %s", &desc)
r.logger.Debugf("Handling packet with descriptor %s", &desc)

if ok {
if nrg == nil {
return errors.New("noiseRouteGroup is nil")
}

// in this case we have already initialized nrg and may use it straightforward
r.logger.Infof("Got new remote packet with size %d and route ID %d. Using rule: %s",
r.logger.Debugf("Got new remote packet with size %d and route ID %d. Using rule: %s",
len(packet.Payload()), packet.RouteID(), rule)

return nrg.handlePacket(packet)
Expand All @@ -542,7 +542,7 @@ func (r *router) handleDataHandshakePacket(ctx context.Context, packet routing.P
}

// handshake packet, handling with the raw rg
r.logger.Infof("Got new remote packet with size %d and route ID %d. Using rule: %s",
r.logger.Debugf("Got new remote packet with size %d and route ID %d. Using rule: %s",
len(packet.Payload()), packet.RouteID(), rule)

return rg.handlePacket(packet)
Expand All @@ -551,7 +551,7 @@ func (r *router) handleDataHandshakePacket(ctx context.Context, packet routing.P
func (r *router) handleClosePacket(ctx context.Context, packet routing.Packet) error {
routeID := packet.RouteID()

r.logger.Infof("Received close packet for route ID %v", routeID)
r.logger.Debugf("Received close packet for route ID %v", routeID)

rule, err := r.GetRule(routeID)
if err != nil {
Expand All @@ -571,14 +571,14 @@ func (r *router) handleClosePacket(ctx context.Context, packet routing.Packet) e
}()

if t := rule.Type(); t == routing.RuleIntermediary {
r.logger.Infoln("Handling intermediary close packet")
r.logger.Debugln("Handling intermediary close packet")
return r.forwardPacket(ctx, packet, rule)
}

desc := rule.RouteDescriptor()
nrg, ok := r.noiseRouteGroup(desc)

r.logger.Infof("Handling close packet with descriptor %s", &desc)
r.logger.Debugf("Handling close packet with descriptor %s", &desc)

if !ok {
r.logger.Infof("Descriptor not found for rule with type %s, descriptor: %s", rule.Type(), &desc)
Expand All @@ -591,7 +591,7 @@ func (r *router) handleClosePacket(ctx context.Context, packet routing.Packet) e
return errors.New("noiseRouteGroup is nil")
}

r.logger.Infof("Got new remote close packet with size %d and route ID %d. Using rule: %s",
r.logger.Debugf("Got new remote close packet with size %d and route ID %d. Using rule: %s",
len(packet.Payload()), packet.RouteID(), rule)

closeCode := routing.CloseCode(packet.Payload()[0])
Expand Down Expand Up @@ -625,15 +625,15 @@ func (r *router) handleNetworkProbePacket(ctx context.Context, packet routing.Pa
desc := rule.RouteDescriptor()
nrg, ok := r.noiseRouteGroup(desc)

r.logger.Infof("Handling packet with descriptor %s", &desc)
r.logger.Debugf("Handling packet with descriptor %s", &desc)

if ok {
if nrg == nil {
return errors.New("noiseRouteGroup is nil")
}

// in this case we have already initialized nrg and may use it straightforward
r.logger.Infof("Got new remote packet with size %d and route ID %d. Using rule: %s",
r.logger.Debugf("Got new remote packet with size %d and route ID %d. Using rule: %s",
len(packet.Payload()), packet.RouteID(), rule)

return nrg.handlePacket(packet)
Expand All @@ -654,7 +654,7 @@ func (r *router) handleNetworkProbePacket(ctx context.Context, packet routing.Pa
}

// handshake packet, handling with the raw rg
r.logger.Infof("Got new remote packet with size %d and route ID %d. Using rule: %s",
r.logger.Debugf("Got new remote packet with size %d and route ID %d. Using rule: %s",
len(packet.Payload()), packet.RouteID(), rule)

return rg.handlePacket(packet)
Expand All @@ -663,7 +663,7 @@ func (r *router) handleNetworkProbePacket(ctx context.Context, packet routing.Pa
func (r *router) handleKeepAlivePacket(ctx context.Context, packet routing.Packet) error {
routeID := packet.RouteID()

r.logger.Infof("Received keepalive packet for route ID %v", routeID)
r.logger.Debugf("Received keepalive packet for route ID %v", routeID)

rule, err := r.GetRule(routeID)
if err != nil {
Expand All @@ -680,11 +680,11 @@ func (r *router) handleKeepAlivePacket(ctx context.Context, packet routing.Packe
// propagate packet only for intermediary rule. forward rule workflow doesn't get here,
// consume rules should be omitted, activity is already updated
if t := rule.Type(); t == routing.RuleIntermediary {
r.logger.Infoln("Handling intermediary keep-alive packet")
r.logger.Debugln("Handling intermediary keep-alive packet")
return r.forwardPacket(ctx, packet, rule)
}

r.logger.Infof("Route ID %v found, updated activity", routeID)
r.logger.Debugf("Route ID %v found, updated activity", routeID)

return nil
}
Expand Down Expand Up @@ -788,7 +788,7 @@ func (r *router) forwardPacket(ctx context.Context, packet routing.Packet, rule
r.logger.Errorf("Failed to update activity for rule with route ID %d: %v", rule.KeyRouteID(), err)
}

r.logger.Infof("Forwarded packet via Transport %s using rule %d", rule.NextTransportID(), rule.KeyRouteID())
r.logger.Debugf("Forwarded packet via Transport %s using rule %d", rule.NextTransportID(), rule.KeyRouteID())

return nil
}
Expand All @@ -810,7 +810,7 @@ func (r *router) RemoveRouteDescriptor(desc routing.RouteDescriptor) {
}

func (r *router) fetchBestRoutes(src, dst cipher.PubKey, opts *DialOptions) (fwd, rev []routing.Hop, err error) {
// TODO(nkryuchkov): use opts
// TODO: use opts
if opts == nil {
opts = DefaultDialOptions() // nolint
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/routing/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,5 @@ func (p Packet) RouteID() RouteID {

// Payload returns payload from a Packet.
func (p Packet) Payload() []byte {
return p[PacketPayloadOffset:] // TODO: consider checking if real payload size differs
return p[PacketPayloadOffset:]
}
2 changes: 1 addition & 1 deletion pkg/setup/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func biRouteFromKeys(fwdPKs, revPKs []cipher.PubKey, srcPort, dstPort routing.Po
revHops[i] = routing.Hop{TpID: determineTpID(srcPK, dstPK), From: srcPK, To: dstPK}
}

// TODO: This should also return a map of format: map[uuid.UUID][]cipher.PubKey
// TODO(evanlinjin): This should also return a map of format: map[uuid.UUID][]cipher.PubKey
// This way, we can associate transport IDs to the two transport edges, allowing for more checks.
return routing.BidirectionalRoute{
Desc: routing.NewRouteDescriptor(fwdPKs[0], revPKs[0], srcPort, dstPort),
Expand Down
2 changes: 1 addition & 1 deletion pkg/transport/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Entry struct {

// Public determines whether the transport is to be exposed to other nodes or not.
// Public transports are to be registered in the Transport Discovery.
Public bool `json:"public"` // TODO: remove this.
Public bool `json:"public"` // TODO(evanlinjin): remove this.
}

// NewEntry constructs *Entry
Expand Down
2 changes: 1 addition & 1 deletion pkg/transport/managed_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ func (mt *ManagedTransport) readPacket() (packet routing.Packet, err error) {
log.WithField("type", packet.Type().String()).
WithField("rt_id", packet.RouteID()).
WithField("size", packet.Size()).
Info("Received packet.")
Debug("Received packet.")
return packet, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/transport/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (tm *Manager) serveNetwork(ctx context.Context, netType string) {
}

func (tm *Manager) serve(ctx context.Context) {
// TODO(nkryuchkov): to get rid of this callback, we need to have method on future network interface like: `Ready() <-chan struct{}`
// TODO: to get rid of this callback, we need to have method on future network interface like: `Ready() <-chan struct{}`
// some networks may not be ready yet, so we're setting a callback first
tm.n.OnNewNetworkType(func(netType string) {
tm.serveNetwork(ctx, netType)
Expand Down
Loading

0 comments on commit e729863

Please sign in to comment.