Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Supernode support seed task pr #1313

Merged

Conversation

hhhhsdxxxx
Copy link

Ⅰ. Describe what this PR did

Ⅱ. 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

@hhhhsdxxxx hhhhsdxxxx force-pushed the supernode_support_seed_task_pr branch from 471d8ba to ed59371 Compare April 28, 2020 09:19
@hhhhsdxxxx hhhhsdxxxx force-pushed the supernode_support_seed_task_pr branch 8 times, most recently from 9bc80a4 to a930c92 Compare April 28, 2020 10:54
@lowzj
Copy link
Member

lowzj commented Apr 28, 2020

There are 6 commits in this pull request. Could you submit several pull requests to separate them?

@hhhhsdxxxx hhhhsdxxxx force-pushed the supernode_support_seed_task_pr branch from a930c92 to a4ee207 Compare April 29, 2020 03:51
@codecov-io
Copy link

codecov-io commented Apr 29, 2020

Codecov Report

Merging #1313 into master will decrease coverage by 0.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1313      +/-   ##
==========================================
- Coverage   51.24%   51.23%   -0.02%     
==========================================
  Files         124      124              
  Lines        8159     8165       +6     
==========================================
+ Hits         4181     4183       +2     
- Misses       3639     3643       +4     
  Partials      339      339              
Impacted Files Coverage Δ
supernode/server/0.3_bridge.go 0.00% <0.00%> (ø)
supernode/server/router.go 66.66% <100.00%> (+1.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4eda7c3...0398dfe. Read the comment docs.

@hhhhsdxxxx
Copy link
Author

There are 6 commits in this pull request. Could you submit several pull requests to separate them?

First PR is ready now

@@ -1511,6 +1511,27 @@ definitions:
This dfget is treated a client and carries a client ID.
Thus, multiple dfget processes on the same peer have different CIDs.

HeartBeatResponse:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the HeartBeatResponse be used in swagger?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API /peer/heartbeat.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I find this api, but its response definition is not HeartBeatResponse.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The definition in file api.md?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, i'll make it complete soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The api is defined in api/swagger.yml, you can search /peer/heartbeat in this file.

@hhhhsdxxxx hhhhsdxxxx force-pushed the supernode_support_seed_task_pr branch from a4ee207 to 0398dfe Compare April 30, 2020 05:53
Copy link
Member

@lowzj lowzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please merge two commits into one

@@ -48,6 +48,8 @@ func initRoute(s *Server) *mux.Router {
{Method: http.MethodGet, Path: "/peer/piece/suc", HandlerFunc: s.reportPiece},
{Method: http.MethodGet, Path: "/peer/service/down", HandlerFunc: s.reportServiceDown},
{Method: http.MethodGet, Path: "/peer/piece/error", HandlerFunc: s.reportPieceError},
{Method: http.MethodPost, Path: "/peer/network", HandlerFunc: s.getP2PNetworkInfo},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this POST method named getP2PNetworkInfo? Is it should be GET method?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix it

+ /peer/network
+ /peer/heartbeat

Signed-off-by: henry.hj <[email protected]>
@hhhhsdxxxx hhhhsdxxxx force-pushed the supernode_support_seed_task_pr branch from a2670f4 to 4525cb4 Compare May 6, 2020 06:06
@hhhhsdxxxx
Copy link
Author

@lowzj Are there any more comments?

Copy link
Member

@lowzj lowzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lowzj lowzj merged commit 849f36f into dragonflyoss:master May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants