-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix:POST request body is missing #260
Conversation
is this way better ? leave config empty for adding more config property for httpclient further |
@@ -51,15 +51,17 @@ type ( | |||
cfg *Config | |||
} | |||
// Config describe the config of Filter | |||
Config struct{} | |||
Config struct { | |||
client *http3.Client |
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.Client 给用户反序列化配置,用户只需要关注 transport 的内容
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.Client交给用户,http.Clinet 里的其它的配置也许用户需要用呢?
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.
transport 是一个 interface,直接反序列化 client 怎么样知道 transport 反序列化到哪个对象上呢。
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.
我不太明白你说的反序列化client场景的所在的意义,如果以为有这个必要的话,我可以重新提交pr
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.
func (fm *FilterManager) Apply(name string, conf map[string]interface{}) (HttpFilter, error) {
plugin, err := GetHttpFilterPlugin(name)
if err != nil {
return nil, errors.New("filter not found")
}
filter, err := plugin.CreateFilter()
if err != nil {
return nil, errors.New("plugin create filter error")
}
factoryConf := filter.Config()
//filter 的 config 是通过 yaml 反序列化去初始化的。
if err := yaml.ParseConfig(factoryConf, conf); err != nil {
return nil, errors.Wrap(err, "config error")
}
err = filter.Apply()
if err != nil {
return nil, errors.Wrap(err, "create fail")
}
return filter, 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.
好的,那我把transport放到filter结构体下,保证当前该代码的统一风格,有问题嘛
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.
这样应该没问题
我觉得可以,另外这种还可以保证代码的统一规范 |
Codecov Report
@@ Coverage Diff @@
## develop #260 +/- ##
========================================
Coverage 39.75% 39.75%
========================================
Files 50 50
Lines 2568 2568
========================================
Hits 1021 1021
Misses 1436 1436
Partials 111 111 Continue to review full report at Codecov.
|
* fix:POST request body is missing * Let users care about http.Client transport and keep a uniform code style Co-authored-by: randy <ztelur@gmail.com> Former-commit-id: 7e8add1
* fix:POST request body is missing * Let users care about http.Client transport and keep a uniform code style Co-authored-by: randy <ztelur@gmail.com>
What this PR does:
Which issue(s) this PR fixes:
Fixes #259
Special notes for your reviewer:
Does this PR introduce a user-facing change?: