mdedetrich opened a new pull request, #252: URL: https://github.com/apache/incubator-pekko/pull/252
This PR is the equivalent of https://github.com/akka/akka/pull/31415 but for Pekko, I would recommend reading the conversation thread from that PR to get the context/justification for the feature. The very brief summary is that the `SubstreamCancelStrategy` abstraction that is only used when creating a `SubFlow` (via usage of `splitWhen` and other associated functions) is not needed since Pekko already provides a generic abstraction for this called a `SupervisionDecider`. It can be said that the `SubstreamCancelStrategy` is worse than pointless duplication because because unlike `SupervisionDecider` you can't do things like log the thrown exception in the `SubFlow` (this is actually the main impetus for the change, because as user I had the situation where `SubFlow` was throwing exceptions and I had no idea what was going on because it wasn't logging them via a custom `SupervisionDecider` which I defined, even worse because its not possible to change the already defined `SubstreamCancelStrategy` you have to do annoying workarounds to log any potentially thrown exceptions in a `SubFlow`). Note that this change although not source/binary breaking it does change behaviour because the default `SupervisionDecider` is to cancel in case of an exception where as the default `SubstreamCancelStrategy` default is to ignore and keep on going (this is also the reason behind the test changes). As stated in the original PR, even though it is a breaking behavioural change, since it fails more explicitly its much easier for a user to notice (see https://github.com/akka/akka/pull/31415#pullrequestreview-983031284). Due to this behaviour change the PR was intended to be looked at by @raboof and @jrudolph to get buy in whether this behavioural change is acceptable (see https://github.com/akka/akka/pull/31415#issuecomment-1141293129), would be great to get feedback if the premise of the breaking change in this PR is acceptable (because otherwise we have to create an entirely new set of functions which overall I think is worse proposition). PR is currently draft because I didn't update the docs for the behavioural change. If @jrudolph / @raboof are happy with this change (along with anyone else that has strong experience with akka/pekko-streams, i.e. @He-Pin ) then I will update the PR with more documentation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
