-
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
3.x: Add RxJavaPlugins.createExecutorScheduler #7306
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.x #7306 +/- ##
============================================
- Coverage 99.53% 99.52% -0.02%
- Complexity 6783 6786 +3
============================================
Files 751 751
Lines 47489 47490 +1
Branches 6378 6378
============================================
- Hits 47268 47263 -5
- Misses 100 103 +3
- Partials 121 124 +3
Continue to review full report at Codecov.
|
Hi there. I'm not sure this change is working as expected. The following code creates the standard default schedulers: ExecutorService executor = Executors.newSingleThreadExecutor();
Scheduler scheduler = RxJavaPlugins.createExecutorScheduler(executor, false, false);
Flowable.intervalRange(1, 10, 1, 1, TimeUnit.SECONDS, scheduler)
.blockingForEach(System.out::println);
executor.shutdown(); But according to the new Javadoc it should not trigger the creation of the default schedulers:
It clearly will though because the Apologies if I'm misunderstanding the intent of this change. If I am then that may indicate that the new Javadoc is a bit misleading. |
Indeed the method should not trigger that initialization. I'll work out how to break that cycle and post a fix shortly. |
Thanks for your quick response David. 👍 |
This PR adds the
RxJavaPlugins.createExecutorScheduler
that instantiates the already existingExecutorScheduler
independent of theSchedulers
class. Thefrom
methods in theSchedulers
class are now pointing to this new method.Resolves #7305
Related #7300