lidavidm commented on a change in pull request #10877:
URL: https://github.com/apache/arrow/pull/10877#discussion_r682888810



##########
File path: cpp/src/arrow/filesystem/s3fs.cc
##########
@@ -483,6 +483,30 @@ std::string FormatRange(int64_t start, int64_t length) {
   return ss.str();
 }
 
+// An AWS RetryStrategy that wraps a provided arrow::fs::S3RetryStrategy
+class WrappedRetryStrategy : public Aws::Client::RetryStrategy {
+ public:
+  explicit WrappedRetryStrategy(const std::shared_ptr<S3RetryStrategy>& 
s3_retry_strategy)
+      : s3_retry_strategy_(s3_retry_strategy) {}
+
+  bool ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors>& error,
+                   long attempted_retries) const override {  // NOLINT

Review comment:
       sorry, what's the NOLINT for? Nothing stands out to me as violating 
standards 

##########
File path: cpp/src/arrow/filesystem/s3fs.h
##########
@@ -69,6 +69,13 @@ enum class S3CredentialsKind : int8_t {
   WebIdentity
 };
 
+/// Pure virtual class for describing custom S3 retry strategies
+class S3RetryStrategy {
+ public:
+  virtual bool ShouldRetry(Status& error, long attempted_retries) = 0;
+  virtual long CalculateDelayBeforeNextRetry(Status& error, long 
attempted_retries) = 0;

Review comment:
       That could work, and/or it could be provided via a StatusDetail though 
that may not be as simple. 
https://github.com/apache/arrow/blob/72b52ef1b2681f927e8cfc2bc6a643648c542058/cpp/src/arrow/status.h#L99-L113




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


Reply via email to