refactor: define api register of supernode #1346
Conversation
lowzj
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1346 +/- ##
==========================================
- Coverage 51.35% 51.35% -0.01%
==========================================
Files 130 130
Lines 8576 8578 +2
==========================================
+ Hits 4404 4405 +1
- Misses 3804 3806 +2
+ Partials 368 367 -1
Continue to review full report at Codecov.
|
supernode/server/api/api.go
Outdated
| registerAPI(LegacyCategory, h) | ||
| } | ||
|
|
||
| func registerAPI(c *Category, h *HandlerSpec) { |
There was a problem hiding this comment.
How about to allow register user-defined Category, may be we want to register more than one groups of API.
There was a problem hiding this comment.
I think about it again, it's better to provide an extension category with prefix /api/ext for users to register APIs. Users can group their own customized APIs in this extension category, not in Dragonfly core APIs category, like /api/ext/groupA, /api/ext/b.
WDYT?
There was a problem hiding this comment.
GET /api/extAnd supernode provides the above API to list the current registered extension APIs.
supernode/server/api/utils.go
Outdated
|
|
||
| // ParseJSONRequest parses the request parameter formed by JSON to a object. | ||
| func ParseJSONRequest(body io.Reader, request interface{}, validator ValidateFunc) error { | ||
| if err := json.NewDecoder(body).Decode(request); err != nil { |
There was a problem hiding this comment.
Add validation of "body == nil " and "request == nil" ?
There was a problem hiding this comment.
It should return 400 response in this case.
supernode/server/router.go
Outdated
| "github.com/gorilla/mux" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/client_golang/prometheus/promhttp" | ||
|
|
||
| "github.com/dragonflyoss/Dragonfly/apis/types" |
There was a problem hiding this comment.
The self-project import path may be better in second part.
lowzj
force-pushed
the
api-register
branch
2 times, most recently
from
214e875 to
552ded5
Compare
Codecov Report
@@ Coverage Diff @@
## master #1346 +/- ##
==========================================
+ Coverage 51.46% 51.80% +0.33%
==========================================
Files 133 135 +2
Lines 8744 8803 +59
==========================================
+ Hits 4500 4560 +60
Misses 3866 3866
+ Partials 378 377 -1
Continue to review full report at Codecov.
|
supernode/server/api/handler.go
Outdated
| "net/http" | ||
| ) | ||
|
|
||
| // HandlerSpec describes a HTTP api |
supernode/server/api/api.go
Outdated
|
|
||
| const ( | ||
| // VersionedPrefix is recommended, any new API should start with this prefix. | ||
| VersionedPrefix = "/api/v1" |
There was a problem hiding this comment.
s/VersionedPrefix/V1VersionedPrefix
Maybe we will add v2,v3 API version in the future. 😄
lowzj
force-pushed
the
api-register
branch
2 times, most recently
from
57b5cac to
72e1efc
Compare
lowzj
force-pushed
the
api-register
branch
5 times, most recently
from
b25637a to
c84f55b
Compare
Signed-off-by: lowzj <[email protected]>
|
LGTM. |
Signed-off-by: lowzj [email protected]
Ⅰ. Describe what this PR did
define and expose the api register of supernode to make it possible to add APIs outside.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews