Call raise_for_status after retrieving a URL with requests #176
Conversation
Sometimes, image files may fail to download due to HTTP errors such as "429 Too Many Requests". As a result of the image file on disk being empty, errors may occur later on in the notebook, which can look somewhat misleading: cv2.error: OpenCV(4.7.0) /io/opencv/modules/imgcodecs/src/loadsave.cpp:798: error: (-215:Assertion failed) !buf.empty() in function 'imdecode_' Add a call to `raise_for_status()` on the returned `response` object from the `requests` library, which at the very least notifies the user that something has gone wrong. (Alternately, perhaps consider relocating the images from imgur to some other CDN, or even commit them directly to this repository.) Also, fix some indentation, spacing, and variable naming inconsistencies.
|
|
|
Hi, @jkitching 👋🏻! I was away for a few days. Awesome 🔥! Thanks for that contribution. Could you sign the CLA so I could merge it into the master? 🙏🏻 |
|
Hi @SkalskiP, thanks for your response! I've signed the CLA. Is the same CLA also valid for this other pull request? |
|
@jkitching The |
capjamesg
left a comment
capjamesg
left a comment
There was a problem hiding this comment.
Looks good to me! Regarding use of Imgur, we will keep that in mind as we write new notebooks. We definitely want to use persistent image URLs so our notebook images are available over time.
4 participants
Description
Sometimes, image files may fail to download due to HTTP errors such as "429 Too Many Requests". As a result of the image file on disk being empty, errors may occur later on in the notebook, which can look somewhat misleading:
cv2.error: OpenCV(4.7.0) /io/opencv/modules/imgcodecs/src/loadsave.cpp:798:
error: (-215:Assertion failed) !buf.empty() in function 'imdecode_'
Add a call to
raise_for_status()on the returnedresponseobject from therequestslibrary, which at the very least notifies the user that something has gone wrong.(Alternately, perhaps consider relocating the images from imgur to some other CDN, or even commit them directly to this repository.)
Also, fix some indentation, spacing, and variable naming inconsistencies.
Type of change
Bug fix (non-breaking change which fixes an issue)
How has this change been tested, please provide a testcase or example of how you tested the change?
Manually tested running the notebooks, ensuring the 429 error is displayed to the user.
Any specific deployment considerations
None.
Docs
No documentation was updated.