search

airbnb / mavericks

Mavericks: Android on Autopilot
https://airbnb.io/mavericks/
Apache License 2.0
5.83k stars 500 forks source link

execute() sets subscribeOn() scheduler #70

Closed chrisbanes closed 5 years ago

chrisbanes commented 6 years ago

Looking at execute(), it seems to set the subscribeOn() scheduler to be the single threaded background subscriber the VM keeps. While I can see why the observeOn() scheduler is set, I don't see why you force the actual subscription on the same scheduler.

All of my observables are coming from a database, so I want them on the io() scheduler ideally.

hellohuanlin commented 6 years ago

@chrisbanes your upstream database call should subscribeOn(io). The downstream subscribeOn in VM will be a no-op.

chrisbanes commented 6 years ago

The observer should be forced, but the subscribe should not be touching the state at all and I see no reason to restrict.

@hellohuanlin relying on that feels flakey. It only takes a small behaviour change in Rx to break that.

hellohuanlin commented 6 years ago

@chrisbanes I agree that it's pretty strange and unexpected to have downstream switching threads for the upstream operation (if upstream doesn't already provide a subscribeOn)

Oknesif commented 6 years ago

As I can see, PR for this issue is here: https://github.com/airbnb/MvRx/pull/75

gpeal commented 5 years ago

Yes, this was fixed in #75