-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Adding ScheduledAction to public API #2592
Conversation
Trying to think through the implications of this being part of the public API. Is it okay if I leave this out of 1.0.5 and we can follow up with 1.0.6 soon? |
Sure. |
* Note, however, if the {@code actualWorker} above didn't return a ScheduledAction, there is no | ||
* good way of untracking the returned {@code Subscription} (i.e., when to call {@code outerParent.remove(s)}). | ||
*/ | ||
public final class ScheduledAction implements Runnable, Subscription { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to add @experimental or @beta for the new public class.
I was playing with this in Hystrix with @mattrjacobs and it seemed we could achieve the desired reuse by adding a static factor methods to Here is example code: https://github.com/Netflix/Hystrix/blob/19320db57cdb4b600021875133c7ec61b4c96b82/hystrix-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixContextScheduler.java#L160-171 Could we replace this with an API call such as |
Sounds like a good alternative, although it would require the |
+1 on moving this complexity to RxJava and not leaving it to consumers to get right. |
Okay. I'll add the utility method and keep the ScheduledAction changes private. |
See #2761 for the new proposal. |
Resubmit of #2579.
I've copied the internal
ScheduledAction
into the publicrx.schedulers
package to allow customScheduler
implementors in other projects to take advantage of its correct unsubscription management.I've removed the internal
ScheduledAction
. Since relying on internal features are discouraged anyway, use places outside RxJava need to be updated. Deprecating it doesn't work because the class is final and the internals now return the newrx.schedulers.ScheduledAction
and would result inClassCastException
anyway.I've introduced a system-wide parameter
io.reactivex.scheduler.interrupt-on-unsubscribe
to enable interruption globally. In addition, eachScheduledAction
has its ownsetInterruptOnUnsubscribe
which allows enabling/disabling interrupts on an individual basis (overrides the default from the system-wide parameter above for the instance).I've also added detailed documentation to it and (while I was in the mood) to the
NewThreadWorker
.