Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kadai3-2 shuheiktgw #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

kadai3-2 shuheiktgw #49

wants to merge 3 commits into from

Conversation

shuheiktgw
Copy link

@shuheiktgw shuheiktgw commented Nov 27, 2018 •
edited
Loading

遅くなりましたが, 課題3-2を書きました! 🙇

分割ダウンローダを作ろう

  • Rangeアクセスを用いる
  • いくつかのゴルーチンでダウンロードしてマージする
  • golang.org/x/sync/errgourpパッケージなどを使ってみる
  • キャンセルが発生した場合の実装を行う

@shuheiktgw
Copy link
Author

shuheiktgw commented Nov 27, 2018 •
edited
Loading

errgroup.WithContext のコメントに

// The derived Context is canceled the first time a function passed to Go
// returns a non-nil error or the first time Wait returns, whichever occurs
// first.

って書いてあるので WithContext 使えばエラーが各gorutineで返ってきた場合はキャンセルされるという認識なのですがあってるのかな...? キャンセルが発生した場合の実装を行う というのはそういう意味ではない?

@shuheiktgw
Copy link
Author

テスト書きたかったけど力付きました 😇

mizkei
mizkei reviewed Nov 30, 2018
View reviewed changes
Copy link

@mizkei mizkei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントしましたので、参考にしていただければと思います

kadai3-2/shuheiktgw/gget/request.go
ss := strings.Split(u.Path, "/")
fname := ss[len(ss)-1]

res, err := http.Head(rawURL)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.mozilla.org/ja/docs/Web/HTTP/Headers/Accept-Ranges
この時点で Accept-Ranges をチェックしておいたほうが良いと思います

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに,ありがとうございます!
RangeRequestとNonRangeRequestに分けて扱うようにしました.

kadai3-2/shuheiktgw/gget/request.go Outdated

// Do sends a real HTTP requests in parallel
func (r *Request) Do() error {
eg, _ := errgroup.WithContext(context.TODO())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://godoc.org/golang.org/x/sync/errgroup#WithContext

The derived Context is canceled the first time a function passed to Go returns a non-nil error or the first time Wait returns, whichever occurs first.

このcontext.Contextを受け取って、それを利用しないと意味がなくなってしまいます
この場合は http.Request の WithContext を利用して渡すなどの必要があります

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

完全に勘違いしてました... 😇
ありがとうございます!

@shuheiktgw
Copy link
Author

今更ですが指摘頂いた箇所を修正しました! 🙇

@suaaa7 suaaa7 mentioned this pull request Aug 31, 2019
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mizkei mizkei mizkei left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shuheiktgw @mizkei