openinx commented on a change in pull request #3498:
URL: https://github.com/apache/iceberg/pull/3498#discussion_r746218826
##########
File path: aliyun/src/main/java/org/apache/iceberg/aliyun/oss/BaseOSSFile.java
##########
@@ -23,18 +23,25 @@
import com.aliyun.oss.OSSErrorCode;
import com.aliyun.oss.OSSException;
import com.aliyun.oss.model.SimplifiedObjectMeta;
+import org.apache.iceberg.aliyun.AliyunProperties;
import org.apache.iceberg.relocated.com.google.common.base.MoreObjects;
abstract class BaseOSSFile {
private final OSS client;
private final OSSURI uri;
+ private AliyunProperties aliyunProperties;
private SimplifiedObjectMeta metadata;
BaseOSSFile(OSS client, OSSURI uri) {
this.client = client;
this.uri = uri;
}
+ BaseOSSFile(OSS client, OSSURI uri, AliyunProperties aliyunProperties) {
+ this(client, uri);
Review comment:
Instead of initializing this constructor on top of the `BaseOSSFile(OSS
client, OSSURI uri)`, I will prefer to initialize the `BaseOSSFile(OSS client,
OSSURI uri)` on top of this constructor. In the current approach, the
`aliyunProperties` will be set as a `null`, then we have to check whether it is
nullable everywhere in the upper classes such as `OSSOutputStream`.
I think the aliyunProperties can be initialized as a `new
AliyunProperties()` by default.
--
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]
