Refactor ES access with new es client #4900
Conversation
cangfengzhs
force-pushed
the
refactor-es
branch
from
72fbb28 to
224f2c7
Compare
cangfengzhs
force-pushed
the
refactor-es
branch
from
30f372a to
653294d
Compare
cangfengzhs
force-pushed
the
refactor-es
branch
from
17cd3f8 to
bc43304
Compare
Codecov ReportBase: 76.86% // Head: 77.20% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #4900 +/- ##
==========================================
+ Coverage 76.86% 77.20% +0.34%
==========================================
Files 1105 1098 -7
Lines 81432 81174 -258
==========================================
+ Hits 62594 62674 +80
+ Misses 18838 18500 -338
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
cangfengzhs
requested review from
pengweisong,
SuperYoko,
panda-sheep and
liwenhui-soul
| virtual HttpResponse delete_(const std::string& url, | ||
| const std::vector<std::string>& headers, | ||
| const std::string& username, | ||
| const std::string& password); |
There was a problem hiding this comment.
Why not store the username and password in HttpClient to avoid to pass them in each function?
There was a problem hiding this comment.
In the beginning, httpclient was designed to be stateless, and it only has some static functions. Later, in order to facilitate the use of gmock in the test, we changed it to non static, but the httpclient is still stateless.
There was a problem hiding this comment.
In addition, username, password and url are bound, but httpclient and url are not bound. Therefore, it is not reasonable to put username and password in httpclient. Maybe you have other better suggestions?
| @@ -6,6 +6,8 @@ | |||
| #include <gtest/gtest.h> | |||
| #include <thrift/lib/cpp/concurrency/ThreadManager.h> | |||
|
|
|||
| #include <map> | |||
There was a problem hiding this comment.
It seems you don't need to modify this test since derived Listener do not need to overload apply function anymore. Not a big issue, up to you.
There was a problem hiding this comment.
Just to compile and pass the test
|
This PR involves doc updated, @randomJoe211 , including:
|
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Description:
Before that, in #4891 and #4970, I have introduced libcurl and created httpClient and esclient.
In this PR, I will discard the original es calling method and use the ESAdapter and ESClient I created to access ES. ESAdapter modifies the data format written to ES by full-text index.
Note, however, that the PR does not modify the use logic of full-text index, and still needs to rely on the native index. This problem will be solved in the next PR.
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: