Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some TODO's #613

Merged
merged 4 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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