change restclient #123
Merged
change restclient #123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
typhoonzero
reviewed
Jun 2, 2017
go/paddlecloud/restclient.go
Outdated
| @@ -14,7 +14,15 @@ import ( | |||
| // HTTPOK is ok status of http api call | |||
| const HTTPOK = "200 OK" | |||
|
|
|||
| var httpTransport = &http.Transport{} | |||
| type RestClient struct { | |||
There was a problem hiding this comment.
What are the benefits of adding a struct? We can not re-use the connection to the server if we need to create a new RestClient every time.
gongweibao
commented
Jun 2, 2017
go/paddlecloud/restclient.go
Outdated
| func NewRestClient() *RestClient { | ||
| client := http.Client{Transport: &http.Transport{}} | ||
| return &RestClient{client: &client} | ||
| } | ||
|
|
||
| func makeRequest(uri string, method string, body io.Reader, | ||
| contentType string, query map[string]string, |
There was a problem hiding this comment.
之所以把query改成string，是因为map[string][]string才能表达，而这样的结构体用起来太复杂，不如让用户在外边自己encode
typhoonzero
reviewed
Jun 2, 2017
go/paddlecloud/restclient.go
Outdated
| const HTTPOK = "200 OK" | ||
|
|
||
| var httpTransport = &http.Transport{} | ||
| type RestClient struct { |
There was a problem hiding this comment.
A struct with only one member seems not necessary. If we want to implement multi-threading download processes, we can use a more simple function like:
func GetChunk(targetURL string, query url.Values, nThreads int) (*http.Response, error) {
req, err := makeRequest()
// use a thread pool of http.client to do multi-threading
}Then, we can call this functions by:
resp := restclient.GetChunk()instead of
client := NewRestClient()
resp := client.GetChunk()There was a problem hiding this comment.
httpclient的确是线程安全的。Done。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.