Simplify the definition of CommitRecovery about fs2-kafka HOT 4 CLOSED

Yeah, it seems like the benefit of changing it to be CommitRecovery[F[_]] will cause more difficulty in working with the API than it is worth. I agree that it should stay as-is, at least for now 🙂

from fs2-kafka.

I’ll take a look at this!

from fs2-kafka.

I may be missing something here, but it seems like this change is going to cause both Jitter and Timer to be required by all callers of CommitRecovery#Default.

def Default[F[_]](
    implicit F: MonadError[F, Throwable],
    jitter: Jitter[F],
    timer: Timer[F]
  ): CommitRecovery[F]

This is assuming that the new CommitRecovery looks like:

abstract class CommitRecovery[F[_]] {
  def recoverCommitWith(
    offsets: Map[TopicPartition, OffsetAndMetadata],
    commit: F[Unit]
  ): Throwable => F[Unit]
}

Do we think it is alright to "leak" Jitter and Timer around the codebase? Or is there another way you had in mind for doing this?

from fs2-kafka.

Thanks for taking a look at this @lewisjkl! The default CommitRecovery would only really be used in ConsumerSettings, where a default Jitter instance could be created. I think we can't get away from ConsumerSettings requiring Timer though, even though it might not be used (as is the case when a custom CommitRecovery is used). Sounds like maybe we should let it stay as-is. WDYT?

from fs2-kafka.