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

Migrate to native ReactiveSession and ReactiveResultSet [DATACASS-710] #827

Open
spring-projects-issues opened this issue Dec 13, 2019 · 3 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

Mark Paluch opened DATACASS-710 and commented

We should deprecate our DefaultBridgedReactiveSession and use the ReactiveSession that is shipped by the unified Datastax driver.

Requires Cassandra driver 4.4


Issue Links:

  • DATACASS-656 Upgrade to Cassandra Driver 4.3
    ("depends on")

1 votes, 4 watchers

@spring-projects-issues
Copy link
Author

Alexandre Dutra commented

Hi Mark Paluch how do you see the new design in terms of API and behavior? We are available to help but I think this ticket requires some prior discussion. I haven't tried anything yet, but it seems it could be challenging to adapt the driver's com.datastax.dse.driver.api.core.cql.reactive.ReactiveResultSet to org.springframework.data.cassandra.ReactiveResultSet

@spring-projects-issues
Copy link
Author

Tomasz Lelek commented

I will work on this ticket and investigate possible ways of dealing with this API change.

Mark Paluch what do you mean by We should deprecate our DefaultBridgedReactiveSession and use the ReactiveSession that is shipped by the unified Datastax driver.

The DefaultBridgedReactiveSession is implementing your org.springframework.data.cassandra.ReactiveSession, but our CqlSession provides own ReactiveSession.

The ReactiveSession from the driver has methods that are returning the Publisher<ReactiveRow>, whereas your API is based on the Flux. So I think that we still need some bridge layer, that will wrap Publishers into Flux. So for example this:

 

@Override 
public Mono<ReactiveResultSet> execute(String query) {
  return execute(SimpleStatement.newInstance(query)); 
}

Will need to be reworked to:

 

@Override
public Mono<ReactiveResultSet> execute(String query) {
  return Flux.from(session.executeReactive(query));
}

 

The second problem that Alexandre Dutra was referring to is that our APIs are using different abstractions.

 

The java-driver ReactiveSession is operating on the Publisher of a single entity (ReactiveRow). However, your API is using Mono of multiple entities (that are retrieved using the rows() method). Tha java-driver API can be transformed into the Flux, for example:

Flux result = Flux.from(session.executeReactive("SELECT ..."))

 

but the type on which the user needs to operate is Flux, not Mono.

I think that the reasonable solution will be to deprecate the DefaultBridgedReactiveSession and create the new wrapper around the java-driver ReactiveSession that only does the translation from the Publisher to Flux. But this will mean that the ReactiveResultSet will also need to be deprecated and replaced by the Publisher of multiple entities. It means that ReactiveResultSet#rows() and ReactiveResultSet#availableRows() methods are no longer needed. But I don't know how far we want to go with deprecating things. Please let me know wdyt

@spring-projects-issues
Copy link
Author

Mark Paluch commented

Right now, I'm not sure there is much we can do since the execution model is a different one between Spring Data's ReactiveResultSet and the one obtained from the Driver's reactive session.

We expect the emission of a result object (ReactiveResultSet) as a result of the query execution. It should report whether the query was successful or not (by either emitting the ReactiveResultSet or an error signal). The emitted object should also return whether the query was applied when using LWT. All methods but the ones for result consumption must be scalar to indicate that the query was actually executed and to allow direct consumption of e.g. wasApplied without reasoning from the API whether a call to Publisher<Boolean> wasApplied() will trigger another query or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants