Feat/add implementation for dfget interface #1352
Conversation
wangforthinker
force-pushed
the
feat/add-implementation-for-dfget-interface
branch
2 times, most recently
from
ae0cf29 to
ed738a8
Compare
Codecov Report
@@ Coverage Diff @@
## master #1352 +/- ##
==========================================
- Coverage 51.34% 51.21% -0.14%
==========================================
Files 130 133 +3
Lines 8578 8717 +139
==========================================
+ Hits 4404 4464 +60
- Misses 3806 3875 +69
- Partials 368 378 +10
Continue to review full report at Codecov.
|
pkg/protocol/http/http.go
Outdated
|
|
||
| const ( | ||
| // http protocol name | ||
| ProtocolName = "http" |
There was a problem hiding this comment.
Should it contain http and https?
There was a problem hiding this comment.
Add htttps support.
| } | ||
|
|
||
| // Client is an implementation of protocol.Client for http protocol. | ||
| type Client struct { |
There was a problem hiding this comment.
create an anonymous instant to check whether the struct implements the interface, like:
var _ protocol.Client = &Client{}
pkg/protocol/http/http.go
Outdated
| type ClientBuilder struct{} | ||
|
|
||
| func (cb *ClientBuilder) NewProtocolClient(clientOpt interface{}) (protocol.Client, error) { | ||
| opt, ok := clientOpt.(*ClientOpt) |
There was a problem hiding this comment.
Allow nil clientOpt.
pkg/protocol/http/http.go
Outdated
| } | ||
|
|
||
| if hd == nil { | ||
| hd = NewHTTPMetaData().(*Headers) |
There was a problem hiding this comment.
Why must create it? I think, it can be nil.
pkg/protocol/http/resource.go
Outdated
|
|
||
| defer res.Body.Close() | ||
| lenStr := res.Header.Get(config.StrContentLength) | ||
| if len(lenStr) == 0 { |
There was a problem hiding this comment.
use lenStr == "" to instead of this?
wangforthinker
force-pushed
the
feat/add-implementation-for-dfget-interface
branch
2 times, most recently
from
b35a1d2 to
7fc0988
Compare
pkg/protocol/http/http.go
Outdated
|
|
||
| // ClientOpt is the argument of NewProtocolClient. | ||
| type ClientOpt struct { | ||
| transport http.RoundTripper |
There was a problem hiding this comment.
change to map[string]interface, tell the user the keys which could be set.
wangforthinker
force-pushed
the
feat/add-implementation-for-dfget-interface
branch
from
7fc0988 to
2086f69
Compare
Signed-off-by: allen.wq <[email protected]>
wangforthinker
force-pushed
the
feat/add-implementation-for-dfget-interface
branch
from
2086f69 to
3cd96a0
Compare
4 participants
Ⅰ. Describe what this PR did
Add implementation of http protocol of dfget interface.
Ⅱ. Does this pull request fix one issue?
NONE.
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
add ut.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews