flow
flow copied to clipboard
Add naming to flow-generated processes
Implements stage naming as discussed in 'Naming stages #48' Optional automatic generated unique names, effective when the flow is explicitly named Automatic stage names can be overruled using :name option. See the module doc for details.
Thanks @MarcVanOevelen, i have added some comments and also please be sure to run the code formatter, otherwise the build won't pass. Thanks!
Only the top level one, yeah!
José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D
@josevalim , I also wanted to include a test for the exception case when specifying a partition name on an unnamed flow but couldn't find a way to suppress the GenServer error log. My test looks as follows :
test "error when naming stage for unnamed flow" do Process.flag(:trap_exit, true) message = "a partition is named ':output', this requires the flow to be named" {:error, reason} = Flow.from_enumerable([1,2,3]) |> Flow.map(&(&1 + 1)) |> Flow.partition(stages: 3, name: :output) |> Flow.reduce(fn -> 0 end, &(&1 + &2)) |> Flow.start_link() assert {%ArgumentError{message: ^message}, _} = reason end
The test passes but the GenServer termination error log is printed :
2:28:21.176 [error] GenServer #PID<0.158.0> terminating ** (ArgumentError) a partition is named ':output', this requires the flow to be named (flow) lib/flow/materialize.ex:71: Flow.Materialize.start_stages/7 (flow) lib/flow/materialize.ex:22: Flow.Materialize.materialize/5 (flow) lib/flow/coordinator.ex:28: Flow.Coordinator.init/1 (stdlib) gen_server.erl:365: :gen_server.init_it/2 (stdlib) gen_server.erl:333: :gen_server.init_it/6 (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3 Last message: {:EXIT, #PID<0.157.0>, {%ArgumentError{message: "a partition is named ':output', this requires the flow to be named"}, [{Flow.Materialize, :start_stages, 7, [file: 'lib/flow/materialize.ex', line: 71]}, ...
Any idea how to fix this ?
You need to put @tag :capture_log before the test.
José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D
Yes, already tried that but it makes no difference :-(
@MarcVanOevelen and @josevalim this PR will be merged?
Sorry for dropping this one. First we need to rebase and we should probably focus this PR on the name changes.
Hi @MarcVanOevelen. Sorry for the delay, do you still have interest in getting this in? If so, can you please rebase it? Thanks.
I rebased this out of curiosity as a way to dig into the internals, but I did something wrong in Git and the commits are attributed to me... if that's OK, I can re-open the PR. Though I'm sure if it's that wise, considering I don't have the technical ability to really own the PR.
@josevalim lmk?
this is what a PR from my rebased branch to current master woudl look like: https://github.com/a8t/flow/pull/1/files