Geode: become Alpakka 1.0 ready

akka / alpakka

Alpakka is a Reactive Enterprise Integration library for Java and Scala, based on Reactive Streams and Akka.

https://doc.akka.io/docs/alpakka/current/

Other

1.26k stars 645 forks source link

Geode: become Alpakka 1.0 ready #1073

Closed ennru closed 5 years ago

ennru commented 6 years ago

[x] Move internal code to impl package (#1096)
[x] Mark internal code @InternalApi and private[connector] (#1096)
[x] Move documentation example code to packages docs.javadsl and docs.scaladsl (#1096)
[x] Make settings to be non-case classes (#1096)

cheleb commented 6 years ago

Should I fix all those point in a single commit ?

ennru commented 6 years ago

Split it in several PR if you like to do it in small steps, but one PR is OK, as well.

2m commented 6 years ago

While reviewing #1096 I noticed a couple of other changes that should be done, to make Geode connector more inline with the others:

[ ] change ReactiveGeode to be a collection of factory methods instead of a class that needs to be instantiated. That will mean that cache and close methods will have to move to stages themselves.
[ ] convert Scala types to corresponding Java ones in javadsl factory methods. For example here.

cheleb commented 6 years ago

Sorry for the late reply. But AFAIK having more then one (active) cache per jvm is not that easy (https://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/client/ClientCacheFactory.html#create--) I will try to investigate further.

cheleb commented 6 years ago

Is there a chance that akka extension mechanism should fit to hold the Geode cache (aka connection) ?

2m commented 6 years ago

We use Akka Extensions in some connectors (reference as an example, dynamodb, google-cloud-pub-sub-grpc, ...) to hold resources that might be shared across instances of Sources, Flows and Sinks.

However Akka Extension allows to have a resource per ActorSystem. If that is fine for the Geode cache, then yes, we could use it. But you mentioned that cache is per JVM. Then I think we should leave it in some static field on an uninstantiable class.