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.

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

RxJava2 Module #407

Merged
merged 26 commits into from
Apr 13, 2017
Merged

RxJava2 Module #407

merged 26 commits into from
Apr 13, 2017

Conversation

VisheshVadhera
Copy link
Contributor

Closes #345

Added Rx2 wrappers for the following classes in a separate module for Rx2 types:

  1. ApolloPrefetch (Completable)
  2. ApolloCall (Single)
  3. ApolloWatcher (Observable)

Felt that there is no need for an explicit function to get a backpressure supported Rx2 type for ApolloWatcher as we can always convert an Observable to a Flowable with toFlowable method on it.

This also means that all those classes which are implementing Cancelable interface now have to check in the callback for asynchronous ops if the this operation has been cancelled or not and then only proceed forward.
Remove toFlowable which I feel is not required
… issue-apollographql#345

Conflicts:
	apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java
	apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloPrefetch.java
	apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloWatcher.java
@VisheshVadhera VisheshVadhera changed the title Issue #345 RxJava2 Module Apr 11, 2017
POM_ARTIFACT_ID=compiler
POM_NAME=Apollo Android Compiler
POM_DESCRIPTION=Implementation for the Gradle plugin
POM_PACKAGING=jar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be aar since it depends on apollo runtime which is aar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it should be aar. Also need to make changes to the other variables. Pushing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be jar, and pure java module due to:
#409

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll change it to jar

}

/**
* Converts an {@link ApolloPrefetch} to an asynchronous Observable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically the Observable is not async, it has a callback but until someone uses a scheduler everything with still run synchronous by default

Copy link
Contributor Author

@VisheshVadhera VisheshVadhera Apr 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I meant ApolloWatcher here not ApolloPrefetch. But the thing with this observable is that clients don't block when they subscribe to it. So that's why I used async Observable.

Copy link
Contributor

@digitalbuddha digitalbuddha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add example to sample project that uses rx (future pr)

@VisheshVadhera
Copy link
Contributor Author

@digitalbuddha Yeah, I'll just create an issue and start work on it once this gets merged.

@digitalbuddha
Copy link
Contributor

Lgtm

@digitalbuddha
Copy link
Contributor

You need to exclude rxjava.properties from apk to make build pass

@VisheshVadhera
Copy link
Contributor Author

VisheshVadhera commented Apr 12, 2017

Fixed the duplicate file exception by adding the following line to apollo-integration's build.gradle

packagingOptions {
    exclude 'META-INF/rxjava.properties'
  }

@digitalbuddha Also wanted to know what kind of examples do you want me to add to the sample module. Do you want me to just add a simple Rx example for apollocall or you want me to add examples of other apollo types as well?

@digitalbuddha
Copy link
Contributor

Just a simple example would be fine showing how to get an observable, how to subscribe to it and how to unsubscribe on destroy. Hmm CI still failing with weird errors not sure why :-/

apply plugin: 'com.android.library'
clearSkipApolloRuntimeDep()

android {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls convert this module to pure java, as runtime and api right now java modules

Copy link
Contributor Author

@VisheshVadhera VisheshVadhera Apr 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to pure Java module

@VisheshVadhera
Copy link
Contributor Author

VisheshVadhera commented Apr 13, 2017

@digitalbuddha The builds are failing with a 137, which is an indication that the gradle process was killed by SIGKILL and is likely due to out of memory issues. Should we try increasing the heap size?

@sav007 sav007 merged commit c8deb5a into apollographql:master Apr 13, 2017
@VisheshVadhera VisheshVadhera deleted the issue-#345 branch May 4, 2017 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants