This repository has been archived by the owner on Jun 23, 2022. It is now read-only.
fix(http): improvement on http api #296
Merged
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
acelyc111
reviewed
Aug 16, 2019
| return error_s::make(ERR_INVALID_PARAMETERS, std::string("repeat parameters")); | ||
| ret.query_args.emplace(arg_vals[0], arg_vals[1]); | ||
| for (const std::string &arg_val : method_arg_val) { | ||
| size_t sep = arg_val.find_first_of('='); |
There was a problem hiding this comment.
感觉这种split类的操作在我们的项目中有好几种实现方式
这里是可以抽象出来, split出多个元素(1个,2个或多个可能是有不同的处理逻辑)
There was a problem hiding this comment.
是的，之前引入 pprof 的时候还引入了 brpc 的 string_splitter，这个是相对 boost::split 性能更好的一种 split 写法，减少了内存拷贝，后面可以主推这种。但是在这里我们因为逻辑简单，杀鸡就不用牛刀了把。
| @@ -47,7 +47,7 @@ class recent_start_time_http_service : public http_service | |||
| std::placeholders::_2)); | |||
| } | |||
|
|
|||
| std::string path() const override { return "startTime"; } | |||
| std::string path() const override { return "recentStartTime"; } | |||
There was a problem hiding this comment.
这里我觉得还是path作为一个http_service的成员变量比较好, 由基类返回就行了
There was a problem hiding this comment.
现在的写法在可靠性上问题不大，理解你的想法是每个 service 要写一个 path 函数比较麻烦？这个交给后面的重构做把。
| {"http://127.0.0.1:34601?query1=value1", ERR_OK, {{"query1", "value1"}}}, | ||
| {"http://127.0.0.1:34601?=", ERR_OK, {{"", ""}}}, | ||
| {"http://127.0.0.1:34601?", ERR_OK, {}}, | ||
|
|
There was a problem hiding this comment.
没有，因为参数比较多，避免看花眼
added 2 commits
August 17, 2019 07:14
acelyc111
approved these changes
Aug 20, 2019
vagetablechicken
approved these changes
Aug 20, 2019
neverchanje
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The major improvements are the followings:
table_printer, so that http result on/recentStartTimeare more readable.the old result is:
{ "RecentStartTime": { //redundant "RecentStartTime": "2019-07-30 14:04:25" } }the new result is:
?arg=true, for example:Now, the url can be simplified to: