-
Notifications
You must be signed in to change notification settings - Fork 773
dfdaemon: specify context when invokes dfget #1041
Conversation
5d17a4a
to
2d4b579
Compare
Codecov Report
@@ Coverage Diff @@
## master #1041 +/- ##
==========================================
- Coverage 46.7% 46.69% -0.02%
==========================================
Files 115 115
Lines 7029 7029
==========================================
- Hits 3283 3282 -1
Misses 3490 3490
- Partials 256 257 +1
Continue to review full report at Codecov.
|
dfdaemon/downloader/downloader.go
Outdated
@@ -16,11 +16,16 @@ | |||
|
|||
package downloader | |||
|
|||
import "context" | |||
|
|||
// Interface specifies on how an plugin can download a file. | |||
type Interface interface { | |||
// Download download url file to file name | |||
// return dst path and download error | |||
Download(url string, header map[string][]string, name string) (string, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. I'm hesitate to rename back to Download
as it is not future-proof yet(I mean, I'm not certain that every implementation can be implemented with Context).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to rename back to Download
. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhouhaibing089 WDYT? And can we make it move on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to keep it as it is just like exec.Command
and exec.CommandContext
unless you are strong on it.
releated #1038 also cc/ @zcc35357949 |
This change is meant to fix dragonflyoss#997. The dfget process is supposed to be terminated when client closes the connection. Signed-off-by: zhouhaibing089 <[email protected]>
2d4b579
to
5444a8e
Compare
LGTM. |
dfdaemon: specify context when invokes dfget
dfdaemon: specify context when invokes dfget
Signed-off-by: Gaius <[email protected]>
This change is meant to fix #997. The dfget process is supposed to
be terminated when client closes the connection.