mdedetrich commented on code in PR #11:
URL: 
https://github.com/apache/pekko-persistence-dynamodb/pull/11#discussion_r1805155361


##########
src/main/scala/org/apache/pekko/persistence/dynamodb/journal/DynamoDBHelper.scala:
##########
@@ -32,6 +32,35 @@ import scala.concurrent.duration._
 case class LatencyReport(nanos: Long, retries: Int)
 private class RetryStateHolder(var retries: Int = 10, var backoff: 
FiniteDuration = 1.millis)
 
+/**
+ * Auxiliary object to help determining whether we should retry on a certain 
throwable.
+ */
+object DynamoRetriableException {

Review Comment:
   This should be marked `@InternalApi` and also be made package private. I 
don't think there is any usecase where the user would want to override this and 
if so then the structure should be redesigned to better accomodate that.



##########
src/main/scala/org/apache/pekko/persistence/dynamodb/journal/DynamoDBHelper.scala:
##########
@@ -32,6 +32,35 @@ import scala.concurrent.duration._
 case class LatencyReport(nanos: Long, retries: Int)
 private class RetryStateHolder(var retries: Int = 10, var backoff: 
FiniteDuration = 1.millis)
 
+/**
+ * Auxiliary object to help determining whether we should retry on a certain 
throwable.
+ */
+object DynamoRetriableException {
+  def unapply(ex: AmazonServiceException) = {
+    ex match {

Review Comment:
   One thing I would add is to the exception retry  prediate is that we 
shouldn't retry on **ANY** 4xx error (which may already include ones in the 
current implementation) as retrying on 4xx errors would never succeed (unless 
Amazon maldesigned their API).



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

Reply via email to