Luigi: Recode the visualizer

Created on 7 Jul 2017  路  14Comments  路  Source: spotify/luigi

The current visualizer si functional but might be:

  • More performant
  • Real-time
  • Sexiest (Visually)
  • Scalable
  • Testable

That's why I propose a complete recode using React.

I wanted to propose this idea here before code it and send a pull request in order to:

  • Get the opinion of the maintainers.
  • Ask if the community has requests or needs for this new visualizer.

Thanks :)

wontfix

Most helpful comment

@QuentinBrosse nice work on that first commit merged! I love that you start small and get something merged. Looking forward to more good stuff. :)

All 14 comments

I completely agree with Quentin regarding React and real time UI update. This would be very nice.

Sounds great. Some input:

In short: Go for it! :)

I think the general idea from the discussion in #1622 was that an incremental approach would be good for getting real-time updates (especially given that this might be a bad thing for some users). That said, when I was looking into this about a month ago, I recall that it seemed to me like quite a bit of work was going to be involved to rearrange some of the parts of the backend to do this. This applies even more so to @QuentinBrosse's suggestions. So this means there are two alternative approaches:

  1. Update the frontend using the existing backend (apart from minor tweaks here and there) before reworking the backend to better accommodate real-time updates on the frontend.
  2. Rework the backend first, keeping in mind what the next generation frontend will be like.

The first approach seems more sensible to me. If I recall correctly, one of the complications with the existing backend was how the templates were being used (I forget the details, but I was originally going to do something with the templates to add rudimentary support for autorefresh and unfortunately I haven't had the time to devote to this). If the idea is to move to a frontend framework that does most of the job of the template engine, then this problem mostly goes away, which could minimize necessary backend changes.

@mivade "React is flexible and can be used in a variety of projects. You can create new apps with it, but you can also gradually introduce it into an existing codebase without doing a rewrite."

You can find more information about the subject here.

If it is possible possible with the existing codebase we can imagine to:

  1. Update certain real-time parts of the frontend using the existing backend (apart from minor tweaks here and there) and React

    1. TaskList View

    2. Dependency Graph

2. After having recode these parts, we will know exactly what we need to improve realtime front-end (needed routes, their payloads, types of websocket messages, etc.). So we will be able to rework the backend with websocket for example.

But, if you think we can have a precise idea of who to rework the backend now, it seems effectively to be a better approach. We can:

  1. Rework entirely the backend
  2. Recode entirely the frontend whit React (and the templates probl猫mes will be resolved)

What do you think ?

As a maintainer I would in general prefer that you contribute the "green+red" patches first. Basically refactoring/recoding the existing functionality rather than introducing new features. I'm more hesitant to merge pull requests with only code additions, as I'm always afraid that a new enthusiastic contributor suddenly disappears, and we're suddenly left with even more features to maintain. It would be nice if we got the tests to work, I prioritize that above real-time features.

@Tarrasch Do you speak about the tox -e visualiser command ? :)

@QuentinBrosse well yea, I would really like if the frontend to be tested somehow (also on Jenkins). I'm not sure what tox -e visualiser is testing.

I'm not an expert on frontend testing by any means, but there are plenty of Javascript testing frameworks (Facebook seems to use Jest for React). That would at least allow unit testing of basic functionality. Then if you want to test UI stuff, there are things like Selenium.

@mivade oh yea, this is definitely a matter of doing rather than asking if it's doable. :)

@Tarrasch tox -e visualiser do all of this tests:
PhantomJS Tests

[ OK ]  failed_info_test
[ OK ]  done_info_test
[ OK ]  upstream_failure_info_test
[FAIL]  result_count_test
[FAIL]  filtered_result_count_test1
[FAIL]  filtered_result_count_test2
[FAIL]  filtered_result_count_test3
[FAIL]  filtered_result_count_test4
[FAIL]  filtered_result_count_test5
[FAIL]  filtered_result_count_test5
[FAIL]  filtered_result_count_test5
[FAIL]  filtered_result_count_test5
[FAIL]  searched_result_count_test1
[FAIL]  searched_result_count_test1

PhantomJS Tests

test_keeps_entries_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_filter_on_server_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_hide_done_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_invert_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_order_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_svg_visualisation_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... FAIL
test_keeps_table_filter_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_task_id_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_synchronizes_fields_on_graph_tab (visualiser.visualiser_test.TestVisualiser) ... FAIL
test_synchronizes_fields_on_tasks_tab (visualiser.visualiser_test.TestVisualiser) ... ok

It would be nice if we got the tests to work, I prioritize that above real-time features.
Many of these tests apparently have probl猫mes.

Yes, Jest + Selenium + React is a perfect combo for a testable modern web application. (Particularly with the state tree of Redux.) :)

So I think we can fix the 13 failing actual tests before begin the React part if you prefer ?

@QuentinBrosse That would be a great start. In addition to check that there are not spurious failures (which caused me to disable the tests before).

@QuentinBrosse nice work on that first commit merged! I love that you start small and get something merged. Looking forward to more good stuff. :)

@Tarrasch So, it's now time to rework the backend in order to include websocket (with Tornado) or you want to start with something else ?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If closed, you may revisit when your time allows and reopen! Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Alex-picwell picture Alex-picwell  路  5Comments

florian-niefind picture florian-niefind  路  7Comments

leafjungle picture leafjungle  路  7Comments

gioelelm picture gioelelm  路  5Comments

stephenpascoe picture stephenpascoe  路  4Comments