paulswartz/vidscraper — comment on commit d6be36090d519043def5575c5890428f432e362f
File:  line 0
By melinath:

Hmm. Yeah, you're right. It makes more sense to have people check whether fields they want are missing, since load_video_data only guarantees that it will fill as many fields as it can.

However, there are still some problems with the approach taken in this commit:

- If the oembed url is returning a 404, chances are it isn't being built correctly for a certain input (say, iframe urls?).
- Even if a 404 (or some other error) is raised, it would be better to let the error propagate one level up.
With this commit, if oembed alone *seems* to satisfy all requirements, it will be the only method that is attempted during _run_methods. If it raises a urllib2.URLError, that will get caught and the loop will skip to the next method which is being run. Since the only method being attempted is oembed, that means that _run_methods will return without having populated anything, and so will load_video_data.
However, if we instead let the error propagate up a level, we could catch it there and try the next method (an API call) which, though probably somewhat more costly, might still be able to supply the fields we need. It might also be a good idea to skip any method groups that include a combination which is known not to work.