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

RxJava support #50

Open
boulderwebdev opened this issue Oct 21, 2020 · 9 comments
Open

RxJava support #50

boulderwebdev opened this issue Oct 21, 2020 · 9 comments
Assignees
Labels
question Further information is requested

Comments

@boulderwebdev
Copy link

Are there plans in motion for adding RxJava(2) support? Also, could you add to the documentation that as the project stands, it is not compatible with vertx-rx?

@vietj
Copy link
Member

vietj commented Oct 25, 2020

this SDK does not use Vert.x generated API, so it will not benefit from an auto generated RxJava API.

it seems to me that the only option is to use RxJava support from the AWS SDK

@vietj
Copy link
Member

vietj commented Oct 25, 2020

@aesteve correct me if I'm wrong

@aesteve
Copy link
Collaborator

aesteve commented Oct 25, 2020 via email

@aesteve aesteve added the question Further information is requested label Oct 25, 2020
@aesteve
Copy link
Collaborator

aesteve commented Oct 25, 2020

What do you mean by "not compatible with vertx-rx" exactly?
What did you try, what were your expecting and was the result? Thanks.

@boulderwebdev
Copy link
Author

boulderwebdev commented Oct 25, 2020

@aesteve Well, for starters, the compiler will complain about the following line

withVertx(builder, vertx.getOrCreateContext()).build();

with a reactivex context. I.e. if I use the imports

import io.vertx.reactivex.core.Context;
import static io.reactiverse.awssdk.VertxSdkClient.withVertx;

At the bare minimum, I want to instantiate a client upon creating a handler to put into the router. Then, I can make calls to S3 whenever I need to retrieve some data that's not cached locally.

@boulderwebdev
Copy link
Author

boulderwebdev commented Oct 25, 2020

Also, from my understanding of reading this codebase, it seems like the main idea is to give amazon a custom HTTP client which is compatible with Vert.x's event loop so calls to any AWS api uses this client. Is this correct? If so, (I think) an Rxified wrapper around this project could be created which is compatible with the Rxified vertx API.

Also, it seems like the single method in the test code takes you 90% of the way for handling requests to S3, but I'm not sure how well it generalizes to the other services. I modified it in some code I wrote to

public static <T> Single<T> single(CompletableFuture<T> future) {
    final SingleOnSubscribe<T> sos = emitter ->
      future.handle((result, error) -> {
        if (emitter.isDisposed()) {
          return future; // maybe should be 'return null' ?
        }
        if (error != null) {
          emitter.onError(error);
        } else {
          emitter.onSuccess(result);
        }
        return future;
      });
    return Single.create(sos);
  }

and this compiles correctly and doesn't throw any errors with various test cases.

@aesteve
Copy link
Collaborator

aesteve commented Oct 26, 2020

In order to fix the compilation error, I think you can use `vertx.delegate.getOrCreateContext()ˋ, let me know if this works.

As for the single method in tests, I'm happy it helps you tackle most of your use cases.

I'll update the docs to mention RxJava support is out of the scope of the lib, but indicate there are some hints in the tests.

Thanks for reporting this.

@aesteve
Copy link
Collaborator

aesteve commented Oct 26, 2020

it seems like the main idea is to give amazon a custom HTTP client which is compatible with Vert.x's event loop so calls to any AWS api uses this client. Is this correct?

Partially yes.
As indicated in the docs, the lib also makes sure callbacks are executed in the same context that initiated the AWS API call, which is one of Vert.x guarantees.

If you think such a wrapper is needed and are ready to test it then maintain it in the long run, it's certainly a good idea.

@boulderwebdev
Copy link
Author

@aesteve thanks for the pointers. I'll go ahead and create a test case soon to see if vertx.getDelegate() fixed my problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants