kotlinx.coroutines icon indicating copy to clipboard operation
kotlinx.coroutines copied to clipboard

Published 20 hours ago • verified publisher icon Kotlin

Add timeout operator to Flow (redux)

Open pablobaxter opened this issue 3 years ago • 9 comments

This adds functionality similar to the timeout() in Rx. This invokes a timeout if the upstream flow doesn't emit after the set period of time, with the option to perform a custom action on timeout.

This PR is in support of #2624. One item that was discussed was the name of this new function. I couldn't think of a better one, but I'm not opposed to renaming from timeout() to something else.

pablobaxter avatar Jan 19 '22 04:01 pablobaxter

Any update on this PR?

pablobaxter avatar Sep 06 '22 17:09 pablobaxter

Hi, sorry, this PR eluded me.

We are very careful with time-related API and still not sure how to properly expose it (e.g. https://github.com/Kotlin/kotlinx.coroutines/pull/2745#pullrequestreview-681969186 and https://github.com/Kotlin/kotlinx.coroutines/pull/2738)

We'll use this work as a reference, but we still haven't found a proper chunk of time to get back to it

qwwdfsad avatar Sep 08 '22 14:09 qwwdfsad

Thanks for the update!

Regarding the reason from #2745, I understand the concern, but the problem currently is that there is no easy way to shutdown an upstream flow if nothing has been emitted after some period of time, without also shutting down the downstream. What this operator provides does not exist with Flow currently. As mentioned in the issue (https://github.com/Kotlin/kotlinx.coroutines/issues/2624), this operator will be a huge benefit for better handling of resource intensive tasks (ex: GPS emitting on Android, which is battery intensive) when used with a flow.

Let me know if there's anything more I can do to help push this along.

pablobaxter avatar Sep 08 '22 16:09 pablobaxter

I think this kind of operator is essential，but do need more careful to took in as a stable.api

He-Pin avatar Sep 08 '22 16:09 He-Pin

Also, please change the base branch to develop and rebase on the latest develop for the sake of simplicity

qwwdfsad avatar Sep 12 '22 13:09 qwwdfsad

@pablobaxter maybe rename to idleTimeout ?

He-Pin avatar Sep 12 '22 16:09 He-Pin

@pablobaxter maybe rename to idleTimeout ?

@qwwdfsad Thoughts? I know you had a concern about the name of the API as well.

pablobaxter avatar Sep 12 '22 16:09 pablobaxter

incase there will be something like backpressureTimeout completionTimeout etc someday.

He-Pin avatar Sep 12 '22 16:09 He-Pin

I'll return to this PR during the course of next week, no worries here

qwwdfsad avatar Sep 15 '22 15:09 qwwdfsad

Sorry for the delay with review -- caught COVID and became even less responsive :(

qwwdfsad avatar Sep 27 '22 11:09 qwwdfsad

Sorry for the delay with review -- caught COVID and became even less responsive :(

No worries. I hope you are resting up and taking care of yourself. Get better soon. I'll work on the changes and update accordingly.

pablobaxter avatar Sep 27 '22 17:09 pablobaxter

Sorry for the delay with review -- caught COVID and became even less responsive :(

No worries. I hope you are resting up and taking care of yourself. Get better soon. I'll work on the changes and update accordingly.

Take good care

He-Pin avatar Sep 29 '22 06:09 He-Pin

Thank you! 👍

qwwdfsad avatar Oct 05 '22 15:10 qwwdfsad

Thank you! 👍

Thank you for helping push this along!

pablobaxter avatar Oct 05 '22 16:10 pablobaxter