fix: cdn-source pattern supports range task #1259
Conversation
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
4127cbe to
8b0ad1a
Compare
dfget/core/api/download_api.go
Outdated
|
|
||
| return httputils.HTTPGetTimeout(url, headers, timeout) | ||
| } | ||
|
|
||
| func isFromSource(req *DownloadRequest) bool { | ||
| return req != nil && strings.Contains(req.Path, "://") |
There was a problem hiding this comment.
It is not enough to judge that req is not nil here.
| // pieceRange: "start-end" | ||
| // rangeHeaderValue: "bytes=sourceStart-sourceEnd" | ||
| // return: "bytes=realStart-realEnd" | ||
| func getRealRange(pieceRange string, rangeHeaderValue string) string { |
There was a problem hiding this comment.
How about adding a unit test for this function. 😄
|
Please add a description for this PR, such as:
|
dfget/core/api/download_api.go
Outdated
| return httputils.ConstructRangeStr(fmt.Sprintf("%d-%d", realStart, realEnd)) | ||
| } | ||
|
|
||
| func parseRange(rangeStr string) (int64, int64, error) { |
There was a problem hiding this comment.
https://github.com/dragonflyoss/Dragonfly/blob/master/supernode/util/range_util.go#L54
It's a repeated function that we extract it as common. WDYT?
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
8b0ad1a to
461f258
Compare
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
461f258 to
2e118bf
Compare
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
2e118bf to
7719400
Compare
Codecov Report
@@ Coverage Diff @@
## master #1259 +/- ##
==========================================
+ Coverage 48.51% 48.75% +0.24%
==========================================
Files 121 119 -2
Lines 7788 7763 -25
==========================================
+ Hits 3778 3785 +7
+ Misses 3698 3663 -35
- Partials 312 315 +3
Continue to review full report at Codecov.
|
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
7719400 to
e7b6c69
Compare
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
e7b6c69 to
96e66ff
Compare
|
@starnop PTAL |
| {"0-1", "", "0-1"}, | ||
| {"0-1", "1-100", "1-2"}, | ||
| {"0-100", "1-100", "1-100"}, | ||
| {"100-100", "1-100", "101-100"}, |
There was a problem hiding this comment.
Is end less than start as expected?
There was a problem hiding this comment.
Yes, this function just do compute the real range the user want to download but not do verify. The verification is done by the remote server.
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
96e66ff to
6673cc6
Compare
|
LGTM. |
|
@lowzj In additional, should we support range task for non-cdn-source pattern? |
Signed-off-by: lowzj <[email protected]>
It has already supported. |
lowzj
force-pushed
the
cdnPattern-support-range
branch
from
6673cc6 to
ef57720
Compare
👍 Merging...... |
…easer (dragonflyoss#1259) add goreleaser dfcache rpm/deb packages and man pages Note that dfcache*.md files are generated by `dfcache doc` command. Signed-off-by: Eryu Guan <[email protected]>
Signed-off-by: lowzj [email protected]
Ⅰ. Describe what this PR did
#1213 has support
cdn sourcepattern. The download behavior is different when user wants to download a part of the source file, just like:supernodeis not the real range of the source file, sodfgetshould correct it based on the Range header passed by user.supernodeis the range of file stored on the other peers, sodfgetcan directly use it.Ⅱ. 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