openinx commented on a change in pull request #3659:
URL: https://github.com/apache/iceberg/pull/3659#discussion_r763569099
##########
File path:
aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java
##########
@@ -36,12 +37,10 @@ public static AliyunClientFactory defaultFactory() {
return ALIYUN_CLIENT_FACTORY_DEFAULT;
}
- public static AliyunClientFactory load(Map<String, String> properties) {
- if (properties.containsKey(AliyunProperties.CLIENT_FACTORY)) {
- return load(properties.get(AliyunProperties.CLIENT_FACTORY), properties);
- } else {
- return defaultFactory();
- }
+ public static AliyunClientFactory from(Map<String, String> properties) {
+ String factoryImpl = PropertyUtil.propertyAsString(
+ properties, AliyunProperties.CLIENT_FACTORY,
DefaultAliyunClientFactory.class.getName());
Review comment:
I think the previous version has a small bug, because the
`DefaultAliyunClientFactory` forget to initialize the aliyun properties in the
constructors. This PR unified the default aliyun client factory and customized
client factory, it's great to avoid the the properties initialization !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
