Ticket/18934

[melinath]

This pull request resolves the following tickets:

• 18934 - Farm the work of handles_video_url out to the individual methods.
• 18615 - Submitting some YouTube URLs doesn't pull in metadata

The actual work involved is a refactor of SuiteMethods into VideoLoaders; it further decouples video data loading from suites.

Travis doesn't seem to be building vidscraper atm, but all tests are passing locally.

[paulswartz]

Looks like Travis did run the build, and it failed: http://travis-ci.org/#!/pculture/vidscraper/builds/1086383

I haven't looked at the code yet, but it's in my queue.

Update: the tests are passing locally for me; not sure what's up with Travis.

[paulswartz]

It seems strange that this is a mixin, rather than a subclass of VideoLoader. Is this functionality useful other than when mixed with VideoLoader?

[melinath]

Actually, it originally was a subclass of VideoLoader; I switched it to a mixin to make it easier to combine with other mixins (specifically, the path mixins used by a couple of suites.)

[paulswartz]

Yeah, I noticed that you made a commit about that. It's not obvious from the code, however; can you add a comment, or a docstring, about why it's implemented this way?

[melinath]

I've added a note.

[paulswartz]

Updated update: tests are failing, looks like a Vimeo problem:

======================================================================
ERROR: test_video (vidscraper.tests.integration.test_vimeo.VimeoIntegrationTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/vagrant/src/vidscraper/vidscraper/tests/integration/test_vimeo.py", line 40, in test_video
    video.load()
  File "/vagrant/src/vidscraper/vidscraper/videos.py", line 153, in load                                                                                                                                                                
    data = self.run_loaders()
  File "/vagrant/src/vidscraper/vidscraper/videos.py", line 223, in run_loaders
    data.update(loader.get_video_data(response))
  File "/vagrant/src/vidscraper/vidscraper/suites/vimeo.py", line 102, in get_video_data
    doc = minidom.parseString(response.text)
  File "/usr/lib/python2.6/xml/dom/minidom.py", line 1928, in parseString
    return expatbuilder.parseString(string)
  File "/usr/lib/python2.6/xml/dom/expatbuilder.py", line 940, in parseString
    return builder.parseString(string)
  File "/usr/lib/python2.6/xml/dom/expatbuilder.py", line 223, in parseString
    parser.Parse(string, True)
ExpatError: mismatched tag: line 33, column 2
-------------------- >> begin captured logging << --------------------
requests.packages.urllib3.connectionpool: INFO: Starting new HTTP connection (1): vimeo.com
requests.packages.urllib3.connectionpool: DEBUG: "GET /api/v2/video/7981161.json HTTP/1.1" 200 487
requests.packages.urllib3.connectionpool: INFO: Starting new HTTP connection (1): www.vimeo.com
requests.packages.urllib3.connectionpool: DEBUG: "GET /moogaloop/load/clip:7981161 HTTP/1.1" 301 249
requests.packages.urllib3.connectionpool: INFO: Starting new HTTP connection (1): vimeo.com
requests.packages.urllib3.connectionpool: DEBUG: "GET /moogaloop/load/clip:7981161 HTTP/1.1" 404 6078
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 82 tests in 15.227s

[melinath]

Travis did run the branch, but it hasn't run the branch since 7cd225f (the first commit in this branch) which is why it failed. The vimeo test does fail, but sporadically - and I'm pretty sure it does the same on the develop branch.

[melinath]

I think all of the current notes have been addressed; let me know if there's anything else you'd like to see.

[paulswartz]

The changes look good.