-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add Flow.LazyInitAsync, and fix materialized value of Sink.LazyInit #5476
Conversation
I must add the documentation before this can be merged. |
@ismaelhamed did my new pull request template pop up when you submitted this? Still trying to see if I set that up correctly last week. |
Yes. Would you prefer it if I follow the template strictly? |
I think it would be helpful going forward but I also need feedback on it to see if it actually is helpful to both reviewers and contributors. You're an experienced contributor so I would welcome any critical feedback you have on them. Best place to do it is probably in our meta-repo where the PR templates are defined: https://github.com/akkadotnet/.github |
bccab5d
to
7f97640
Compare
@Aaronontheweb documentation added, and PR template updated. This is ready for review. |
7f97640
to
782a539
Compare
Looks like you have some unrelated unit tests failing - will take a look at this PR today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1910,6 +1911,10 @@ namespace Akka.Streams.Dsl | |||
public static Akka.Streams.Dsl.Sink<TIn, System.Threading.Tasks.Task> Ignore<TIn>() { } | |||
public static Akka.Streams.Dsl.Sink<TIn, System.Threading.Tasks.Task<TIn>> Last<TIn>() { } | |||
public static Akka.Streams.Dsl.Sink<TIn, System.Threading.Tasks.Task<TIn>> LastOrDefault<TIn>() { } | |||
public static Akka.Streams.Dsl.Sink<TIn, System.Threading.Tasks.Task<Akka.Util.Option<TMat>>> LazyInitAsync<TIn, TMat>(System.Func<System.Threading.Tasks.Task<Akka.Streams.Dsl.Sink<TIn, TMat>>> sinkFactory) { } | |||
[System.ObsoleteAttribute("Use LazyInitAsync instead. LazyInitAsync no more needs a fallback function and th" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -4188,6 +4193,17 @@ namespace Akka.Streams.Implementation.Fusing | |||
protected override Akka.Streams.Stage.GraphStageLogic CreateLogic(Akka.Streams.Attributes inheritedAttributes) { } | |||
} | |||
[Akka.Annotations.InternalApiAttribute()] | |||
public sealed class LazyFlow<TIn, TOut, TMat> : Akka.Streams.Stage.GraphStageWithMaterializedValue<Akka.Streams.FlowShape<TIn, TOut>, System.Threading.Tasks.Task<Akka.Util.Option<TMat>>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Currently, we have
LazySource
andLazySink
. This PR adds the missingLazyFlow
and theFlow.LazyInitAsync
stage that builds on top of it.Changes
Also,
Sink.LazyInit
materialized value is a now wrapped in aTask
to clearly indicate whether the internal sink was materialized or not. It is marked deprecated in favor of the newSink.LazyInitAsync
, aligned withFlow.LazyInitAsync
.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
Latest
dev
BenchmarksDon't apply.
This PR's Benchmarks
Don't apply.