D3visualizer #920

hadesbox · 2015-04-29T11:35:32Z

We have added a new visualization graph with d3, instead of the svg. The svg was not deleted, so you can choose to use between d3 or svg either by using either

[core]
visualization_graph: svg
or
visualization_graph: d3

The visualization_graph defaults to svg, if the option is not present. In any case you can directly acces the d3 or svg visualizations by going in your browser to

svg -> http://localhost:8082/static/visualiser/index.html

d3 -> http://localhost:8082/static/visualiser/index.d3.html

The new visualization in d3 is much better for bigger graphs, since it auto zooms in the canvas and fits it into the page (without scrolling).

I'm also senfing a new foo.comple.py example, which generates a bigger graph (for testing purposes).

Finally, the d3visualizer branch is update with todays last commit in the official master branch so its should merge smoothly.

cheers!

Luis.

Tarrasch · 2015-04-29T12:10:20Z

Wow, 25'000 additions? You also seem to have added that 12000-line file twice. Hmm :)

erikbern · 2015-04-29T12:41:43Z

Nice!!

Do you have any screen shots to share?

gpoulin · 2015-04-29T12:51:21Z

I don't know if it would be possible to add management of the js dependencies without complexifying the the installation.

hadesbox · 2015-04-29T13:01:44Z

Hi Arash!

You are right... we probably did not prepare properly the pull request, but let me explain my self.

We create this new visualization for the graphs on d3, and today when we merge with the HEAD of master, we realized that many changes were applied on the last week to the svg graph, mainly performance improvements, also on the forum themalkom suggested that he wanted to keep using current svg, so since there is not stateful backend for luigi's front to store this options, we thought the best was to keep both the SVG and tthe D3 visualizations and that each user can chose which one he wants or likes more. This is the reason why we wanted to keep separated all index.html, visualizerApp.js in separate files (since we modify them).

Regarding all those 25k lines I guess most of them are from the d3 static files css+js, we thought we could use some public cdn to fetch them, but since luigi is most likely deployed in a DMZ (at least in our case.. I work for a bank) with no outbound internet connection we discarded this, instead we decided to include them in the commit.

If you guys can run the branch https://github.com/hadesbox/luigi/tree/d3visualizer branch in our fork and let us know any comments or suggestion we can work, we will fix them.

We tried to have an option on the URL to switch between d3 and svg, but since the URL is the way luigi identifies which graph to show, it was going to be a huge change and we wanted to avoid it.

Here they are Eric, its the same example (foo.complex.py) on both svg and d3 that we sent on this pull request so you can compare.

http://imgur.com/a/XuhOG

cheers!

hadesbox · 2015-04-29T13:23:41Z

BTW you are totally correct Arash, luigi/static/visualiser/js/dagre-d3.js is dupped, will check that.

gpoulin · 2015-04-29T13:26:34Z

@hadesbox Is the many difference between index.html and index.d3.html. Would it be easy to handle this case with template inheritance? Similar for visualiserApp.d3.js.

Tarrasch · 2015-04-29T13:27:12Z

Yea, that was what I was refering too. ^^

Anyway. I'll review this soon when I'm done with some other things. I'm really happy to see this being submitted as a PR. I'm fine with a 10k lines of code file being in this PR. As long it's not two identical files like that. :)

erikbern · 2015-05-05T11:04:48Z

not sure why tests are broken – might have to rebase this one?

hadesbox · 2015-05-05T13:00:55Z

Aparently there are some quality code problems/warnings in the foo.complex example I added, I'll be sure to fix those:

examples/foo.complex.py:17:1: W293 blank line contains whitespace
examples/foo.complex.py:22:1: W293 blank line contains whitespace
examples/foo.complex.py:24:1: W293 blank line contains whitespace
examples/foo.complex.py:28:1: W293 blank line contains whitespace
examples/foo.complex.py:29:1: E302 expected 2 blank lines, found 1
examples/foo.complex.py:30:1: W293 blank line contains whitespace
examples/foo.complex.py:33:1: W293 blank line contains whitespace
examples/foo.complex.py:36:27: E226 missing whitespace around arithmetic operator
examples/foo.complex.py:37:26: E225 missing whitespace around operator
examples/foo.complex.py:39:1: W293 blank line contains whitespace
examples/foo.complex.py:40:1: E302 expected 2 blank lines, found 1
examples/foo.complex.py:42:1: W293 blank line contains whitespace
examples/foo.complex.py:46:1: W293 blank line contains whitespace
examples/foo.complex.py:49:1: W293 blank line contains whitespace
examples/foo.complex.py:52:34: E226 missing whitespace around arithmetic operator
examples/foo.complex.py:53:30: E225 missing whitespace around operator
examples/foo.complex.py:55:1: W293 blank line contains whitespace
examples/foo.complex.py:59:1: W293 blank line contains whitespace
examples/foo.complex.py:65:1: W293 blank line contains whitespace
examples/foo.complex.py:66:1: W293 blank line contains whitespace
examples/foo.complex.py:70:1: W293 blank line contains whitespace
examples/foo.complex.py:71:70: W292 no newline at end of file
´´´
before resending the PR.

hadesbox · 2015-05-05T13:01:59Z

btw, I rebased with master on 29th April (before sending the PR), I'll rebase again as needed.

Tarrasch · 2015-05-05T14:25:59Z

Also, make sure the remove the duplicate 12000 line file. :)

hadesbox · 2015-05-05T15:12:10Z

Sure Arash. I'm waiting to hear some feedback from you guys before sending another pull request, so if you can try the visualizer and let me know your thoughts.

Or should I send the PR directly??

Tarrasch · 2015-05-05T15:26:27Z

Just update this PR I think. I'll try it out then and come with some feedback. I'm in particular interested in seeing if it's renders faster than the svg implementation. :)

…ble d3 graph you need to set in the config visualization_graph in the [core] section either svg (defaults) or d3. This will tell tornado to return as default home either visualization, in any case regarless of this config param value, you can always to either /static/visualiser/index.html or /static/visualiser/index.d3.html in your browser.

hadesbox · 2015-05-11T11:23:42Z

So I removed the dupped file (-11k lines), rebased again to latest master, fixed the edge labels to say "secs" and fixed all python-codestyle rules that poped on the last travis, lastely I retested that everything worked as expected. So the PR its ready, let me know if you find anything that should be changed/fixed :).

Tarrasch · 2015-05-11T13:18:35Z

luigi/server.py

@@ -214,7 +214,11 @@ def get(self, path):
 class RootPathHandler(tornado.web.RequestHandler):

    def get(self):
-        self.redirect("/static/visualiser/index.html")
+        visualization_graph = configuration.get_config().get('core', 'visualization_graph', '')


Can you use the new syntax instead? core().visualization_graph I mean, and set the parameter to have a default of "svg" maybe?

On the other hand this is in the server code. Maybe it's good to not intermix with the client logic.

I didn't understood what you mean about

On the other hand this is in the server code. Maybe it's good to not intermix with the client logic.

The server.py was changed so Tornado knows what is the default url to serve index.html or index.d3.html, and this is read from the configuration so there is no client code here I think, actually regarless the configuration of the visualization_graph you can visit in your browser both

localhost:8082/index.html or localhost:8082/index.d3.html.

I will change the code().visualization_graph as you suggested, dont know what else to do here maybe you can suggest some ideas?

Let me know (or post screenshots) of your super mega diagram and let me know if it worked fine :).

Ah, nah. I just meant that configuration.get_config().get('core', 'visualization_graph', '') can be replaced with core().visualization_graph. But that's probably a bad idea as then the server needs to import the client code. Which doesn't make sense. :)

Ignore me :)

Tarrasch · 2015-05-11T13:19:17Z

LGTM. :)

hadesbox · 2015-05-11T14:17:53Z

@Tarrasch , Im trying to change it like this

     def get(self):
-        visualization_graph = configuration.get_config().get('core', 'visualization_graph', '')
+        visualization_graph = configuration.core().visualization_graph 
+
         if visualization_graph == "d3":

can you suggest me a code snippet? I can't find the rest of luigi's code the "new way" to fetch the config params (probably I'm not looking into the right place).

… and see the deps of a particular node (and not the whole tree).

hadesbox · 2015-05-11T14:23:56Z

I pushed a thing we have missed, which is the an click event on the d3 graphs, so you can navigate your graph, once travis finishes I thinks its good to go.

Tarrasch · 2015-05-11T14:45:20Z

examples/foo.complex.py

+            yield Bar(i)
+
+
+class Bar(luigi.Task):


For reasons, can you add task_namespace = 'examples' to Foo and Bar?

max_depth, max_total_nodes, current_nodes are global variables, to store the STATE of the graph that is going to get built. The idea is that you can specify by parameters the size and limits of the foo graph. This means that this variables are needed globally within each luigi's task (requires and run) so the graph generation has a stop condition, while been dynamically generated on each run.

Tarrasch · 2015-05-11T14:58:26Z

Can you use another section instead of [core]? I Mean, there is one called [scheduler].

Actually, do it the proper way, add a parameter here:

luigi/luigi/scheduler.py

Lines 79 to 100 in eab3c4c

    
           class scheduler(Config): 
        
               # TODO(erikbern): the config_path is needed for backwards compatilibity. We should drop the compatibility 
        
               # at some point (in particular this would force users to replace all dashes with underscores in the config) 
        
               retry_delay = parameter.FloatParameter(default=900.0) 
        
               remove_delay = parameter.FloatParameter(default=600.0) 
        
               worker_disconnect_delay = parameter.FloatParameter(default=60.0) 
        
               state_path = parameter.Parameter(default='/var/lib/luigi-server/state.pickle') 
        
               # Jobs are disabled if we see more than disable_failures failures in disable_window seconds. 
        
               # These disables last for disable_persist seconds. 
        
               disable_window = parameter.IntParameter(default=3600, 
        
                                                       config_path=dict(section='scheduler', name='disable-window-seconds')) 
        
               disable_failures = parameter.IntParameter(default=None, 
        
                                                         config_path=dict(section='scheduler', name='disable-num-failures')) 
        
               disable_hard_timeout = parameter.IntParameter(default=None, 
        
                                                             config_path=dict(section='scheduler', name='disable-hard-timeout')) 
        
               disable_persist = parameter.IntParameter(default=86400, 
        
                                                        config_path=dict(section='scheduler', name='disable-persist-seconds')) 
        
               max_shown_tasks = parameter.IntParameter(default=100000) 
        
               prune_done_tasks = parameter.BoolParameter(default=False) 
        
               record_task_history = parameter.BoolParameter(default=False)

class scheduler(Config):
    ...
    visualization_graph = parameter.Parameter(default='svg')
    ...

Then access it the way you do self._scheduler._config.visualization_graph. Also, you should check that the value is either svg or d3. Since the default is set to be svg it's going to be backward compatible.

hadesbox · 2015-05-11T16:00:54Z

So I think these changes should do the trick you suggest, I have tested them in local and everything is working, if you think they are fine I'll push them so you can review them.

luigi/scheduler.py

adding the visualization_graph in the scheduler class, with default value svg.

@@ -99,6 +99,8 @@ class scheduler(Config):

     record_task_history = parameter.BoolParameter(default=False)

+    visualization_graph = parameter.Parameter(default="svg", config_path=dict(section='scheduler', name='visualization-graph'))
+

 def fix_time(x):
     # Backwards compatibility for a fix in Dec 2014. Prior to the fix, pickled state might store datetime objects

luigi/server.py

I changed the tornado.web.RequestHandler for BaseTaskHistory so it has the initialize method and set the local property scheduler.

@@ -211,12 +211,14 @@ class StaticFileHandler(tornado.web.RequestHandler):
         self.write(data)


-class RootPathHandler(tornado.web.RequestHandler):
+class RootPathHandler(BaseTaskHistoryHandler):

     def get(self):
-        visualization_graph = configuration.get_config().get('core', 'visualization_graph', '')
+        visualization_graph =  self._scheduler._config.visualization_graph
         if visualization_graph == "d3":
             self.redirect("/static/visualiser/index.d3.html")
+        elif visualization_graph == "svg":
+            self.redirect("/static/visualiser/index.html")
         else:
             self.redirect("/static/visualiser/index.html")

@@ -226,7 +228,7 @@ def app(scheduler):
     handlers = [
         (r'/api/(.*)', RPCHandler, {"scheduler": scheduler}),
         (r'/static/(.*)', StaticFileHandler),
-        (r'/', RootPathHandler),
+        (r'/', RootPathHandler, {'scheduler': scheduler}),
         (r'/tasklist', AllRunHandler, {'scheduler': scheduler}),
         (r'/tasklist/(.*?)', SelectedRunHandler, {'scheduler': scheduler}),
         (r'/history', RecentRunHandler, {'scheduler': scheduler}),

Tarrasch · 2015-05-11T16:06:03Z

Looks good! :)

…ep backwards compatibility

hadesbox · 2015-05-11T16:14:11Z

Ok its pushed.

…er operator

Tarrasch · 2015-05-12T15:45:31Z

Ok, nobody has commented on this for a while, so I merge. :)

D3visualizer

erikbern · 2015-05-12T17:00:57Z

NICE

hadesbox · 2015-05-12T19:20:33Z

If you have comments or improvements let me know on this thread or PM or luigi user forum so we can work on them 👍

hadesbox force-pushed the d3visualizer branch from 3d1bb65 to e3366ae Compare May 11, 2015 10:34

hadesbox force-pushed the d3visualizer branch 2 times, most recently from 9a00376 to 6518b4e Compare May 11, 2015 10:49

Changing edges labels

3e50457

hadesbox force-pushed the d3visualizer branch from 6518b4e to 3e50457 Compare May 11, 2015 10:55

Tarrasch reviewed May 11, 2015
View reviewed changes

adding link to the graph nodes, so its possible to navigate the graph…

3c1c919

… and see the deps of a particular node (and not the whole tree).

Tarrasch reviewed May 11, 2015
View reviewed changes

Adding scheduler class to server.py, to fetch config properties to ke…

69304c7

…ep backwards compatibility

Removing pep8 error: luigi/server.py:217:30: E222 multiple spaces aft…

916c9b0

…er operator

Tarrasch added a commit that referenced this pull request May 12, 2015

Merge pull request #920 from hadesbox/d3visualizer

aa7ba82

D3visualizer

Tarrasch merged commit aa7ba82 into spotify:master May 12, 2015

hadesbox deleted the d3visualizer branch May 12, 2015 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

D3visualizer #920

D3visualizer #920

hadesbox commented Apr 29, 2015

Tarrasch commented Apr 29, 2015

erikbern commented Apr 29, 2015

gpoulin commented Apr 29, 2015

hadesbox commented Apr 29, 2015

hadesbox commented Apr 29, 2015

gpoulin commented Apr 29, 2015

Tarrasch commented Apr 29, 2015

erikbern commented May 5, 2015

hadesbox commented May 5, 2015

hadesbox commented May 5, 2015

Tarrasch commented May 5, 2015

hadesbox commented May 5, 2015

Tarrasch commented May 5, 2015

hadesbox commented May 11, 2015

Tarrasch May 11, 2015

hadesbox May 11, 2015

Tarrasch May 11, 2015

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

hadesbox commented May 11, 2015

Tarrasch May 11, 2015

hadesbox May 11, 2015

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

Tarrasch commented May 12, 2015

erikbern commented May 12, 2015

hadesbox commented May 12, 2015

D3visualizer #920

D3visualizer #920

Conversation

hadesbox commented Apr 29, 2015

Tarrasch commented Apr 29, 2015

erikbern commented Apr 29, 2015

gpoulin commented Apr 29, 2015

hadesbox commented Apr 29, 2015

hadesbox commented Apr 29, 2015

gpoulin commented Apr 29, 2015

Tarrasch commented Apr 29, 2015

erikbern commented May 5, 2015

hadesbox commented May 5, 2015

hadesbox commented May 5, 2015

Tarrasch commented May 5, 2015

hadesbox commented May 5, 2015

Tarrasch commented May 5, 2015

hadesbox commented May 11, 2015

Tarrasch May 11, 2015

Choose a reason for hiding this comment

hadesbox May 11, 2015

Choose a reason for hiding this comment

Tarrasch May 11, 2015

Choose a reason for hiding this comment

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

hadesbox commented May 11, 2015

Tarrasch May 11, 2015

Choose a reason for hiding this comment

hadesbox May 11, 2015

Choose a reason for hiding this comment

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

luigi/scheduler.py

luigi/server.py

Tarrasch commented May 11, 2015

hadesbox commented May 11, 2015

Tarrasch commented May 12, 2015

erikbern commented May 12, 2015

hadesbox commented May 12, 2015