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.

Sign up for GitHub

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

Jump to bottom

RxJava 2 support #685

Closed
geralt-encore opened this issue Sep 27, 2016 · 45 comments
Closed

RxJava 2 support #685

geralt-encore opened this issue Sep 27, 2016 · 45 comments
Assignees
@nikitin-da
Labels
hacktoberfest

Comments

@geralt-encore
Copy link
Collaborator

Any thoughts about how to introduce it in a better manner without dropping RxJava 1 support?
Also object methods have to be reworked since they are emitting null if there is no requested object.
@geralt-encore
Copy link
Collaborator Author

Totally forgot about Maybe - it's perfect for that case.

@artem-zinnatullin
Copy link
Member

Yeah, there is some work required to do RxJava v2 support.

TL;TR plan:

  1. Wait for final release.
  2. Add basic support for Flowable because it supports backpressure which is
    required for IO operations.
  3. Add support for Single/Completable.
  4. Add/Think about support for Maybe.

RxJava 2 can work with RxJava 1 in same classpath, so I don't see problems
with that except naming of the methods (I think we'll just add "2" to it).

On 2 Oct 2016 10:30 am, "Ilya Zorin" [email protected] wrote:

Totally forgot about Maybe - it's perfect for that case.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#685 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA7B3FCuyaL5AWs-sb4gOL3psnCF8K3Pks5qv8ABgaJpZM4KHoyr
.

@geralt-encore
Copy link
Collaborator Author

Not sure if I understand you correctly, so you want to have both RxJava and RxJava 2 in the same library? It sounds not really good having both of these dependencies when you need only one of them in your project.
Regarding supporting Maybe - it's a perfect fit for object methods since null is prohibited in RxJava 2 and was kinda hacky in a first place.

@nikitin-da
Copy link
Collaborator

StorIO has just provided rxjava dependency - you can use rx only when you already have this dependency in your project

@geralt-encore
Copy link
Collaborator Author

Thanks for clarification @nikitin-da!
Anyway, I will be happy to help with this task.

@artem-zinnatullin
Copy link
Member

Great, let's wait for release then :)

On 3 Oct 2016 5:00 am, "Ilya Zorin" [email protected] wrote:

Thanks for clarification @nikitin-da https://github.com/nikitin-da!
Anyway, I will be happy to help with this task.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#685 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA7B3LHi2oPNnNQosC_7eDxso5pEMCCMks5qwMQlgaJpZM4KHoyr
.

@cmelchior cmelchior mentioned this issue Oct 29, 2016
Closed
@geralt-encore
Copy link
Collaborator Author

https://github.com/ReactiveX/RxJava/releases/tag/v2.0.0

@artem-zinnatullin
Copy link
Member

Yup, still waiting for more stable build though, but we can start migration.

@skrugly
Copy link

skrugly commented Oct 31, 2016

Good reason to lay the foundation for StorIO v2, huh?

@artem-zinnatullin
Copy link
Member

Hah, probably, but we're using semantic versioning so StorIO will become v2 as soon as we break API, adding RxJava v2 is planned as non-breaking change at the moment, so I guess StorIO will still be v1.

BUT: I'd like to keep both RxJava v1 and v2 only for the period of migration (in our internal and yours, users's) apps from RxJava v1 to v2 and when we'll see that RxJava v1 is not actively used anymore -> we'll drop support for RxJava v1 and that'll in fact require us to upgrade major version of StorIO!

@Adambl4 @nikitin-da @geralt-encore sounds good to you?

@nikitin-da
Copy link
Collaborator

I agree, both versions will be required for a certain period of time

@geralt-encore
Copy link
Collaborator Author

Sounds good 👍

@artem-zinnatullin
Copy link
Member

Though I admit that with https://github.com/akarnokd/RxJava2Interop from @akarnokd we can leave only RxJava v2 support and force users to use interop library to convert types between v1 and v2, but without extension methods support in plain Java it'll be non-fluent and add a lot of boilerplate to the code.

@geralt-encore
Copy link
Collaborator Author

What about using interop library internally? Completely migrate to v2 and provide methods for v1 interop via the library. Then just wipe it after some time.

@skrugly
Copy link

skrugly commented Nov 1, 2016

RxJava1 has 5k methods, RxJava2 another 9k.
Forcing users to have v2 in classpath if they don't need it is a crime against humanity.

@nikitin-da
Copy link
Collaborator

I don't like the idea to keep RxJava v2 methods only.
On the one hand it will speed up migration, but on the other hand it will lead to boilerplate on java6 and step towards to multidex 😱 on this period.
👍 for asRxObservable and asRxObservable2
Sure it will be more difficult for us, but much more democratic for users

@artem-zinnatullin
Copy link
Member

@geralt-encore

What about using interop library internally? Completely migrate to v2 and provide methods for v1 interop via the library. Then just wipe it after some time.

👍

PR welcome!

@Adambl4 nobody will force you download RxJava v2, we'll add it same way we added RxJava v1 support, it's optional dependency!

@artem-zinnatullin artem-zinnatullin added this to the v1.12.0 milestone Nov 13, 2016
@outofdate
Copy link

Any updates on RxJava2 support?

@artem-zinnatullin
Copy link
Member

artem-zinnatullin commented Dec 14, 2016 via email

@mayuroks
Copy link

Bump!, any progress on RxJava v2 support?

@artem-zinnatullin
Copy link
Member

Not yet, starting integrating it into work projects, then I'll look into support for StorIO, PRs welcome!

@artem-zinnatullin artem-zinnatullin modified the milestone: v1.12.0 Feb 19, 2017
@artem-zinnatullin artem-zinnatullin mentioned this issue Feb 19, 2017
Closed
@geralt-encore
Copy link
Collaborator Author

geralt-encore commented Feb 27, 2017 •
edited
Loading

I guess I can start working on what we agreed in #767.
There are still a couple of open questions:

  • Should we expose database only as Flowable or with a new one Observable as well?
  • What are we going to do with nulls in object method?

Everything else looks pretty clear to me.
Also, I want to add Maybe to both object and objects method, but it should be addressed after the migration in a separate PR, I guess.

@artem-zinnatullin @nikitin-da what do you guys think?

@geralt-encore
Copy link
Collaborator Author

@artem-zinnatullin so I have been looking into RxJava 2 migration lately and I was thinking since 2.0 is going to introduce API breaking changes because of interceptors we can introduce more of them to make migration easier?
I was thinking about:

  • Migrating completely to RxJava 2
  • Changing Observable to Flowable everywhere and method names as well to reflect it
  • Removing object method since it doesn't fit into RxJava 2 non-null model

I understand that it is not the best way for migration I honestly don't see any way to migrate while maintaining backwards compatibility since we haven't done anything about it for quite some time. And I think that it is better to make migration happen like that than don't make it at all.

btw I need some help to implement OnSubscribeExecuteAsBlocking for RxJava2. The current implementation is based on some internal RxJava APIs which I am not aware of. OnSubscribeExecuteAsBlockingSingle and OnSubscribeExecuteAsBlockingCompletable were pretty straightforward to implement, but I have no idea how to correctly rewrite OnSubscribeExecuteAsBlocking.

@artem-zinnatullin
Copy link
Member

artem-zinnatullin commented May 21, 2017 via email

@geralt-encore
Copy link
Collaborator Author

geralt-encore commented May 21, 2017 •
edited
Loading

Yes! It would be really nice if you made sure that the foundation works correctly since you are the guy who originally wrote it and you know rx way better than me =)
I can take care of all the more high-level stuff, tests and etc. And @nikitin-da of course if he is interested in it and has enough time =)

@artem-zinnatullin
Copy link
Member

So I've started migration yesterday, it'll definitely take a lot of evenings after work to complete. So what I'll do is — convert all StorIO main/src to RxJava 2 and use Interop library in tests so tests won't be modified that much. Then I'll create a PR so you could review it and merge, and then we'll be able to convert tests to RxJava 2 little by little.

Does that sound good?

@nikitin-da
Copy link
Collaborator

Sound nice! It will allow to do it iteratively and together 👍

@geralt-encore
Copy link
Collaborator Author

Sounds good! Let me know if can help you. I would be happy to help as much as possible.

@nikiJava
Copy link

Hello, guys! Are there any news about rxjava 2 integration in library?

@artem-zinnatullin
Copy link
Member

All work is public and can be found in initial-2.x branch, I've migrated almost all StorIO codebase to RxJava 2.x

I need to make couple tests however, regarding need in Flowable and then a week of evenings or so to finish the work. If someone wants to continue my work, I'm more than happy to make a PR to 2.x branch so somebody could work on top of it

@geralt-encore
Copy link
Collaborator Author

I am ready to help. I was stuck on some low-level parts when I was trying to migrate it myself. I assume that you already have dealt with them so I am ready to help with whatever needed.

@ZakTaccardi
Copy link

For nullabillity in RxJava2, we need a way to emit a null field for when a record is not found while keeping a hot stream, so Maybe does not suffice.

Has this been considered? Maybe some type of converter/adapter? Or does .onErrorReturn() suffice to catch the null pointer exception and return our own Optional implemention (does this style of error handling have side effects?)

@geralt-encore
Copy link
Collaborator Author

I was thinking about the same thing recently. Converter/adapter won't work since they apply after the emission. Emitting nulls and catching them with onErrorReturn() seems like an anti-pattern to be honest. The only solution that I see is to bundle some Optional implementation with StorIO and use it for that case.

@ZakTaccardi
Copy link

Or does .onErrorReturn() suffice to catch the null pointer exception and return our own Optional implemention (does this style of error handling have side effects?)

This definitely does not work, because it terminates the stream.

The only solution that I see is to bundle some Optional implementation with StorIO and use it for that case.

This seems like the best solution. Just make sure that it's not called Optional and something more specific to this lib to not confuse it with our own Optional implantations.

@Rainer-Lang
Copy link

Will you add a migration guide?

@nikitin-da
Copy link
Collaborator

@Rainer-Lang Right now it is only available via https://github.com/akarnokd/RxJava2Interop
When we will finish build in rxjava2 support of cause we should add some readme about migration

@Rainer-Lang
Copy link

@nikitin-da Thanks for your feedback.

@nikitin-da nikitin-da self-assigned this Sep 22, 2017
@geralt-encore
Copy link
Collaborator Author

@artem-zinnatullin can I continue migration based on your 2.x branch or is it still early and some crucial things are missing?

@nikitin-da
Copy link
Collaborator

@geralt-encore I was stuck on this issue ReactiveX/RxJava#5234 but it seems some workaround was released ReactiveX/RxJava#5590

@geralt-encore
Copy link
Collaborator Author

ah, you are working on it! Didn't know about that. So let me know if you need any help then.

@Rainer-Lang
Copy link

@nikitin-da @geralt-encore Thanks for the update.
BTW, I don't want to use the Interop-lib for something so essential like the database.

@nikitin-da nikitin-da mentioned this issue Oct 14, 2017
Closed
@pavlospt
Copy link

@geralt-encore Is there any ETA on RxJava2 support? We are heavily dependent on StorIO for some basic functionality and we would like to see if we need to investigate for other solutions :) !!

@nikitin-da
Copy link
Collaborator

@pavlospt sorry for delay.
Migration is in progress in branch https://github.com/pushtorefresh/storio/commits/az/initial-2.x
There are couple of unresolved issues for now: optional wrapper and some problems with class loader for use cases without rx dependency

@nikitin-da
Copy link
Collaborator

I hope we will resolve it in 2 weeks

@pavlospt
Copy link

@nikitin-da No need to say sorry :) Thank you for your work on the project and for the insights :) !!

This was referenced Nov 6, 2017
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nikitin-da nikitin-da

Labels
hacktoberfest
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@ZakTaccardi @artem-zinnatullin @pavlospt @geralt-encore @mayuroks @outofdate @nikitin-da @Rainer-Lang @skrugly @nikiJava