You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/12/18 12:27:15 UTC

[GitHub] [pulsar] gaoran10 commented on a change in pull request #8985: [Issue 8887][tiered-storage-jcloud] support tiered-storage provider by aliyun OSS

gaoran10 commented on a change in pull request #8985:
URL: https://github.com/apache/pulsar/pull/8985#discussion_r545789742



##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
         }
     },
 
+
+    /**
+     * Aliyun OSS is compatible with the S3 API
+     * https://www.alibabacloud.com/help/doc-detail/64919.htm
+     */
+    ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+        @Override
+        public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+            VALIDATION.validate(config);
+        }
+
+        @Override
+        public BlobStore getBlobStore(TieredStorageConfiguration config) {
+            return OSS_BLOB_STORE_BUILDER.getBlobStore(config);

Review comment:
       `S3_STANDARD_BLOB_STORE_BUILDER` or other names? It seems that `oss` is a universal name.

##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
         }
     },
 
+
+    /**
+     * Aliyun OSS is compatible with the S3 API
+     * https://www.alibabacloud.com/help/doc-detail/64919.htm
+     */
+    ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+        @Override
+        public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+            VALIDATION.validate(config);

Review comment:
       Does the `serviceEndpoint` is a necessary configuration? If necessary it's better to add a configuration validationcheck.

##########
File path: tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/JCloudBlobStoreProvider.java
##########
@@ -162,6 +165,28 @@ public void buildCredentials(TieredStorageConfiguration config) {
         }
     },
 
+
+    /**
+     * Aliyun OSS is compatible with the S3 API
+     * https://www.alibabacloud.com/help/doc-detail/64919.htm
+     */
+    ALIYUN_OSS("aliyun-oss", new AnonymousProviderMetadata(new S3ApiMetadata(), "")) {
+        @Override
+        public void validate(TieredStorageConfiguration config) throws IllegalArgumentException {
+            VALIDATION.validate(config);
+        }
+
+        @Override
+        public BlobStore getBlobStore(TieredStorageConfiguration config) {
+            return OSS_BLOB_STORE_BUILDER.getBlobStore(config);
+        }
+
+        @Override
+        public void buildCredentials(TieredStorageConfiguration config) {
+            AWS_CREDENTIAL_BUILDER.buildCredentials(config);

Review comment:
       Does the Aliyun security validate is the same as AWS? Do we need to add environment configurations for the Aliyun service? Currently, the AWS security configurations could refer to [doc](https://pulsar.apache.org/docs/en/tiered-storage-aws/#authentication-required).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org