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

[Experimental] Reactive ReqlAst.run #12

Closed
wants to merge 9 commits into from

Conversation

NotJustAnna
Copy link
Member

@NotJustAnna NotJustAnna commented Feb 15, 2020

WARNING: This PR contains a lot of breaking changes.

This is a pull request on top of develop/experimental (#7), that replaces Cursors with Project Reactor's Flux<T>.

From #8:

At first, ReQL syntax feels nice, but after some usage, it starts to freak you out.

Some queries return Cursor<T>, others return List<T>, others just return T.If you're a JS/Python/Ruby developer, it is fine, but in Java it makes you feel lost.

The reason lies on this line:

public <T, P> T run(Connection conn, OptArgs runOpts, Class<P> pojoClass) {

In Java, experienced developers orient themselves by the type system. But in this case, there is no type logic at all!

T, the return type, can be P, but can also be Cursor<P>, which makes all code a runtime guessing game of "will it return a cursor, a list or something else?".

This pull request breaks in almost all senses the current Java driver API, BUT it gives users a power tool for handling everything from massive tables to changefeeds.

I wanna discuss how feasable is to do this change, and if so, how can we deliver it to the users.

@NotJustAnna
Copy link
Member Author

Since 037429c, it now has a reactive Connection implementation, with detached Socket and Response Pump, if you want to write your custom implementation of the socket or integrate the response pump into your main service, for example Vert.x or Spring Framework.

@NotJustAnna
Copy link
Member Author

NotJustAnna commented Feb 17, 2020

This experiment is being dropped. Expect the branch to be cherry-picked into #7

Reason why this PR is being dropped is:

The reactive streams specification disallows null values in a sequence.

RethinkDB can return null in sequence and atom response, therefore not compatible with the driver.

(The branch will be deleted once everything was cherry-picked)

@NotJustAnna NotJustAnna deleted the experiment/reactive branch February 20, 2020 14:21
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.

1 participant