refactor: refactor dfget/core with SupernodeLocator #1325

lowzj · 2020-05-06T13:25:08Z

Ⅰ. Describe what this PR did

This pull request uses interface SupernodeLocator to refactor the dfget/core to get supernode list instead of configurations.
It also provides a way for users to customized their own SupernodeLocator to replace the default implementation, for example:

package main

import (
    "github.com/dragonflyoss/Dragonfly/cmd/dfget/app"
    "github.com/dragonflyoss/Dragonfly/dfget/config"
    "github.com/dragonflyoss/Dragonfly/dfget/locator"
)

func init() {
    // use customized implementation to replace default one
    // execute it in the `init` function
    // or move it into `main` function, before `app.Execute()`
    locator.RegisterLocator(func(cfg *config.Config) locator.SupernodeLocator {
        return NewDynamicLocator(cfg)
    })
}

// customized implementation of SupernodeLocator

func NewDynamicLocator(cfg *config.Config) *DynamicLocator {
    return &DynamicLocator{}
}

var _ locator.SupernodeLocator = &DynamicLocator{}

type DynamicLocator struct {
}
// implementation the interface SupernodeLocator here

func main() {
    app.Execute()
}

Ⅱ. Does this pull request fix one issue?

fixes: #1293

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io · 2020-05-07T09:47:53Z

Codecov Report

Merging #1325 into master will decrease coverage by 0.46%.
The diff coverage is 53.26%.

@@            Coverage Diff             @@
##           master    #1325      +/-   ##
==========================================
- Coverage   51.32%   50.86%   -0.47%     
==========================================
  Files         126      127       +1     
  Lines        8327     8346      +19     
==========================================
- Hits         4274     4245      -29     
- Misses       3709     3755      +46     
- Partials      344      346       +2

Impacted Files	Coverage Δ
dfget/config/config.go	`89.28% <ø> (-0.13%)`	⬇️
dfget/locator/manager.go	`0.00% <0.00%> (ø)`
dfget/locator/static_locator.go	`70.42% <35.71%> (-10.23%)`	⬇️
dfget/core/core.go	`44.32% <39.28%> (-0.89%)`	⬇️
dfget/core/regist/register.go	`82.82% <88.57%> (-5.25%)`	⬇️
dfget/locator/locator.go	`100.00% <100.00%> (ø)`
dfget/config/supernode_value.go	`57.54% <0.00%> (-15.10%)`	⬇️
supernode/daemon/mgr/scheduler/manager.go	`21.91% <0.00%> (-0.69%)`	⬇️

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 164e390...d00c4cb. Read the comment docs.

starnop · 2020-05-07T15:30:55Z

dfget/core/core.go

-		register     = regist.NewSupernodeRegister(cfg, supernodeAPI)
-		err          error
-		result       *regist.RegisterResult
+		// TODO make it pluggable


s/TODO/TODO:

starnop · 2020-05-07T15:33:30Z

dfget/core/core.go

-		if resp != nil && resp.IsSuccess() {
-			return
+	node := locator.Get()
+	if node != nil {


return quickly when node equals nil

starnop · 2020-05-07T15:35:54Z

dfget/core/regist/register.go

-	nLen := len(s.cfg.Nodes)
-	if nLen <= 0 {
-		return
+func (s *supernodeRegister) nodeHostStr(node *locator.Supernode) string {


use locator.Supernode.String() function?

the parameter node maybe nil, it will panic

I do not think nodeHostStr is a function of type supernodeRegister .

Except this, LGTM.

lowzj · 2020-05-08T00:09:02Z

@starnop PTAL

starnop · 2020-05-08T16:10:52Z

dfget/core/regist/register.go

@@ -188,28 +179,26 @@ func getTaskPath(taskFileName string) string {
 }

 // NewRegisterResult creates an instance of RegisterResult.
-func NewRegisterResult(node string, remainder []string, url string,
+func NewRegisterResult(node string, url string,


node, url, taskID string

starnop · 2020-05-08T16:26:14Z

dfget/core/regist/register.go

-	nLen := len(s.cfg.Nodes)
-	if nLen <= 0 {
-		return
+func (s *supernodeRegister) nodeHostStr(node *locator.Supernode) string {


Except this, LGTM.

lowzj · 2020-05-09T07:52:53Z

@starnop @allencloud done, PTAL

Signed-off-by: lowzj <[email protected]>

starnop · 2020-05-13T16:26:21Z

LGTM.

lowzj force-pushed the refactor-dfget-core-with-locator branch from 1bdfa6b to 1d5d7b6 Compare May 6, 2020 13:25

pouchrobot added the size/XL label May 6, 2020

lowzj changed the title ~~refactor: refactor dfget/core with SupernodeLocator~~ [WIP] refactor: refactor dfget/core with SupernodeLocator May 6, 2020

pouchrobot added the kind/refactor label May 6, 2020

lowzj force-pushed the refactor-dfget-core-with-locator branch from 1d5d7b6 to e0e8bb5 Compare May 7, 2020 01:02

pouchrobot added size/L and removed size/XL labels May 7, 2020

lowzj force-pushed the refactor-dfget-core-with-locator branch from e0e8bb5 to f7ae939 Compare May 7, 2020 09:43

pouchrobot added size/XL and removed size/L labels May 7, 2020

lowzj force-pushed the refactor-dfget-core-with-locator branch 3 times, most recently from c2e4c91 to 68ef2c5 Compare May 7, 2020 14:10

lowzj changed the title ~~[WIP] refactor: refactor dfget/core with SupernodeLocator~~ refactor: refactor dfget/core with SupernodeLocator May 7, 2020

starnop reviewed May 7, 2020

View reviewed changes

lowzj force-pushed the refactor-dfget-core-with-locator branch from 68ef2c5 to 2787e16 Compare May 7, 2020 15:56

lowzj force-pushed the refactor-dfget-core-with-locator branch 2 times, most recently from 66a7c56 to 5d5733b Compare May 8, 2020 08:51

starnop reviewed May 8, 2020

View reviewed changes

lowzj force-pushed the refactor-dfget-core-with-locator branch 2 times, most recently from 97b1ddd to 25d7443 Compare May 9, 2020 06:47

lowzj force-pushed the refactor-dfget-core-with-locator branch 4 times, most recently from ceb881e to d00c4cb Compare May 13, 2020 06:41

refactor: refactor dfget/core with SupernodeLocator

122ac3c

Signed-off-by: lowzj <[email protected]>

lowzj force-pushed the refactor-dfget-core-with-locator branch from d00c4cb to 122ac3c Compare May 13, 2020 11:14

starnop merged commit 2443cba into dragonflyoss:master May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: refactor dfget/core with SupernodeLocator #1325

refactor: refactor dfget/core with SupernodeLocator #1325

lowzj commented May 6, 2020 •

edited

Loading

codecov-io commented May 7, 2020 •

edited

Loading

starnop May 7, 2020

starnop May 7, 2020

starnop May 7, 2020

lowzj May 7, 2020

allencloud May 8, 2020

starnop May 8, 2020

lowzj commented May 8, 2020

starnop May 8, 2020

starnop May 8, 2020

lowzj commented May 9, 2020

starnop commented May 13, 2020

refactor: refactor dfget/core with SupernodeLocator #1325

refactor: refactor dfget/core with SupernodeLocator #1325

Conversation

lowzj commented May 6, 2020 • edited Loading

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

codecov-io commented May 7, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowzj commented May 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowzj commented May 9, 2020

starnop commented May 13, 2020

lowzj commented May 6, 2020 •

edited

Loading

codecov-io commented May 7, 2020 •

edited

Loading