Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unused field from info_dict in FileDownloader #938

Closed
jaimeMF opened this issue Jun 26, 2013 · 3 comments
Closed

Unused field from info_dict in FileDownloader #938

jaimeMF opened this issue Jun 26, 2013 · 3 comments

Comments

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented Jun 26, 2013

I was reading the code in FileDownloader.py and I see this piece of code (source):

try:
    if count == 0 and 'urlhandle' in info_dict:
        data = info_dict['urlhandle']
    data = compat_urllib_request.urlopen(request)
    break
except (compat_urllib_error.HTTPError, ) as err:
    ...

If I'm not wrong, the info_dict['urlhandle'] is being overwritten always, so it's never used. Is that the intended behaviour, or it's just that I have misundestoond something?

@phihag
Copy link
Contributor

@phihag phihag commented Jun 26, 2013

That looks like a mistake. Feel free to remove after checking that the code that uses urlhandle at all still works.

@jaimeMF
Copy link
Collaborator Author

@jaimeMF jaimeMF commented Jun 26, 2013

If I understand it well (I haven't dive deeply in the downloader) the urlhandle should be used instead of compat_urllib_request.urlopen(request), so my solution would be to do:

 if count == 0 and 'urlhandle' in info_dict:
    data = info_dict['urlhandle']
else:
    data = compat_urllib_request.urlopen(request)

I would like to test it, but only Blip.tv uses this field and only with some videos, and I can't find any of them.
Also this is the only part on the code where the urlhandle is used, have I missed something?

@jaimeMF
Copy link
Collaborator Author

@jaimeMF jaimeMF commented Dec 23, 2013

Fixed in 1538eff, thanks.

@jaimeMF jaimeMF closed this Dec 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.