feature: add golbal dfget params to the dfdaemon config #771

starnop · 2019-08-01T11:41:26Z

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes part of #687

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

TODO.

Ⅳ. Describe how to verify it

cat /etc/dragonfly/dfdaemon.yml
# node is the supernode address
# ip should be the ip address that the other nodes in the p2p network can access.
# port is the port that peer server will listen on
global_dfget_params: ["--node","192.168.33.21","--verbose","--ip","192.168.33.22","--port","15001","--alivetime","0s","--expiretime","5m0s"]
......

Ⅴ. Special notes for reviews

In my opition, I think it's not a good way to use CLI args to pass values to the dfget compared to the config file. So I have not added a flag for dfget params. @lowzj @yeya24 WDYT?

In this PR, I only add a globalDfgetParams through the dfdaemon config file firstly. And we can add a dfgetParam for every proxy later.

In addition, there is one more thing we need to pay attention to: how should we verify the correctness of the dfget params? Do you have any good ideas?

codecov-io · 2019-08-01T11:58:56Z

Codecov Report

Merging #771 into master will decrease coverage by 0.19%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master     #771     +/-   ##
=========================================
- Coverage   39.41%   39.21%   -0.2%     
=========================================
  Files         108      108             
  Lines        6310     6309      -1     
=========================================
- Hits         2487     2474     -13     
- Misses       3618     3630     +12     
  Partials      205      205

Impacted Files	Coverage Δ
cmd/dfdaemon/app/root.go	`63.63% <ø> (-1.37%)`	⬇️
dfdaemon/config/config.go	`90.24% <0%> (-9.76%)`	⬇️

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 68ba7b4...1599fa2. Read the comment docs.

yeya24 · 2019-08-01T12:21:57Z

dfdaemon/config/config.go

 	Verbose bool `yaml:"verbose" json:"verbose"`

 	MaxProcs int `yaml:"maxprocs" json:"maxprocs"`
+
+	// dfget config
+	GlobalDfgetParams []string `yaml:"global_dfget_params" json:"global_dfget_params"`


I don't think this should be a slice. Can we define a struct for this such as DfgetParams? Then we can reuse it in proxy. And we can do some validation using this struct.

If that, we have to redefine a struct just like dfget config.

just use dfgetflags? And it should be assigned a default value.

yeya24

Can you provide an example configuration? Since we remove the flags, I think it is better to add a configuration with some default global dfget params.

starnop · 2019-08-02T12:42:17Z

Can you provide an example configuration? Since we remove the flags, I think it is better to add a configuration with some default global dfget params.

I have added the corresponding comment in the code. In addition, we need to add a docs dir which including all docs about configuration files. And we should make it ASAP.

starnop · 2019-08-02T12:42:31Z

@lowzj @yeya24 PTAL. THX

yeya24

LGTM

lowzj · 2019-08-02T18:03:42Z

dfdaemon/config/config.go

 	Verbose bool `yaml:"verbose" json:"verbose"`

 	MaxProcs int `yaml:"maxprocs" json:"maxprocs"`
+
+	// dfget config
+	GlobalDfgetParams []string `yaml:"global_dfget_params" json:"global_dfget_params"`


just use dfgetflags? And it should be assigned a default value.

lowzj · 2019-08-02T18:07:55Z

dfdaemon/downloader/dfget/dfget.go

-
-	if dfGetter.config.Verbose {
-		args = append(args, "--verbose")
+	for _, param := range dfGetter.config.GlobalDfgetParams {


use args = append(args, dfGetter.config.GlobalDfgetParams...)

lowzj · 2019-08-02T18:09:55Z

dfdaemon/downloader/dfget/dfget.go

-		args = append(args, "--notbs")
-	}
-
-	if dfGetter.config.Verbose {


This should not be deleted if the --verbose flag of dfdaemon has been set.

Done. PTAL again. THX

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

lowzj

LGTM

feature: add golbal dfget params to the dfdaemon config

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

pouchrobot added kind/feature size/M labels Aug 1, 2019

starnop force-pushed the bug-ip branch from 2ab5cf2 to f8498e3 Compare August 1, 2019 11:55

pouchrobot added size/L and removed size/M labels Aug 1, 2019

yeya24 reviewed Aug 1, 2019

View reviewed changes

starnop force-pushed the bug-ip branch from f8498e3 to e1f0495 Compare August 1, 2019 13:20

pouchrobot added size/M and removed size/L labels Aug 1, 2019

starnop changed the title ~~[WIP]feature: add golbal dfget params to the dfdaemon config~~ feature: add golbal dfget params to the dfdaemon config Aug 1, 2019

yeya24 reviewed Aug 2, 2019

View reviewed changes

pouchrobot added the conflict/needs-rebase label Aug 2, 2019

starnop force-pushed the bug-ip branch from e1f0495 to 1f195da Compare August 2, 2019 12:32

pouchrobot removed the conflict/needs-rebase label Aug 2, 2019

starnop force-pushed the bug-ip branch 3 times, most recently from 11227bf to 7f14569 Compare August 2, 2019 12:40

yeya24 approved these changes Aug 2, 2019

View reviewed changes

lowzj reviewed Aug 2, 2019

View reviewed changes

starnop force-pushed the bug-ip branch 3 times, most recently from 4d0eb89 to a84c2a7 Compare August 5, 2019 02:08

feature: add golbal dfget params in the dfdaemon config

1599fa2

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

starnop force-pushed the bug-ip branch from a84c2a7 to 1599fa2 Compare August 5, 2019 03:31

lowzj reviewed Aug 5, 2019

View reviewed changes

lowzj merged commit 24aa469 into dragonflyoss:master Aug 5, 2019

pouchrobot mentioned this pull request Aug 9, 2019

WeeklyReport of Dragonfly from 2019-08-02 to 2019-08-09 #799

Closed

starnop pushed a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019

Merge pull request dragonflyoss#771 from Starnop/bug-ip

3fe62b2

feature: add golbal dfget params to the dfdaemon config

inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019

Merge pull request dragonflyoss#771 from Starnop/bug-ip

8167f6a

feature: add golbal dfget params to the dfdaemon config

sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022

feat: add user list interface (dragonflyoss#771)

4aa395a

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add golbal dfget params to the dfdaemon config #771

feature: add golbal dfget params to the dfdaemon config #771

starnop commented Aug 1, 2019 •

edited

Loading

codecov-io commented Aug 1, 2019 •

edited

Loading

yeya24 Aug 1, 2019

starnop Aug 1, 2019

yeya24 Aug 1, 2019

lowzj Aug 2, 2019

yeya24 left a comment

starnop commented Aug 2, 2019

starnop commented Aug 2, 2019

yeya24 left a comment

lowzj Aug 2, 2019

lowzj Aug 2, 2019

lowzj Aug 2, 2019 •

edited

Loading

starnop Aug 3, 2019

lowzj left a comment

feature: add golbal dfget params to the dfdaemon config #771

feature: add golbal dfget params to the dfdaemon config #771

Conversation

starnop commented Aug 1, 2019 • 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 Aug 1, 2019 • edited Loading

Codecov Report

yeya24 Aug 1, 2019

Choose a reason for hiding this comment

starnop Aug 1, 2019

Choose a reason for hiding this comment

yeya24 Aug 1, 2019

Choose a reason for hiding this comment

lowzj Aug 2, 2019

Choose a reason for hiding this comment

yeya24 left a comment

Choose a reason for hiding this comment

starnop commented Aug 2, 2019

starnop commented Aug 2, 2019

yeya24 left a comment

Choose a reason for hiding this comment

lowzj Aug 2, 2019

Choose a reason for hiding this comment

lowzj Aug 2, 2019

Choose a reason for hiding this comment

lowzj Aug 2, 2019 • edited Loading

Choose a reason for hiding this comment

starnop Aug 3, 2019

Choose a reason for hiding this comment

lowzj left a comment

Choose a reason for hiding this comment

starnop commented Aug 1, 2019 •

edited

Loading

codecov-io commented Aug 1, 2019 •

edited

Loading

lowzj Aug 2, 2019 •

edited

Loading