-
Notifications
You must be signed in to change notification settings - Fork 20
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
add client version message in connect response for gateway #82
Conversation
ccore/nebula/gateway/dao/dao.go
Outdated
@@ -276,10 +276,10 @@ func getMapInfo(valWarp *wrapper.ValueWrapper, _verticesParsedList *list, _edges | |||
} | |||
|
|||
// Connect return if the nebula connect succeed | |||
func Connect(address string, port int, username string, password string, version nebula.Version) (nsid string, err error) { | |||
nsid, err = pool.NewClient(address, port, username, password, version) | |||
func Connect(address string, port int, username string, password string, version nebula.Version) (nsid string, ver nebula.Version, err error) { |
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.
How about just return nebula.GraphClient, error
?
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.
Yes, ok.
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.
How about just return
nebula.GraphClient, error
?
The Studio need the returned nsid to identify a connect session. So If return GraphClient
interface may cause some problems.
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.
How about return nsid, nebula.GraphClient, error
?
Or define an interface to return?
This is convenient for expansion, and if more information is needed in the future, more and more return values will be returned.
controllers/db.go
Outdated
@@ -59,7 +59,7 @@ func (this *DatabaseController) Connect() { | |||
this.Ctx.SetCookie("SameSite", "Strict") | |||
this.SetSession(beego.AppConfig.String("sessionkey"), nsid) | |||
|
|||
res.Message = "Login successfully" | |||
res.Message = string(ver) |
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.
The business code does not provide new features, and the follow-up is only maintenance.
Let studio do this?
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.
ok.
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.
The business code does not provide new features, and the follow-up is only maintenance. Let studio do this?
But the Studio don't konw which nebula client version it connected.
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.
The studio
is use golang server now, the api will no longer use the controller here.
You need to rebase to resolve the conflict. |
Ok. |
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.
LGTM
No description provided.