-
Notifications
You must be signed in to change notification settings - Fork 17
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
Ensure download destination directory exists #31
Conversation
@rlaphoenix Thank you for contributing this and at the same time raising an issue I wasn't aware of. Would you mind contributing a minimal working example which replicates the error? |
Hi, sorry for the wait. I've made a minimal reproducible example repository here: https://github.com/rlaphoenix/rtd-dir-reproducible-example It seems the root of the issue may actually be caused by sphinxcontrib-youtube 1.2.0 (the new thumbnail feature it has). You can see I progressively added further advanced usage of sphinxcontrib-images and it built correctly up until I added sphinxcontrib-youtube 1.2.0. The build failed on rtd at the final stage:
Log of the call (long)Running Sphinx v4.5.0 Downloading remote images...[ 20%] https://www.lipsum.com/images/banners/black_234x60.gif Traceback (most recent call last): The above exception was the direct cause of the following exception: Traceback (most recent call last): Extension error (sphinxcontrib.images): |
You can see in the log it made a download of the thumbnail to a
|
@rlaphoenix Thank you for the example. I've studied it and the behavior of the build. I find the same thing as you, with sphinxcontrib-youtube added things fail with the current version 0.9.4 of sphinxcontrib-images (sphinx 5.1.1 and sphinxcontrib-youtube 1.2.0, likely with other version combinations as well). The error I get, when building with 'make html' is this:
And what I do not after looking at things this afternoon is why this extension should process files in So currently I can not shake the feeling that this extension should ignore those files but do not. Making an extra directory (as you PR#31 does) might very well keep the extension from failing, but I'm not convinced it is the right place to fix the error. I'll look at it again later. |
Hi everyone, I'm just passing by for some context. I'm responsible for the conflict between sphinxcontrib-youtube and sphinxcontrib-image as I inspired myself from this lib to download image thumbnails for youtube and vimeo videos. Normally this conflict is solved since sphinx-contrib/youtube#40 if it is of any help. I'll listen to this thread if any further help is needed. |
Yeah I can confirm it's now fixed by sphinx-contrib/youtube#40, now we are just waiting for a new release. |
Otherwise, a FileNotFound exception will be raised on the open() call.
This issue may be somewhat related to Pull Request #23 I'm not sure.
It wasn't as simple as making the
env.srcdir
, as theenv.remote_images[src]
typically contains at least one folder stem. e.g.,'_video_thumbnail/657905289.jpg'
. This is why it takes the fulldst
, gets its parent directory, and makes the directory of that including its parents' leafs.I've personally encountered this on readthedocs in a poetry build environment.
I can confirm the rtd environment did already have
env.srcdir
made, but not the further_video_thumbnail/
folder fromenv.remote_images[src]
.