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

Typed Distributed Data requires untyped Cluster #25746

Closed
johanandren opened this Issue Oct 5, 2018 · 9 comments

Comments

@johanandren
Copy link
Member

johanandren commented Oct 5, 2018

Since we use the original replicated datatypes and operations on those require a Cluster, would be good if we could have some nicer solution for using them in typed.

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Oct 5, 2018

I have wanted to introduce some cluster independent type and perhaps this is the moment we should. The only thing that is needed in the end is the selfUniqueAddress. UniqueAddress isn't unique enough as type to use as an implicit so we would have to add a new more specific type for this.

I don't know if it's possible to do in a compatible way. In java it's probably ok to add overloaded methods. In scala it's an implicit in second param list so overload will probably not work?

@patriknw patriknw added this to Backlog in Akka Typed Oct 15, 2018

@helena helena self-assigned this Nov 26, 2018

@patriknw patriknw moved this from Backlog to In Progress in Akka Typed Nov 26, 2018

@helena

This comment has been minimized.

Copy link
Member

helena commented Nov 26, 2018

@patriknw can you elaborate a bit on this, the implicit in question?

UniqueAddress isn't unique enough as type to use as an implicit so we would have to add a new more specific type for this.

Thanks

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Nov 27, 2018

Currently the API is based on implicit node: Cluster, which should be the self node, i.e. Cluster(system). E.g. in ORSet:

def +(element: A)(implicit node: Cluster): ORSet[A]

It's only the UniqueAddress that we need in the end, node.selfUniqueAddress.

Implicit parameter types should be rather specific, and I think UniqueAddress itself is not specific enough so we need to introduce another type, something like

final case class SelfUniqueAddress(uniqueAddress: UniqueAddress)

For the non-implicit parameter (Java API) we can use UniqueAddress.

@helena

This comment has been minimized.

Copy link
Member

helena commented Nov 27, 2018

Ah, that implicit, in its current form it is an overly heavy weight pattern and agree that specifying is an improvement. SelfUniqueAddress sounds good.

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Nov 27, 2018

What we are trying to fix is that akka.cluster.Cluster is the untyped Cluster, and that can be confusing since other cluster things are using akka.cluster.typed.Cluster.

There is another challenge here. It's not possible to add an overloaded method that is only different in second parameter list. We can't have:

def +(element: A)(implicit node: Cluster): ORSet[A]
def +(element: A)(implicit node: SelfUniqueAddress): ORSet[A]
@helena

This comment has been minimized.

Copy link
Member

helena commented Nov 27, 2018

👍

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Dec 3, 2018

@helena What do you think about adding overloaded methods replacing the Cluster parameter with SelfUniqueAddress and make that the way we want it to be used for both Typed and Untyped. Deprecating the methods with Cluster.

For the + method we have to use another name, which could be :+.

Something like this:

  def :+(element: A)(implicit node: SelfUniqueAddress): ORSet[A] = add(node, element)
  
  @deprecated("Use `:+` that takes a `SelfUniqueAddress` parameter instead.", since = "2.5.19")
  def +(element: A)(implicit node: Cluster): ORSet[A]

  def add(node: SelfUniqueAddress, element: A): ORSet[A]
  
  @deprecated("Use `add` that takes a `SelfUniqueAddress` parameter instead.", since = "2.5.19")
  def add(node: Cluster, element: A): ORSet[A] = add(node.selfUniqueAddress, element)
@helena

This comment has been minimized.

Copy link
Member

helena commented Dec 6, 2018

@patriknw Thanks, just seeing your suggestion, this looks great - adding.

@helena

This comment has been minimized.

Copy link
Member

helena commented Dec 7, 2018

@helena helena moved this from In Progress to Reviewing in Akka Typed Dec 11, 2018

helena added a commit that referenced this issue Dec 14, 2018

Typed Distributed Data requires untyped Cluster #25746 (#26074)
Typed Distributed Data requires untyped Cluster [#25746](#25746)

@helena helena added this to the 2.5.20 milestone Dec 14, 2018

@helena helena closed this Dec 14, 2018

Akka Typed automation moved this from Reviewing to Done Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment