morningman commented on code in PR #56163:
URL: https://github.com/apache/doris/pull/56163#discussion_r2356230756
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java:
##########
@@ -50,6 +53,8 @@ public class StorageVaultMgr {
private static final Logger LOG =
LogManager.getLogger(StorageVaultMgr.class);
private static final ExecutorService ALTER_BE_SYNC_THREAD_POOL =
Executors.newFixedThreadPool(1);
private final SystemInfoService systemInfoService;
+
+ public static final String S3_ROOT_PATH = "s3.root.path";
Review Comment:
duplicate in `S3StorageVault.java`.
I think we can move it to `S3Properties`?
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java:
##########
@@ -61,9 +61,8 @@ public class PrintableMap<K, V> {
SENSITIVE_KEY.add("jdbc.password");
SENSITIVE_KEY.add("elasticsearch.password");
SENSITIVE_KEY.addAll(Arrays.asList(
- S3Properties.SECRET_KEY,
- S3Properties.Env.SECRET_KEY,
MCProperties.SECRET_KEY));
+ SENSITIVE_KEY.addAll(S3Properties.SENSITIVE_KEYS);
Review Comment:
Use method like
`SENSITIVE_KEY.addAll(ConnectorPropertiesUtils.getSensitiveKeys(OBSProperties.class));`
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AzureProperties.java:
##########
@@ -94,6 +94,10 @@ public class AzureProperties extends StorageProperties {
@Getter
protected String forceParsingByStandardUrl = "false";
+ public static final String ENDPOINT = "azure.endpoint";
Review Comment:
These 4 strings are not used, can be removed
##########
fe/fe-core/src/main/java/org/apache/doris/task/PushStoragePolicyTask.java:
##########
@@ -100,4 +103,37 @@ public TPushStoragePolicyReq toThrift() {
ret.setDroppedStoragePolicy(droppedStoragePolicy);
return ret;
}
+
+ private static TS3StorageParam getS3TStorageParam(Map<String, String>
properties) {
Review Comment:
Need to refactor this later.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java:
##########
@@ -156,6 +161,56 @@ private Cloud.StorageVaultPB.Builder
buildAlterS3VaultRequest(Map<String, String
return alterObjVaultBuilder;
}
+ private static Cloud.ObjectStoreInfoPB.Builder
getObjStoreInfoPB(Map<String, String> properties) {
Review Comment:
I think this method should be moved to S3Properties?
1. properties -> S3StorageProperties
2. S3StorageProperties -> ObjectStoreInfoPB
--
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]