flow icon indicating copy to clipboard operation
flow copied to clipboard

Add naming to flow-generated processes

Open MarcVanOevelen opened this issue 7 years ago • 9 comments

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.

MarcVanOevelen avatar Sep 11 '18 18:09 MarcVanOevelen

Thanks @MarcVanOevelen, i have added some comments and also please be sure to run the code formatter, otherwise the build won't pass. Thanks!

josevalim avatar Sep 11 '18 18:09 josevalim

Only the top level one, yeah!

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

josevalim avatar Sep 11 '18 18:09 josevalim

@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 ?

MarcVanOevelen avatar Sep 11 '18 20:09 MarcVanOevelen

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

josevalim avatar Sep 11 '18 20:09 josevalim

Yes, already tried that but it makes no difference :-(

MarcVanOevelen avatar Sep 11 '18 21:09 MarcVanOevelen

@MarcVanOevelen and @josevalim this PR will be merged?

rafaelwkerr avatar Aug 16 '19 12:08 rafaelwkerr

Sorry for dropping this one. First we need to rebase and we should probably focus this PR on the name changes.

josevalim avatar Aug 16 '19 13:08 josevalim

Hi @MarcVanOevelen. Sorry for the delay, do you still have interest in getting this in? If so, can you please rebase it? Thanks.

josevalim avatar Feb 03 '20 11:02 josevalim

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

a8t avatar Jun 06 '21 14:06 a8t