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

Ignore missing dependencies #77

Merged
merged 1 commit into from
Jun 28, 2024
Merged

Ignore missing dependencies #77

merged 1 commit into from
Jun 28, 2024

Conversation

dolfinus
Copy link
Contributor

@dolfinus dolfinus commented Jun 20, 2024

I have a README file which includes image from docs/_static folder:
https://github.com/MobileTeleSystems/onetl/blob/develop/README.rst?plain=1#L28

And I also include this file in docs/index.rst file of my documentation:
https://github.com/MobileTeleSystems/onetl/blob/06b561a7a7e642893c1feb500109a83de741207c/docs/index.rst?plain=1#L1-L2

This leads to including file with path docs/docs/_static because Sphinx does resolve file paths only after the entire content is being included. Fortunately, Sphinx just ignores missing files, and I can use a small hack to include this image to docs/index.rst (see file below).

Unfortunately, this extension expects all the files from directives, like an image above, to exist. When git ls-tree receives a file which does not exist, it fails with a message like this:

Extension error (sphinx_last_updated_by_git):
Handler <function _env_updated at 0x7daaea7b4e00> for event 'env-updated' threw an exception (exception: [Errno 2] No such file or directory: PosixPath('/home/maxim/Repo/onetl/docs/docs/_static'))

I've added a small check that dependency should exist, to avoid such an issue.

@mgeier
Copy link
Owner

mgeier commented Jun 20, 2024

Thanks for this PR!

I'm willing to add something like this (maybe with a warning?), but not before having a look at the root cause.

Doesn't that sound like a bug in Sphinx?

I found two issues that sound related:

Those issues don't look very active, but maybe you can comment there or maybe you even have an idea how to fix this in Sphinx?

@pep8speaks
Copy link

pep8speaks commented Jun 24, 2024

Hello @dolfinus! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-06-28 11:01:21 UTC

@dolfinus
Copy link
Contributor Author

dolfinus commented Jun 24, 2024

Thank for the links, I'll give it a try.

Regarding current PR, there could be more cases which can lead to the same issue. For example, some dependency may be generated by some external process, e.g. OpenAPI specification or some kind of example file. This change allows to avoid issues in such cases.

maybe with a warning?

Added a warning.

@mgeier
Copy link
Owner

mgeier commented Jun 28, 2024

For example, some dependency may be generated by some external process, e.g. OpenAPI specification or some kind of example file.

Yes, we should definitely handle such a case. I think a warning is the right thing to do, because users might not even be aware of the situation.

Would you like to add a point in the "Caveats" section of the README about missing dependencies?
There you could also mention how to disable the warning, if desired.

@dolfinus
Copy link
Contributor Author

dolfinus commented Jun 28, 2024

Would you like to add a point in the "Caveats" section of the README about missing dependencies?

If this not lead to issues, why mention that?

@mgeier
Copy link
Owner

mgeier commented Jun 28, 2024

I was mostly thinking about it as a way to document how to disable the warnings.

If someone runs Sphinx with warnings turned into errors (with the -W flag, which I normally do), they will want to know that.

@dolfinus
Copy link
Contributor Author

Added a note to README

@mgeier mgeier merged commit 1a1e4d5 into mgeier:master Jun 28, 2024
9 checks passed
@mgeier
Copy link
Owner

mgeier commented Jun 28, 2024

Thanks a lot!

@dolfinus dolfinus deleted the patch-1 branch June 28, 2024 12:10
@dolfinus
Copy link
Contributor Author

May I request a release?

@mgeier
Copy link
Owner

mgeier commented Aug 11, 2024

Sure: https://pypi.org/project/sphinx-last-updated-by-git/0.3.8/

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

Successfully merging this pull request may close these issues.

3 participants