tgdwyer / WebCola

Javascript constraint-based graph layout
http://marvl.infotech.monash.edu/webcola/
MIT License
2.02k stars 258 forks source link

Online Graph Exploration (movies) Example Not Working #171

Open pembeci opened 8 years ago

pembeci commented 8 years ago

Incremental exploration of large graph example is not working any more. When I click the collapsed pink nodes I am getting the following errors in FF and Chrome:

// Firefox: TypeError: node.cast is undefined  ---   tmdbgraph.js:79:17
// Chrome: tmdbgraph.ts:68 Uncaught TypeError: Cannot read property 'map' of undefined
var dn = node.cast.map(function (c) { return _this.getNode(node.type.next(), c.id, function (v) {

Can this be related to my browsers or is it possible that an update broke the code? Thanks.

tgdwyer commented 8 years ago

Hmm, I don't see that. Are you running it from the marvl website or are you serving the webcola repo locally? Are you getting 404 errors from the tmdb gets also?

pembeci commented 8 years ago

From the marvl website. tmdb GETs working fine. I just noticed some of the api.themoviedb.org GETs are returning 429 with status message: "Your request count (47) is over the allowed limit of 40." So I guess this is the problem. Not a big issue, I can check the source code and the other molecules example which looks similar. I discovered it after opening this issue.

aspiers commented 7 years ago

I see this too. It's a shame, since that was one of my favourite WebCola examples.

gordonwoodhull commented 7 years ago

It looks like it could probably be fixed by upgrading to jQuery 1.4 and putting $.delay(1000/40).get(...) instead of just $.get(...) in tmdbgraph.ts - they are rate limiting at 40 requests per second, which is reasonable and should not hurt the example much.

However, I didn't see how the build system works for the examples so I wasn't able to try this out.

tgdwyer commented 7 years ago

I had a play with this. It seems you can't $.delay(), the delay function is only available on selections, not in the global $ scope.

I also tried some hacky promise code with a timeout, but then realised that that isn't actually going to throttle the calls, just send them all out at once after a uniform delay. Does $.delay() do the same or does it work differently to actually throttle?

tgdwyer commented 7 years ago

I've now upgraded all the examples to jquery 3.1.1 and I've refactored the tmdb example so it's all in one big ugly .ts file. It's built with browserify like so:

> grunt examples

You can see my commented out attempts to add throttling.

I believe when uncommented the throttling works, but I still get the 429 error... so I guess the limit checking is not as simple as 40 requests per second?

gordonwoodhull commented 7 years ago

I see - you're right, all the promises would start at the same time, so it's the same problem.

Rats, it looks like it takes about 100 lines to throttle promises in a general way. There must be a simple solution to this somehow.

gordonwoodhull commented 7 years ago

Thanks for making it easier to build the example.

I was able to get the example working without 429s by using this kind of dumb technique, but only when I throttled it down to no more than 3 requests/second. I am guessing part of the problem is that they also consider requests to image.tmdb.org in the throttling, so just throttling the ajax requests is not enough.

I don't know much typescript, so my "solution" looks likes this:

    var dumb_throttle = function(timeout) {
        var requests = [];
        setInterval(function() {
            if(requests.length) {
                var request = requests.pop();
                request();
            }
        }, timeout);
        return function(f) {
            requests.push(f);
        };
    }(1000/3);
    // ...
        var dfd = $.Deferred();
        dumb_throttle(function() {
            $.get(query).then((data)=>dfd.resolve(data));
        });
        return dfd;

Hopefully there is a better way!

Edit: ah, I misread the tmdb FAQ: it's 40 requests every 10 seconds. So this sort of makes sense.

tgdwyer commented 7 years ago

Yep, that's it... it's just a little bit too slow to give the full effect :-(. Even a greedy method where the throttling doesn't kick in until you use your quota wouldn't be ideal - because the nodes in this graph seem to have a lot of neighbours - you hit the limit quickly and then you'd have to wait 10 seconds,

Does anybody know another nice example of on-line graph that doesn't have such quotas?

gordonwoodhull commented 7 years ago

Thanks for fixing this demo, @tgdwyer! The throttling is unfortunate but it's great to see this working.