Add execute response wrappers and fit db http apis with ccore #69
Conversation
ccore/nebula/errors/errors.go
Outdated
| ErrUnsupportedVersion = errors.New("unsupported version") | ||
| ErrUnsupported = errors.New("unsupported") | ||
| ErrNoEndpoints = errors.New("no endpoints") | ||
| ErrVersionEstimateFailed = errors.New("version infer failed") |
There was a problem hiding this comment.
Use ErrUnsupportedVersion? And why use the different words Estimate and infer ?
There was a problem hiding this comment.
Ok, I will use infer to replace the estimate
ccore/nebula/helper.go
Outdated
| V2_5, | ||
| V2_6, | ||
| V3_0, |
There was a problem hiding this comment.
Give priority to the higher version?
There was a problem hiding this comment.
You need to verify it after connected.
ref: https://github.com/vesoft-inc/nebula-go/blob/40030d441885c03c34054391e841e8f59443e0af/connection.go#L75
controllers/db.go
Outdated
| "encoding/json" | ||
| "github.com/astaxie/beego/logs" | ||
| "github.com/vesoft-inc/nebula-http-gateway/ccore/nebula/types" | ||
| "github.com/vesoft-inc/nebula-http-gateway/ccore/service/pool" | ||
|
|
||
| "github.com/astaxie/beego" | ||
| "github.com/vesoft-inc/nebula-http-gateway/ccore/nebula" | ||
| "github.com/vesoft-inc/nebula-http-gateway/ccore/service/dao" | ||
| "github.com/vesoft-inc/nebula-http-gateway/common" |
There was a problem hiding this comment.
import group, std, vesoft then others.
controllers/db.go
Outdated
| nsid, err := dao.Connect(params.Address, params.Port, params.Username, params.Password) | ||
|
|
||
| if params.Version == "" { | ||
| version, err := nebula.VersionHelper(params.Address, params.Port, params.Username, params.Password) |
There was a problem hiding this comment.
How about auto infer version into nebula pkg?
ccore/service/dao/dao.go
Outdated
| } | ||
|
|
||
| func Execute(nsid string, gql string) (result ExecuteResult, err error) { | ||
| result = ExecuteResult{ | ||
| func Execute(nsid string, gql string) (ExecuteResult, interface{}, error) { |
There was a problem hiding this comment.
Why return a new interface{} value? And the interface{} is unclear, the caller is confused about this.
There was a problem hiding this comment.
the returned interface{} is the runtime panic error. I will add code commit later.
ccore/service/pool/pool.go
Outdated
| @@ -0,0 +1,149 @@ | |||
| package pool | |||
There was a problem hiding this comment.
move service to ccore/nebula/gataway?
I think it's temporary, and will add Manager to manager client in ccore/nebula pkg.
ccore
nebula
gateway
pool
dao
ccore/nebula/client_graph.go
Outdated
| resp, err := c.graph.VerifyClientVersion() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to verify client version: %s", err.Error()) | ||
| } | ||
| if resp != nil && resp.ErrorCode != nerrors.ErrorCode_SUCCEEDED { | ||
| return fmt.Errorf("incompatible version between client and server: %s", string(resp.ErrorMsg)) | ||
| } |
There was a problem hiding this comment.
return ErrVersionConflict if version conflicts, nor return the raw err?
There was a problem hiding this comment.
And auto check the next version only if err is ErrVersionConflict.
ccore/nebula/errors/errors.go
Outdated
| @@ -6,4 +6,5 @@ var ( | |||
| ErrUnsupportedVersion = errors.New("unsupported version") | |||
| ErrUnsupported = errors.New("unsupported") | |||
| ErrNoEndpoints = errors.New("no endpoints") | |||
| ErrMethodNotSupported = errors.New("method not supported") | |||
ccore/nebula/examples/main.go
Outdated
| //nebula.Version2_5, | ||
| //nebula.Version2_6, | ||
| //nebula.Version3_0, | ||
| nebula.VersionAuto, |
There was a problem hiding this comment.
Range multi connection info?
ccore/nebula/examples/main.go
Outdated
| "github.com/vesoft-inc/nebula-http-gateway/ccore/nebula/types" | ||
| "github.com/vesoft-inc/nebula-http-gateway/ccore/nebula/wrapper" | ||
| "log" | ||
| "strings" |
There was a problem hiding this comment.
sort import group ? std, vesoft and then others.
ccore/nebula/factory.go
Outdated
| //NewNSetBuilder() types.NSetBuilder | ||
| //NewGeographyBuilder() types.GeographyBuilder | ||
| //NewTagBuilder() types.TagBuilder | ||
| //NewStepBuilder() types.StepBuilder | ||
| //NewPointBuilder() types.PointBuilder | ||
| //NewLineStringBuilder() types.LineStringBuilder | ||
| //NewPolygonBuilder() types.PolygonBuilder | ||
| //NewCoordinateBuilder() types.CoordinateBuilder | ||
| //NewPlanDescriptionBuilder() types.PlanDescriptionBuilder | ||
| //NewPlanNodeDescriptionBuilder() types.PlanNodeDescriptionBuilder | ||
| //NewPairBuilder() types.PairBuilder | ||
| //NewProfilingStatsBuilder() types.ProfilingStatsBuilder | ||
| //NewPlanNodeBranchInfoBuilder() types.PlanNodeBranchInfoBuilder |
No description provided.