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

bugfix: cdn cannot re-download uncompleted task with header param #1381

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

lowzj
Copy link
Member

@lowzj lowzj commented Jun 5, 2020

Signed-off-by: lowzj [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes: #1380

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/M labels Jun 5, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2020

Codecov Report

Merging #1381 into master will increase coverage by 0.06%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1381      +/-   ##
==========================================
+ Coverage   51.77%   51.83%   +0.06%     
==========================================
  Files         137      137              
  Lines        9078     9088      +10     
==========================================
+ Hits         4700     4711      +11     
+ Misses       3995     3993       -2     
- Partials      383      384       +1     
Impacted Files Coverage Δ
supernode/httpclient/origin_http_client.go 53.77% <42.85%> (+3.77%) ⬆️
supernode/daemon/mgr/cdn/downloader.go 84.00% <81.81%> (+5.05%) ⬆️
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 80eae52...e64a762. Read the comment docs.

@zhouhaibing089
Copy link
Contributor

zhouhaibing089 commented Jun 7, 2020

I do have a general question. Header is usually typed with map[string][]string.

And it looks like there is a builtin Clone in standard library.

@lowzj
Copy link
Member Author

lowzj commented Jun 7, 2020

I do have a general question. Header is usually typed with map[string][]string.

And it looks like there is a builtin Clone in standard library.

Yes, the arg headers will be parsed into http.Header in the supernode/httpclient. I think that it's better to change the content after parsing.

@lowzj
Copy link
Member Author

lowzj commented Jun 8, 2020

And it looks like there is a builtin Clone in standard library.

The Clone method is not public until go version 1.13.

@lowzj lowzj force-pushed the fix-downloader-header branch from 516d62f to 5a91f96 Compare June 8, 2020 04:03
@pouchrobot pouchrobot added size/L and removed size/M labels Jun 8, 2020
@zhouhaibing089
Copy link
Contributor

The Clone method is not public until go version 1.13.

Oh, good to know. If we're using go1.13 for compilation, then I think it is nice to align with it. Otherwise, I think we could just copy it over.

@lowzj lowzj force-pushed the fix-downloader-header branch from 5a91f96 to e64a762 Compare June 8, 2020 07:45
@starnop
Copy link
Contributor

starnop commented Jun 9, 2020

LGTM.

@starnop starnop merged commit 2f07b25 into dragonflyoss:master Jun 9, 2020
lowzj added a commit that referenced this pull request Jun 11, 2020
bugfix: cdn cannot re-download uncompleted task with header param

backport to 1.0.x
@lowzj lowzj deleted the fix-downloader-header branch June 11, 2020 08:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug This is bug report for project size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: supernode cannot re-download a uncompleted task which has header parameter
5 participants