-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kadai3-2: lfcd85 #33
base: master
Are you sure you want to change the base?
Kadai3-2: lfcd85 #33
Conversation
kadai3-2/lfcd85/Makefile
Outdated
PHONY: fmt | ||
fmt: | ||
go fmt ./... | ||
go vet ./... |
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.
実は最近はgo testで自動でかかる
kadai3-2/lfcd85/mypget/mypget.go
Outdated
} | ||
|
||
// Execute do the split download. | ||
func (d *Downloader) Execute() 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.
せっかくなので引数にcontext.Context
とって、nil
ならBackground
にしても良いかも。
kadai3-2/lfcd85/mypget/mypget.go
Outdated
ctx, cancel := context.WithCancel(bc) | ||
defer cancel() | ||
|
||
req, err := http.NewRequest("GET", d.url.String(), nil) |
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.
http.MethodGet
return err | ||
} | ||
|
||
resp, err := http.DefaultClient.Do(req) |
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.
HTTPクライアントを変更できるようにしておくとよいかも。
最近はあんまり必要ないかもしれないけど、GAEだと特殊なHTTPクライアントを使う必要があったりするので。
kadai3-2/lfcd85/mypget/mypget.go
Outdated
return err | ||
} | ||
|
||
err = d.combine(tempDir) |
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.
if err := ...
にしてスコープを小さくする
if err != nil { | ||
return err | ||
} | ||
fmt.Printf("Download completed! saved at: %v\n", d.outputPath) |
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.
出力先を変更できるようにする
kadai3-2/lfcd85/mypget/mypget.go
Outdated
} | ||
fmt.Printf("Download completed! saved at: %v\n", d.outputPath) | ||
|
||
return err |
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.
nil
を返す
kadai3-2/lfcd85/mypget/mypget.go
Outdated
} | ||
|
||
func (d *Downloader) downloadByRanges(ctx context.Context, tempDir string) error { | ||
var eg errgroup.Group |
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.
errgourp.WithContext
を呼ばないとキャンセルされない
return err | ||
} | ||
|
||
partialPath := generatePartialPath(tempDir, i) |
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.
MEMO
tempDirは読み込みだけなのでセーフ
|
||
for i, _ := range d.ranges { | ||
partialPath := generatePartialPath(tempDir, i) | ||
partial, err := os.Open(partialPath) |
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.
閉じてない
|
||
w.Header().Set("Content-Length", strconv.Itoa(len(body))) | ||
w.WriteHeader(http.StatusPartialContent) | ||
w.Write(body) |
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.
エラー処理
return ts, func() { ts.Close() } | ||
} | ||
|
||
func (tsf *testServerFile) testServerHandler(t *testing.T, w http.ResponseWriter, r *http.Request) { |
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.
せっかくなんで大きなファイルもテストできるようにするとよいかも。
課題3-2
分割ダウンローダを作ろう
の実装です。Specs
TODO
testdata
からテストできるようにしました: 0dd26ec(
hioki-daichi
さんのこちらを参考にしました)