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

image-checker.py fails under Python 3 #104

Closed
IgnoredAmbience opened this issue May 2, 2017 · 5 comments
Closed

image-checker.py fails under Python 3 #104

IgnoredAmbience opened this issue May 2, 2017 · 5 comments

Comments

@IgnoredAmbience
Copy link
Contributor

IgnoredAmbience commented May 2, 2017

Stack trace:

Checking ../packs/scrabble.yaml.Traceback (most recent call last):
  File "image-checker.py", line 182, in <module>
    errors, warnings = check_yaml(filename, args.resize)
  File "image-checker.py", line 106, in check_yaml
    download = wget.download(url, tempfile.gettempdir(), bar=None)
  File "/home/thomas/ingress/emojipacks/test/venv/lib/python3.6/site-packages/wget.py", line 526, in download
    (tmpfile, headers) = ulib.urlretrieve(binurl, tmpfile, callback)
  File "/usr/lib64/python3.6/urllib/request.py", line 248, in urlretrieve
    with contextlib.closing(urlopen(url, data)) as fp:
  File "/usr/lib64/python3.6/urllib/request.py", line 223, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib64/python3.6/urllib/request.py", line 532, in open
    response = meth(req, response)
  File "/usr/lib64/python3.6/urllib/request.py", line 642, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib64/python3.6/urllib/request.py", line 570, in error
    return self._call_chain(*args)
  File "/usr/lib64/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/usr/lib64/python3.6/urllib/request.py", line 650, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 403: Forbidden

It looks like the monkey patch line to set the UserAgent for the wget library isn't taking effect for Python 3, this is probably because that library uses different urllib implementations on different Python versions.

Ran into this whilst trying to update the test suite for #52.

@zachfeldman
Copy link
Contributor

@edgemaster thanks for reporting! Is this something you'd feel comfortable fixing yourself or do you need an assist or have any questions?

@IgnoredAmbience
Copy link
Contributor Author

I'm happy to raise a PR to fix it. (Tbh, a rewrite into JS would be nicer, but I don't have the time for that(!))

@zachfeldman
Copy link
Contributor

That would be awesome! Thanks @edgemaster . Yeah a JS rewrite might be helpful long run, but considering how this library most likely isn't a major production dependency for everyone...let's do it the lazy way :)

@hugovk
Copy link
Collaborator

hugovk commented May 2, 2017

Hello, I wrote image-checker.py using Python 2.7, so I'm not entirely surprised, but sorry about that!

Python 3 support would be good, and probably not too difficult.

It'd be a good idea to enable Travis CI for your repo, and add Python 3 (e.g. 3.6): https://github.com/hugovk/emojipacks/blob/master/.travis.yml#L3
e.g. https://travis-ci.org/hugovk/emojipacks/builds/228087738

Some options:

  • Download the files natively in Python 3 and use wget for Python 2
  • Download the files natively in Python 2 and 3
  • Fix the wget library to use the UserAgent in Python 3
  • Something else!

It would be a good idea to at least report the problem to wget even though it's not very active:
https://bitbucket.org/techtonik/python-wget/src If you do fix wget itself, it might not get merged, but we can always pip install it from Git.

IgnoredAmbience added a commit to IgnoredAmbience/emojipacks that referenced this issue May 2, 2017
Move from using wget library to direct calls to urllib, doesn't add too
much additional complexity to the code. Also handle HTTP errors as
script errors instead of terminating abnormally.

Fixes lambtron#104
@IgnoredAmbience
Copy link
Contributor Author

I've opted to remove the dependency on wget and use urllib2 directly to download the images. It also removes the need to write out to a temporary file, as everything can be done in memory. It also simplifies the handling of http errors that wget would throw. I don't think it provides less functionality, but I could have missed something.

zachfeldman pushed a commit that referenced this issue May 3, 2017
Move from using wget library to direct calls to urllib, doesn't add too
much additional complexity to the code. Also handle HTTP errors as
script errors instead of terminating abnormally.

Fixes #104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants