Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added retryRaise and retryEither functions #3373

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

akotynski
Copy link
Contributor

Hey, with the latest API, it is easy to write Either version of the retry function. I think it could be quite useful.
I based the implementation on
public suspend fun <A> Schedule<Throwable, *>.retry(action: suspend () -> A): A

@kyay10
Copy link
Collaborator

kyay10 commented Feb 10, 2024

I think it might be preferable if this API used Raise directly instead. Could you perhaps make that change? It also seems like the step is only evaluated on Lefts, so perhaps in a Raise version it should be evaluated on the error type only

@akotynski akotynski force-pushed the either-schedule branch 2 times, most recently from ab2f35f to 7caa814 Compare February 10, 2024 16:29
@akotynski
Copy link
Contributor Author

akotynski commented Feb 10, 2024

do you mean like this?

public suspend fun <Error, Result, Output> Schedule<Error, Output>.retry(
  action: suspend Raise<Error>.() -> Result,
): Either<Error, Result>

I could even wrap it in raise context but it is not enabled in arrow

context (Raise<Error>)
public suspend fun <Error, Result, Output> Schedule<Error, Output>.retry(
  action: suspend Raise<Error>.() -> Result,
): Result

@akotynski akotynski changed the title introduce retry on Schedule<Either<*, *>, *> type added retryRaise and retryEither functions Feb 11, 2024
@akotynski akotynski force-pushed the either-schedule branch 3 times, most recently from c131af6 to dec5037 Compare February 12, 2024 08:08
Copy link
Member

@serras serras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great to me, but I would like to ask you to add everything to the Schedule.kt file instead. The reason is that this way the functions go into the ScheduleKt class, which makes our life a bit easier when checking binary compatibility aftewards.

Thanks for your contribution, @akotynski !

@akotynski
Copy link
Contributor Author

sure! Moved code to Schedule.kt file

Thank you all for your quick reply and support :)

Copy link
Collaborator

@kyay10 kyay10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful implementation!!

@serras serras merged commit af3814b into arrow-kt:main Feb 16, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants