gavinchou commented on code in PR #48624:
URL: https://github.com/apache/doris/pull/48624#discussion_r1979756623


##########
cloud/src/recycler/s3_accessor.cpp:
##########
@@ -263,8 +354,30 @@ int S3Accessor::init() {
         // Therefore, you only need to add a policy to check if the response 
code is 429 and if the retry count meets the condition, it can record the retry 
count.
         options.PerRetryPolicies.emplace_back(
                 
std::make_unique<AzureRetryRecordPolicy>(config::max_s3_client_retry));
+
         auto container_client = 
std::make_shared<Azure::Storage::Blobs::BlobContainerClient>(
                 uri_, cred, std::move(options));
+
+        try {
+            // Test Azure connection
+            auto connection_result = testAzureConnection(container_client, 
conf_.bucket);
+            switch (connection_result) {
+            case ConnectionTestResult::SUCCESS:
+                break;
+            case ConnectionTestResult::AUTH_ERROR:
+                LOG(WARNING) << ("Azure S3 authentication failed");
+                return -1;
+            case ConnectionTestResult::TIMEOUT_ERROR:
+                LOG(WARNING) << ("Azure S3 connection timed out");
+                return -1;
+            default:
+                LOG(WARNING) << ("Azure S3 connection failed");
+                return -1;
+            }
+        } catch (const std::exception& e) {

Review Comment:
   `testAzureConnection()` already catch all exceptions why do we need this?
   and the switch case can be hidden in `testAzureConnection()` too.



##########
cloud/src/recycler/s3_accessor.cpp:
##########
@@ -292,12 +406,35 @@ int S3Accessor::init() {
         if (config::s3_client_http_scheme == "http") {
             aws_config.scheme = Aws::Http::Scheme::HTTP;
         }
+
         aws_config.retryStrategy = std::make_shared<S3CustomRetryStrategy>(
                 config::max_s3_client_retry /*scaleFactor = 25*/);
         auto s3_client = std::make_shared<Aws::S3::S3Client>(
                 std::move(aws_cred), std::move(aws_config),
                 Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
-                conf_.use_virtual_addressing /* useVirtualAddressing */);
+                conf_.use_virtual_addressing);
+
+        try {
+            // Test AWS S3 connection
+            auto connection_result = testAwsS3Connection(s3_client, 
conf_.bucket);
+            switch (connection_result) {
+            case ConnectionTestResult::SUCCESS:
+                break;
+            case ConnectionTestResult::AUTH_ERROR:
+                LOG(WARNING) << "AWS S3 authentication failed";
+                return -1;
+            case ConnectionTestResult::TIMEOUT_ERROR:
+                LOG(WARNING) << "AWS S3 connection timed out";
+                return -1;
+            default:
+                LOG(WARNING) << "AWS S3 connection failed";
+                return -1;
+            }
+        } catch (const std::exception& e) {

Review Comment:
   ditto



-- 
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: commits-unsubscr...@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to