-
Notifications
You must be signed in to change notification settings - Fork 36
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
Monitor Server should be passed to clients #87
Conversation
Monitor Server should be passed to clients to chain event recieved for connections. Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
pkg/test/monitor/test_monitor.go
Outdated
"runtime" | ||
"time" | ||
|
||
"github.com/networkservicemesh/sdk/pkg/tools/serialize" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why imports are moving around here? More interestingly... do we know why linting isn't catching it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, no idea.
pkg/test/monitor/server_test.go
Outdated
@@ -0,0 +1,194 @@ | |||
// Copyright (c) 2020 Doc.ai and/or its affiliates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just put this in the pkg/networkservice/common/monitor/ directory using package monitor_test ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to tests package inside monitor
event := &networkservice.ConnectionEvent{ | ||
Type: networkservice.ConnectionEventType_UPDATE, | ||
Connections: map[string]*networkservice.Connection{conn.GetId(): conn}, | ||
} | ||
m.send(ctx, event) | ||
if sendErr := m.send(ctx, event); sendErr != nil { | ||
logrus.Errorf("Error during sending event: %v", sendErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more sense to use trace.Log(ctx).Errorf here?
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
Ah… I was unclear Let me find an example: In Golang, you can put your foo_test.go file in the same directory with your code, and simply give it package foo_test, that way you are testing your code from outside its package, but you don't have to have a separate tests/ subdir |
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
}) | ||
} | ||
return conn, err | ||
} | ||
|
||
func (m *monitorServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { | ||
// Pass metrics monitor, so it could be used later in chain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What that comment related to?
} | ||
|
||
// BeginMonitoring - start monitoring inside go routine, use Cancel() to perform stop. | ||
func (t *TestMonitorClient) BeginMonitoring(server networkservice.MonitorConnectionServer, segmentName string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just roll this into the constructor?
Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
…i@master networkservicemesh/api# networkservicemesh/api PR link: https://github.com/networkservicemesh/api/pull/ networkservicemesh/api commit message: commit b0b334ca68727bf6a33074803e6831bf713a0ac3 Author: Denis Tingaikin <49399980+denis-tingaikin@users.noreply.github.com> Date: Sat Apr 3 23:56:46 2021 +0700 fix gogenerate job actually check nothing (#87) Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Monitor Server should be passed to clients to chain event recieved for connections.
Signed-off-by: Andrey Sobolev haiodo@gmail.com