-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[ISSUE #3094]abstract create nacos restTemplate factory #3095
Conversation
…ze the commonly used http client config
# Conflicts: # common/src/main/java/com/alibaba/nacos/common/http/HttpClientManager.java
|
本来打算替换客户端的http client使用,但是发现每个使用的http client都有自己独有配置参数。为了保证PR内容尽量少,我这边暂时先不做替换的工作,先把nacos template创建重新抽象了,在这个PR合并之后,我将会继续替换客户端http client的任务,麻烦 @chuntaojun @KomachiSion 同学帮忙review该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.
Good job for all implementation.
I comment some problem. Change please
* @date 2020/6/16 | ||
*/ | ||
@SuppressWarnings("all") | ||
public final class HttpClientBeanFactory { |
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.
Maybe rename to HttpClientBeanCache
or HttpClientBeanHolder
will be better.
|
||
public static NacosRestTemplate getNacosRestTemplate() { | ||
HttpClientFactory httpClientFactory = new DefaultHttpClientConfig(); | ||
return getNacosRestTemplate(httpClientFactory); |
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.
merge these line to single line to remove tmp variable.
like return getNacosRestTemplate(new DefaultHttpClientConfig());
|
||
public static NacosAsyncRestTemplate getNacosAsyncRestTemplate() { | ||
HttpClientFactory httpClientFactory = new DefaultHttpClientConfig(); | ||
return getNacosAsyncRestTemplate(httpClientFactory); |
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.
Same as above description.
* @author mai.jh | ||
* @date 2020/6/15 | ||
*/ | ||
public abstract class AbstractHttpClientFactoryWrapper implements HttpClientFactory { |
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.
AbstractHttpClientFactory
is enough.
return HttpClientBeanFactory.getNacosRestTemplate(HTTP_CLIENT_FACTORY); | ||
} | ||
|
||
private static class NamingHttpClientConfig extends AbstractHttpClientFactoryWrapper { |
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.
NamingHttpClientFactory
will be better. NamingHttpClientConfig
make other think it is a config pojo.
* @author mai.jh | ||
* @date 2020/6/15 | ||
*/ | ||
public class DefaultHttpClientConfig extends AbstractHttpClientFactoryWrapper { |
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.
Same problem as NamingHttpClientConfig
. DefaultHttpClientFactory
will be better.
@@ -55,4 +55,6 @@ public void onEvent(Event event) { | |||
} | |||
}); | |||
} | |||
|
|||
|
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.
Why add this two blank line? Please remove.
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
fix: #3094
When using nacos templates,can customize common parameters of http client, such as timeout time and connection time.
NacosRestTemplate
andNacosAsyncRestTemplate
AbstractHttpClientFactoryWrapper
class, encapsulate the method created, provide custom http client config methodNacosRestTemplate
andNacosAsyncRestTemplate
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.