Cats Effect context for cassanda

[voropaevp]

Fixes #issue_number

Problem

Cats Effect context for cassanda. Cats ecosystem currently missing cassandra orms.

Solution

Cats Effect context for cassanda

Notes

2.11 is not support. Makes empty artifact.

Increased the ReservedCodeCacheSize to avoid warning on 2.12 compilation.

OpenJDK 64-Bit Server VM warning: CodeCache is full. Compiler has been disabled.
OpenJDK 64-Bit Server VM warning: Try increasing the code cache size using -XX:ReservedCodeCacheSize=
CodeCache: size=262144Kb used=229683Kb max_used=257547Kb free=32460Kb
 bounds [0x00007f75e0000000, 0x00007f75f0000000, 0x00007f75f0000000]
 total_blobs=51233 nmethods=50316 adapters=826
 compilation: enabled

Also changed the courier directory to the /home/sbtuser/ from /root.

Checklist

• Unit test all changes
• Update README.md if applicable
• Add [WIP] to the pull request title if it's work in progress
• Squash commits that aren't meaningful changes
• Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

[mdedetrich]

Nice work! Could you squash the PR into a single commit? Also a lot of files don't have new lines at the end of the file.

[mdedetrich]

Why is this called GuavaCeUtils, specifically why is the Guave here?

[voropaevp]

That object hold and implicit class to convert guava ListenableFuture[T] to the F[T]. What do you think will be an approtiate name for it?

[deusaquilus]

I think this is reasonable. The Cassandra session code uses Guava's Future to do Async so we have to deal with Guava in any case.

[deusaquilus]

This is fine. The Cassandra session directly uses Guava Futures so you have no choice but to wrap that stuff into F[_] and this is a reasonable implementation of that.

[mdedetrich]

Ah I didn't realize that cassandra uses Guave futures, in that case its okay. I still personally think its better design to put this inside of CassandraCeContext or CeContext rather than making a new top level object since this is consistent with how the rest of quill works.

[djspiewak]

I would recommend doing this differently. ListenableFuture is an eagerly-evaluating construct, similar to CompletableFuture and Future itself. The conversion here should really be F[ListenableFuture[T]] => F[T].

[voropaevp]

good idea. Please check the new implementaion.

[deusaquilus]

That makes sense. Either way works for me.

[deusaquilus]

@voropaevp Thank you for your contribution! In terms of implementation, I think everything here is good. My main concern is with the build. Since we support ~8 different databases, it's extremely important to make sure that all the images play well together and no single image takes too many resources. In the bigdata build matrix we have the Cassandra image, the Orientdb image, and the Spark stuff all run on the same CI node so we need to be extremely careful with the amount of resources that all of these things take.

[djspiewak]

A better approach here would be to get the Async[F].executionContext, then wrap it into an Executor by delegating execute. All uses of global are highly suspect.

[deusaquilus]

This makes sense, @adamgfraser did the same thing in FutureConversions.scala in ListenableFutureConverter.asScala to help with ZIO interop.
Maybe something like this?

val asyncEcExecutor =
  new Executor {
    override def execute(command: Runnable): Unit = Async[F].executionContext.execute(command)
  }

[djspiewak]

See above

[djspiewak]

Suggested change

	import cats.implicits._
	import cats.syntax.all._

[djspiewak]

This seems kind of unnecessary. You don't have to capture the Async[IO], you can just ask for it any time you want it. Unless there's some type witchcraft here that I'm not aware of?

[deusaquilus]

Don't you need it for the new CassandraCeContext below? I imagine you could also pass it explicitly?

    new CassandraCeContext(Literal, c.cluster, c.keyspace, c.preparedStatementCacheSize)(Async[IO]) with CassandraTestEntities with Encoders with Decoders

[voropaevp]

I went with inlining Async[F] to avoid following error:

[error] /app/quill-cassandra-ce/src/test/scala-2.12/io/getquill/context/cassandra/catEffect/package.scala:14:9: diverging implicit expansion for type cats.effect.Async[F]
[error] starting with method asyncForKleisli in object Async
[error]     new CassandraCeContext(Literal, c.cluster, c.keyspace, c.preparedStatementCacheSize) with CassandraTestEntities with Encoders with Decoders
[error]         ^
[error] /app/quill-cassandra-ce/src/test/scala-2.12/io/getquill/context/cassandra/catEffect/package.scala:14:5: type mismatch;
[error]  found   : <error> with io.getquill.context.cassandra.CassandraTestEntities with io.getquill.context.cassandra.encoding.Encoders with io.getquill.context.cassandra.encoding.Decoders
[error]  required: io.getquill.CassandraCeContext[io.getquill.Literal.type,cats.effect.IO]
[error]     new CassandraCeContext(Literal, c.cluster, c.keyspace, c.preparedStatementCacheSize) with CassandraTestEntities with Encoders with Decoders

When using cats.effect.syntax._

[deusaquilus]

Whoa! Make sure to keep this as -SNAPSHOT since we're not doing a release-build yet.

[deusaquilus]

Btw, the latest version is 3.8.1-SNAPSHOT

[mdedetrich]

Yeah I think PR needs to be rebased off of master

[jilen]

Is it possible abstract over the Effect types like Future / IO / ZIO ? instead of encoding multiple times for each.

[deusaquilus]

@jilen It is possible, but the amount of Tagless-Final/Free-Monadic machinery it would take to do that would ultimately be waaay more complex then all of the context-implementations combined. I've tried. I don't think it's worth it.

[deusaquilus]

(Rant warning! This comment has nothing to do with the current PR. Please feel free to skip!)

@jilen The problem is that the operating paradigms of these different DB systems are different enough that we would be building abstractions that literally make no sense half the time. For example, in order to have the flexibility that Future(s) have of passing a ExecutionContext into each map call etc... we would have to encode into a Tagless-Final/Free-Monadic solution as a dummy-implicit for the other contexts corresponding map method. For ZIO where we don't pass any kind of DataSource into the context at all... that would be another dummy argument. When combined, all of these things would create so many no-op placeholders that they would be insanely hard to understand, let alone debug and explain to others.
On top of that, we might end up building machinery that is actually wrong. In the Monix contexts for instance, we rely on Monix's equivalent of thread-locals to store the connection state. In other synchronous contexts we use actual thread locals. In other contexts (e.g. JAsync) we rely on a implicit re-passing pattern. Creating a layer to abstract out all of the details of these things would make it very, very easy to make mistakes in some of them.
Every time I think about this stuff I come to the same conclusion. Quill is not "in the business" of high-level abstracting effects for 8 different databases, 5 different database-protocols, and 3 different effect types (actually if you count NDBC and Finagle it's more like 5). Even if it is actually possible to do it well (and I'm not convinced it is), we don't have the time or resources to do it properly.
Our io.getquill.context.Context might be a "lowest common denominator" of sorts but it's a remarkably stable construct (stable as in not-changing that much over time). The same cannot be said at all about effect-type libraries.

[deusaquilus]

@voropaevp I think we're very close to wrapping this up. Please merge in the latest change (I think it's just version.sbt) and make sure the builds work. I can merge and then push out a release later this week.

[djspiewak]

I think you might have more success just abstracting over ZIO, Cats Effect, and Monix. This is, after all, part of what Cats Effect was designed to accomplish. You do indeed need to build some relatively complex abstractions to solve that problem, but in the case of those three, the abstractions are already built.

Future, on the other hand, and other similar types are a different story. There isn't really any meaningful way of abstracting them and being sure that things still work correctly, because the types in question have relatively weak compositional properties. Since the desire is to maintain compatibility with these unruly constructs, abstraction really is probably out of the question.

[voropaevp]

Zstream, Observable and fs2.Stream will require abstracting over too. And these are further away from each other than Task, IO or ZIO.

[djspiewak]

Zstream, Observable and fs2.Stream will require abstracting over too. And these are further away from each other than Task, IO or ZIO.

That's a good point. And abstracting over those three is pretty much not even a meaningful thing to do. fs2.Stream works with all three ecosystems, but I suspect that (e.g.) Monix users would prefer Observable to Stream, so it's not an entirely helpful compatibility.

[diesalbla]

A few suggestions :)

[voropaevp]

@diesalbla thank you for the suggested improvements. All were included.
Rerun the tests and pushed new squashed commit.

[deusaquilus]

There is one additional change that I need to merge to master before this one that adds the session directly to Prepare. It will require this PR to be changed slightly. Sorry about the delay. Will explain more soon.

[deusaquilus]

Hey guys, sorry about the wait. I just merged what is hopefully the last of a series of large refactorings that needed to be done in order to align Quill with the ProtoQuill API modifications.
Two major things happened:

1. As a result of Pass Session to all Encoders/Decoders allowing UDT Encoding without local session varaible in contexts e.g. ZIO and others #2219, the session (i.e. the context Session type) was added to the function type Prepare (check RowContext that propagates to all other contexts). That means that prepare(prepStatement) invocation in prepareRowAndLog now needs the session variable to be passed in. Now, because CassandraCeContext extends CassandraClusterSessionContext and that extends CassandraSession which extends UdtValueOf, you should be able to do override type Session = CassandraSession directly in CeContext and then change the method to prepare(prepStatement, this) (or define a self-type if you want).
   This change was done so that Cassandra contexts don't necessarily need to pass in the session variable into the context itself since it is now available on the encoder. The Cassandra UDTs were not working in some contexts because of this and still don't work in the lagom contexts. I suggest you copy the code from UdtEncodingSessionContextSpec in io.getquill.context.cassandra.zio and add it to io.getquill.context.cassandra.catEffect (with the appropriate modifications) in order to test that UDTs work in CassandraCeContext.
   (Also this change opens up the possibility of writing Clob/Blob/Text encoders in the future)
2. As a result of Line-up core API with ProtoQuill so child contexts can have same code #2231, an additional two parameters (info: ExecutionInfo, dc: DatasourceContext) have been added to streamQuery, executeQuery, executeAction, executeBatchAction (as well as some others that I don't think this implementation uses). If these parameters are added to the above methods everything should be fine. Also, add type DatasourceContext = Unit to CeContext.
   This change was done so that most ProtoQuill contexts and Quill child contexts can have identical code.

@voropaevp Is there any chance you can merge in the latest code and make the above changes? I know you've already done an amazing amount of work and it's almost embarrassing for me to ask you to do more. If you're willing to make these changes please let me know. If not, also please let me know and I will try to find time to merge them in myself.

[voropaevp]

Will do.

@deusaquilus
Could you maybe advice me on the https://gitter.im/getquill/quill?at=61106d0b3eec6d41d1476982 . Is it possible to achive without macros?

[voropaevp]

Sorry for the mess, intial merge went wrong.
@deusaquilus Please review final commit, all tested and updated according to PR2219.

[deusaquilus]

Hey, will look at this this weekend. Rosh Hashanah and Yom Kippur just happened so I'm on a backlog 😅

[voropaevp]

closed in favour of #2343