Bugfix/strava limit

[b3b00]

No description provided.

[b3b00]

@orium did you see my pull-request ?

[orium]

Sorry @b3b00, I didn't yet :/ I will take a look later today.

[orium]

Thanks for the PR. I have some suggestions on how to improve things. If you need some extra mentoring let me know, I'm happy to help :)

[orium]

We don't need to replicate so much of this logic.

I think we can have some sort of parameterizeble backup strategy. For instance, we can have something an ADT like:

sealed trait BackoffStrategy
object BackoffStrategy {
  case class ExponentialBackoff(startBackoff: Duration, exponent: Double = 2.0) extends BackoffStrategy
  case class ConstantBackoff(backoff: Duration) extends BackoffStrategy
}

This way we wouldn't need a StravaRateLimiter and RateLimiter would receive a BackoffStrategy. E.g.

object RateLimiter {
  ...

  def byException[T <: Throwable: ClassTag]: RateLimiter =
    new RateLimiter(exceptionCausedBy[T], BackoffStrategy.ExponentialBackoff(startBackoff = 1.minute, exponent = 2.0))

  def byExceptionForStrava[T <: Throwable: ClassTag]: RateLimiter =
    new RateLimiter(exceptionCausedBy[T], BackoffStrategy.ConstantBackoff(startBackoff = 15.minutes))
}

We could still have the nice message saying that we are waiting x time, and will resume at y. Also, to print that message you do some math and manual formatting. There are methods that can help you with that (e.g. take a look at Instant (where you can do math with durations) and Duration).

[b3b00]

the constantbackoff is not optimal as you will always wait for 15 minutes even thiugh you may only need 1 minute if the exception occurs let say at 03:14 and rate limit will be reset at 3:15 as strava API states. So we must have some specific logic here. Strava rate limits are very specific I have never seen such conditions in other API.

so the constantbackoff must be renamed as the baxkoff is not constant at all.

I will try to provide a refactoring using a strategy pattern approach.

I agree for the flrmatting part.

[orium]

Yeah, it can be a BackoffStrategy.StravaBackoff or something like that :)

[orium]

This is not the best place to do this. The function that populates the cache is Strava.populateRunCache. I think easiest approach might be to just catch exceptions when fetching runs, logging them, and move on.

The fetch is done here:

strava-pr/src/main/scala/stravapr/Strava.scala

Line 80 in ded8f06

val r = fetchRunActivity(id).flatMap(activityToRun)

We can put a try { ... } catch { .. } and log that we failed to fetch a run. To report the error add another parameter to the function like the reportFetch (a reportFetchError I guess). This will it abstracted away from the output.

[b3b00]

you are right. my proposal was a first draft.