morningman commented on code in PR #56861:
URL: https://github.com/apache/doris/pull/56861#discussion_r2433020054
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AzureProperties.java:
##########
@@ -155,24 +159,46 @@ public static String formatAzureEndpoint(String endpoint,
String accessKey) {
@Override
public String validateAndNormalizeUri(String url) throws UserException {
- return S3PropertyUtils.validateAndNormalizeUri(url, usePathStyle,
forceParsingByStandardUrl);
+ return AzurePropertyUtils.validateAndNormalizeUri(url);
}
@Override
public String validateAndGetUri(Map<String, String> loadProps) throws
UserException {
- return S3PropertyUtils.validateAndGetUri(loadProps);
+ return AzurePropertyUtils.validateAndGetUri(loadProps);
}
@Override
public String getStorageName() {
- return "Azure";
+ return "AZURE";
Review Comment:
why uppercase?
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/LocationPath.java:
##########
@@ -185,6 +185,21 @@ public static LocationPath of(String location,
}
}
+ public static LocationPath of(String location,
+ StorageProperties storageProperties) {
+ try {
+ String schema = extractScheme(location);
+ String normalizedLocation =
storageProperties.validateAndNormalizeUri(location);
+ String encodedLocation = encodedLocation(normalizedLocation);
+ URI uri = URI.create(encodedLocation);
+ String fsIdentifier = Strings.nullToEmpty(uri.getScheme()) + "://"
+ + Strings.nullToEmpty(uri.getAuthority());
+ return new LocationPath(schema, normalizedLocation, fsIdentifier,
storageProperties);
Review Comment:
```suggestion
return new LocationPath(schema, normalizedLocation,
fsIdentifier, storageProperties);
```
##########
fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java:
##########
@@ -330,7 +371,7 @@ public Status globList(String remotePath, List<RemoteFile>
result, boolean fileN
java.nio.file.Path blobPath =
Paths.get(blobItem.getName());
boolean isPrefix = false;
- while
(blobPath.normalize().toString().startsWith(listPrefix)) {
+ while (null != blobPath &&
blobPath.normalize().toString().startsWith(listPrefix)) {
Review Comment:
Why adding `null != blobPath`, is this a bug?
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AzureProperties.java:
##########
@@ -64,18 +65,20 @@ public class AzureProperties extends StorageProperties {
@Getter
- @ConnectorProperty(names = {"azure.access_key", "s3.access_key",
"AWS_ACCESS_KEY", "ACCESS_KEY", "access_key"},
+ @ConnectorProperty(names = {"azure.account_name", "azure.access_key",
"s3.access_key",
+ "AWS_ACCESS_KEY", "ACCESS_KEY", "access_key"},
description = "The access key of S3.")
protected String accessKey = "";
Review Comment:
how about change this to "accountName"?
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AzureProperties.java:
##########
@@ -137,6 +140,7 @@ public Map<String, String> getBackendConfigProperties() {
s3Props.put("AWS_SECRET_KEY", secretKey);
s3Props.put("AWS_NEED_OVERRIDE_ENDPOINT", "true");
s3Props.put("provider", "azure");
+ s3Props.put("PROVIDER", "AZURE");
Review Comment:
Why need a uppercase?
##########
fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java:
##########
@@ -427,6 +506,13 @@ public Status multipartUpload(String remotePath, @Nullable
InputStream inputStre
@Override
public void close() throws Exception {
// Create a BlobServiceClient instance (thread-safe and reusable).
- // Note: BlobServiceClient does NOT implement Closeable and does not
require explicit closing.
+ // Note: BlobServiceClient does NOT implement Closeable and does not
require explicit closing.
+ }
+
+ public static String generateBlockId(int index) {
Review Comment:
not used?
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSTransaction.java:
##########
Review Comment:
Change the name to `objCommit`?
##########
fe/pom.xml:
##########
@@ -1008,6 +1011,16 @@ under the License.
<artifactId>objenesis</artifactId>
<version>${objenesis.version}</version>
</dependency>
+ <dependency>
Review Comment:
Add comment to explain why need these deps
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AzureProperties.java:
##########
@@ -64,18 +65,20 @@ public class AzureProperties extends StorageProperties {
@Getter
- @ConnectorProperty(names = {"azure.access_key", "s3.access_key",
"AWS_ACCESS_KEY", "ACCESS_KEY", "access_key"},
+ @ConnectorProperty(names = {"azure.account_name", "azure.access_key",
"s3.access_key",
+ "AWS_ACCESS_KEY", "ACCESS_KEY", "access_key"},
description = "The access key of S3.")
protected String accessKey = "";
@Getter
- @ConnectorProperty(names = {"azure.secret_key", "s3.secret_key",
"AWS_SECRET_KEY", "secret_key"},
+ @ConnectorProperty(names = {"azure.account_key", "azure.secret_key",
"s3.secret_key",
+ "AWS_SECRET_KEY", "secret_key"},
sensitive = true,
description = "The secret key of S3.")
protected String secretKey = "";
Review Comment:
dito
--
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]
