-
Notifications
You must be signed in to change notification settings - Fork 423
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
Missing icons #95
Missing icons #95
Conversation
# Write file and show message | ||
_write_file(filename, contents) | ||
# Retrieve file and show message | ||
urlretrieve(asset_url, filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this more robust than using requests and writing to a file?
Requests handles things like following redirects for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I just read your description.
What was causing the "wrong file size"? Was it requests, or the _write_file
? If it's _write_file
, that should probably be fixed instead of worked around so that other uses of function don't fail.
Just tested it and it works. Nice job! Thanks for the fix, @madflow! |
This is great, thanks ! |
Just released a new version with your fix. Be sure to |
👍 Thanks for the merge . Let's see which new bugs I introduced with this :D |
@madflow 😄 |
This is a possible fix for #85. The problem seems to be twofold:
requests.get(asset_url, verify=False).text
seemed to bail on .woff etc files (wrong file size)<path:filename>
route did not match the icons url.This is a typical "works for me" hack - maybe it does some good ;)