!str Make flatMapPrefix javadsl using java.util.List

[He-Pin]

This will make it easier to work with in Java. And the scaladsl was seq

[pjfanning]

A 1.1.0 change for me and we should continue to include the Old API and deprecate it.

[He-Pin]

I think it's source compatible(if lambda is being used), and can't have both because they are the same after being erased.

[mdedetrich]

@He-Pin @pjfanning So I just noticed this and its kind of annoying because if we do need to do this then it kind of should be done now but I would like more context as to why this is problematic in Java.

@He-Pin What exactly is the issue here from Java's side in using Iterable? I mean java.util.List might be easier but its also probably too strict of a type and there are other types of collections that aren't List which you would want to use this function with.

Maybe this calls for adding some helper functions to org.apache.pekko.util.Helpers?

[pjfanning]

I think it is too late to do this in 1.0.0. I really thought we were ready for a 1.0.0 RC and to sneak in changes like this and force us to go through even more testing.

I am not too sure we need the change at all - Iterable is an ok return type - I don't see the major benefit and then we have the cost of the API disruption.

[mdedetrich]

Yeah I want to avoid sneaking this in as well unless it really has ramifications further down the line which is why I want context as to what problem we are solving. If by "easer to work in Java" we just mean saving some lines of code when converting from java.util.List to java.lang.Iterable within Java then such a solution creates more problems for the reasons stated earlier.

[He-Pin]

@mdedetrich With List type, it can gives you more methods to be accessible.

i come up it with real production code which i have to do a cast instead.

And as this is not a binary compatible change after release of 1.0.0

[He-Pin]

And this is another reason.

[pjfanning]

I am very much against a 1.0.0 change. I don't think this change justifies delaying the release. It is an API change and I think the majority of Pekko community users just want an Akka like release.

[He-Pin]

I have to say it's a mistake when it's Java.lang.Iterable, it should be Java.util.List instead as what we said in the contribution guide.

And if it's not landing in 1.0.0, it may never have a chance anymore.

[He-Pin]

@mdedetrich It instead liberating the usage on Java side.

And prefixAndTail is using java.util.List too, just as the contribution guide.

[pjfanning]

I have to say it's a mistake when it's Java.lang.Iterable, it should be Java.util.List instead as what we said in the contribution guide.

And if it's not landing in 1.0.0, it may never have a chance anymore.

We've had months to discuss. Why choose the absolute last minute before the release to discuss this?

Akka users have used the existing API for years. I do not understand why this is so important to change in Pekko.

The Pekko community seemed to have an agreement to try to get an Akka like release done and to follow up with bigger changes in 1.1.0.

We really have to release soon. The Akka CVEs that they are refusing to open source means that there are Akka users who want a quick Pekko release so that they can use a CVE-free alternative.

[He-Pin]

This change will not affect who using lamda, but those who using Function Constant.

If it has to take long time to discuss, i think we can defer it to next major version, but it still a inherented mistake with using Java.lang.Iterable.

[pjfanning]

This change will not affect who using lamda, but those who using Function Constant.

If it has to take long time to discuss, i think we can defer it to next major version, but it still a inherented mistake with using Java.lang.Iterable.

I would support a 1.1.0 change. There are other places in the code that use Function of Iterable and they should be considered to be changed too - I found 6 and this PR just changes 2.

[He-Pin]

@pjfanning PLS reschedule it, i'm away from computer.

[pjfanning]

[mdedetrich]

If we do breaking API changes that would need to be 2.0.0 according to semver. The easiest option would be to come up with a different method name and deprecate the older one.

[mdedetrich]

@pjfanning On this point, should I make a thread on the mailing list talking about semver? I did PR's in the projects setting the versioning scheme of the projects to semver but it sounds a lot of people are not aware of this (either by what semver means or the fact that we follow semver at all).

[pjfanning]

@mdedetrich I'd like to keep the mailing threads focused on the release - semver can be discussed at any time.

I would be shocked if many people don't expect us to follow semver principles.

I think this issue can be fixed at any time by adding new methods and not trying to change the existing ones. A new method could be called flatMapPrefixWithList instead of flatMapPrefix (or some other name, this is just a suggestion).

[mdedetrich]

@mdedetrich I'd like to keep the mailing threads focused on the release - semver can be discussed at any time.

@pjfanning The thing is, semver is very black and white since we have https://github.com/apache/incubator-pekko/blob/main/build.sbt#L17. Thats why I am bringing this up, if you have ThisBuild / versionScheme := Some(VersionScheme.SemVerSpec) set in sbt, you MUST follow semver because that setting is literally instructs sbt to refuse resolving dependencies that break semver (i.e. you resolve pekko 1.1 and pekko 2.1 in the same project).

The reason why I set that is that Akka follows semver which Pekko was meant to follow and this setting makes it official.

I think this issue can be fixed at any time by adding new methods and not trying to change the existing ones. A new method could be called flatMapPrefixWithList instead of flatMapPrefix (or some other name, this is just a suggestion).

If we decide to do this then its fine (flatMapPrepend is probably a better alternative name), but if we instead decide to change the existing method then we either have to frontload these change/s before release OR remove ThisBuild / versionScheme := Some(VersionScheme.SemVerSpec) and don't claim we follow semver. On that note, not following semver is a serious decision because you are basically stating you can potentially break the Pekko ecosystem at any time. While having a Pekko 2.x at some point (which is what allows for a breaking change) is probably going to happen at some time, it should not be done lightly.

There are other points that you raised like pekko-connectors, i.e. there is an argument that pekko-connectors probably shouldn't follow semver because of how frequently they change and more importantly how historically frequent those changes have technically broken ABI

[pjfanning]

pekko-connectors has docs stating that it is subject to change.

pekko core libs have been stable for a long time. We should enable the mima filter checks soon after the 1.0.0 release. Any time we have to add an exception to the mima checks is a very very sad day and should be signed off by the community. And the next release number should be controlled by those binary compatible changes. Any major breakages mean that we need a major release.

[mdedetrich]

pekko-connectors has docs stating that it is subject to change.

In that case ThisBuild / versionScheme := Some(VersionScheme.SemVerSpec) should be removed from pekko-connectors, is there any other module that documents this?

pekko core libs have been stable for a long time. We should enable the mima filter checks soon after the 1.0.0 release.

That means we are going to follow semver.

Any time we have to add an exception to the mima checks is a very very sad day and should be signed off by the community. And the next release number should be controlled by those binary compatible changes. Any major breakages mean that we need a major release.

In that case we either change the function now or add another method name, the latter being much more appealing since it also provides an upgrade path.

Regarding new method name, I think that flatMapTakeAndApply` is a good candidate, and is arguably better than the current name in describing what the function actually does.

[pjfanning]

I'm begging - can we stop discussing adding a change for this now? We can add a change later - and one that doesn't break binary or source compatibility.

The community wants a release.

This is not a critical issue. I have never come across a scenario where minor issues are turned into release blockers at the very last minute before a release.

We can do a v1.0.1 with an additional API in a few months.

[mdedetrich]

I'm begging - can we stop discussing adding a change for this now? We can add a change later - and one that doesn't break binary or source compatibility.

I never said we should add the change now. I am saying we should be clear about semver, that's it. And the discussion was needed because adding semver to pekko-connectors was a mistake, one that could have been easily forgotten and create a lot of harm/chaos to users.

It was suggested earlier to add a breaking change in 1.1.0, I am saying we should not do that.

I came up with alternate names if @He-Pin wants to update the PR with an added method and if he follows that course of action its obviously not a priority, it can be done whenever.

[pjfanning]

With semver, we may want a mail thread but can we exclude pekko and pekko-http? I think it is fair to say that those 2 libs are very stable and the whole community expects strict semver. These projects are also used by many downstream libs so binary incompatible changes will have big negative impacts.

There are questions about the rest of the Pekko repos. You could argue that we impose strict semver now and ramp up version numbers if we have to make binary incompatible changes in future releases. Or we say that 1 or 2 of the repos are still unstable and we can't impose strict semver on them. All in all, binary incompatible changes outside of pekko and pekko-http will have more limited impacts that those for pekko and pekko-http.

[mdedetrich]

With semver, we may want a mail thread but can we exclude pekko and pekko-http? I think it is fair to say that those 2 libs are very stable and the whole community expects strict semver. These projects are also used by many downstream libs so binary incompatible changes will have big negative impacts.

I agree with that, possible pekko-grpc ontop of that as well

There are questions about the rest of the Pekko repos. You could argue that we impose strict semver now and ramp up version numbers if we have to make binary incompatible changes in future releases. Or we say that 1 or 2 of the repos are still unstable and we can't impose strict semver on them. All in all, binary incompatible changes outside of pekko and pekko-http will have more limited impacts that those for pekko and pekko-http.

I am making a PR to remove semver from pekko-connectors because after skimming through the history, its kind of delusional for it to ever realistically follow semver due to the fact that it supports so many artifacts (via different connectors) inside a single mono repo with a single version (if it did follow semver it would break the major so often that the semver becomes pointless). There is an argument to be made here whether we should gradually split out the connectors into their own versions/repos, but not one that should be had now especially with the amount overhead separate git repos create. As you pointed out, we have bigger fish to fry (i.e. a release).

With the other repos we can evaluate on a case by case basis, a lot of them aside of connectors seem quite stable so the risk is lower but it should be looked into properly.

[He-Pin]

@pjfanning @mdedetrich Can we do this in the 1.1.0 version?

[laglangyue]

Even though the function parameter is an iterable, we passed a Seq while invoking it,and I checked the relevant tests and found that they were all list . Perhaps users are more accustomed to using lists,or this could be a design errors in the API.

[pjfanning]

I remain -1 on changing the request signature of the method. We can add new methods and deprecate the old ones.

[He-Pin]

What about the new method name? switchOnFirst , prefixFlatmap?

[pjfanning]

What about the new method name? prefixAndFlatmap?

Thinking about it again, it might be worth keeping the flatMapPrefix without deprecating it - users of Sets and classes like that might still want to use the method. How about calling the new method flatMapPrefixList to make it more obvious that it is related to flatMapPrefix?

[He-Pin]

@pjfanning *List is not a good name I think, we can come up a better name or keep it as it is and only change it when we can.

[pjfanning]

@mdedetrich do you have any preferences for function names here? My preference is have something similar to the existing API name (flatMapPrefix). I think it would be confusing to use a different name for the new functions.

[mdedetrich]

flatMapListPrefix?

[He-Pin]

I think this change should be binary compatible too, as the function is erased to function[any,any]

  // access flags 0x1
  // signature <Out2:Ljava/lang/Object;Mat2:Ljava/lang/Object;>(ILorg/apache/pekko/japi/function/Function<Ljava/lang/Iterable<TOut;>;Lorg/apache/pekko/stream/javadsl/Flow<TOut;TOut2;TMat2;>;>;)Lorg/apache/pekko/stream/javadsl/Flow<TIn;TOut2;TMat;>;
  // declaration: org.apache.pekko.stream.javadsl.Flow<In, Out2, Mat> flatMapPrefix<Out2, Mat2>(int, org.apache.pekko.japi.function.Function<java.lang.Iterable<Out>, org.apache.pekko.stream.javadsl.Flow<Out, Out2, Mat2>>)
  public flatMapPrefix(ILorg/apache/pekko/japi/function/Function;)Lorg/apache/pekko/stream/javadsl/Flow;
    // parameter final  n
    // parameter final  f
   L0
    LINENUMBER 2260 L0
    ALOAD 0
    GETFIELD org/apache/pekko/stream/javadsl/Flow.delegate : Lorg/apache/pekko/stream/scaladsl/Flow;
    ALOAD 2
    INVOKEDYNAMIC apply(Lorg/apache/pekko/japi/function/Function;)Lscala/Function1; [
      // handle kind 0x6 : INVOKESTATIC
      java/lang/invoke/LambdaMetafactory.altMetafactory(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
      // arguments:
      (Ljava/lang/Object;)Ljava/lang/Object;, 
      // handle kind 0x6 : INVOKESTATIC
      org/apache/pekko/stream/javadsl/Flow.$anonfun$flatMapPrefix$1(Lorg/apache/pekko/japi/function/Function;Lscala/collection/immutable/Seq;)Lorg/apache/pekko/stream/scaladsl/Flow;, 
      (Lscala/collection/immutable/Seq;)Lorg/apache/pekko/stream/scaladsl/Flow;, 
      5, 
      1, 
      (Lscala/collection/immutable/Seq;)Lorg/apache/pekko/stream/scaladsl/Flow;
    ]
    ASTORE 5
    DUP
    IFNONNULL L1
    ACONST_NULL
    ATHROW
   L1
   FRAME FULL [org/apache/pekko/stream/javadsl/Flow I org/apache/pekko/japi/function/Function T T scala/Function1] [org/apache/pekko/stream/scaladsl/Flow]
    ASTORE 4
   L2
    NEW org/apache/pekko/stream/impl/fusing/FlatMapPrefix
    DUP
    ILOAD 1
    ALOAD 5
    INVOKESPECIAL org/apache/pekko/stream/impl/fusing/FlatMapPrefix.<init> (ILscala/Function1;)V
    ASTORE 6
   L3
    ALOAD 4
    ALOAD 6
    INVOKEVIRTUAL org/apache/pekko/stream/scaladsl/Flow.via (Lorg/apache/pekko/stream/Graph;)Lorg/apache/pekko/stream/scaladsl/Flow;
   L4
    ACONST_NULL
    ASTORE 6
   L5
    ACONST_NULL
    ASTORE 4
    ACONST_NULL
    ASTORE 5
    ASTORE 3
   L6
    LINENUMBER 2261 L6
    NEW org/apache/pekko/stream/javadsl/Flow
    DUP
    ALOAD 3
    INVOKESPECIAL org/apache/pekko/stream/javadsl/Flow.<init> (Lorg/apache/pekko/stream/scaladsl/Flow;)V
    ARETURN
   L7
    LOCALVARIABLE newDelegate Lorg/apache/pekko/stream/scaladsl/Flow; L6 L7 3
    LOCALVARIABLE this Lorg/apache/pekko/stream/javadsl/Flow; L0 L7 0
    LOCALVARIABLE n I L0 L7 1
    LOCALVARIABLE f Lorg/apache/pekko/japi/function/Function; L0 L7 2
    LOCALVARIABLE flatMapPrefix_this Lorg/apache/pekko/stream/scaladsl/Flow; L2 L5 4
    LOCALVARIABLE flatMapPrefix_f Lscala/Function1; L2 L5 5
    LOCALVARIABLE via_flow Lorg/apache/pekko/stream/Graph; L3 L4 6
    MAXSTACK = 4
    MAXLOCALS = 7