search

dashbitco / broadway_dashboard

Keep track of your Broadway pipelines from Phoenix LiveDashboard
https://elixir-broadway.org/
Apache License 2.0
213 stars 17 forks source link

Fix crash when named using `:via` #14

Closed feng19 closed 3 years ago

feng19 commented 3 years ago

The PR #13 didn't test scene what named using :via, It will crash when in use.

I'm sorry about that, please help to review it again.

josevalim commented 3 years ago

@feng19 I am thinking we will have to change our strategy here altogether. Instead of having the tab name be {:via, ...}, we should probably call GenServer.whereis(name), whenever the name is not an atom, and work with PIDs instead. This way we don't need to encode name and so on, we only take care of working with PIDs. WDYT?

feng19 commented 3 years ago

I haven't really latched on to what you mean could you explain it again?

josevalim commented 3 years ago

@feng19 all of the Broadway pipelines are actually processes behind the scenes, so all pipelines can also be identified by a pid I think? If so, instead of working with their names, such as {:via, ..., ...}, we should call GenServer.whereis on the :via and get its PID instead. Then we will show its PID on the tab names and work the PID throughout the dashboard (except for atoms, which we can keep as atoms).

feng19 commented 3 years ago

Because of the restriction of Broadway.topology(pipeline), only the name of pipeline can be passed. So all the places that use Broadway.topology/1 cannot be replaced.

Therefore, what can be done now is to change the value of nav & nav tab without changing Broadway.topology/1.

josevalim commented 3 years ago

I see, thanks. I will drop just two small notes. Please check if they work and then we can ship it. :)

josevalim commented 3 years ago

:green_heart: :blue_heart: :purple_heart: :yellow_heart: :heart: