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

Fixed a bug in intersphinx failure reporting #6139

Closed
wants to merge 1 commit into from

Conversation

lsaffre
Copy link
Contributor

@lsaffre lsaffre commented Mar 6, 2019

Subject: fix a bug in intersphinx reporting failures

I had a published Sphinx doctree at http://de.presto.lino-framework.org with an invalid :xfile:objects.inv file so that intersphinx was not able to read it. It said::

ValueError: unknown or unsupported inventory version: ValueError(u'invalid inventory header: ',)

This message is not the problem (I guess it was because that site was generated with another Sphinx version, but that's unrelevant). My problem is that this error was never reported. It was never reported because :func:sphinx.ext.intersphinx.fetch_inventories does something forbidden::

try:
   ...
except Exception as err:
    err.args = ('intersphinx inventory %r not fetchable due to %s: %s',
                inv, err.__class__, err)  # LS20190306
    raise
try:
    ...
except Exception as err:
    err.args = ('intersphinx inventory %r not readable due to %s: %s',
                inv, err.__class__.__name__, err)  # LS20190306

To fix my problem, I replaced both lines::

    err.args = (msgtpl, inv, err.__class__, err)

by::

    err.args = (msgtpl, inv, err.__class__, str(err))

The problem might occur only when there is an additional exception "intersphinx inventory has moved" involved. Here is the output before the bugfix::

loading intersphinx inventory from http://de.presto.lino-framework.org...
intersphinx inventory has moved: http://de.presto.lino-framework.org -> http://de.presto.lino-framework.org/
WARNING: failed to reach any of the inventories with the following issues:
intersphinx inventory 'http://de.presto.lino-framework.org' not readable due to ValueError: ('intersphinx inventory %r not readable due to %s: %s', 'http://de.presto.lino-framework.org', 'ValueError',  ValueError(...))

Another problem fixed en passant was that intersphinx later reports every failure as a separate call to :func:warning and when tolerate_warnings is False, we see only the first warning.

@codecov
Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #6139 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6139      +/-   ##
==========================================
- Coverage   82.67%   82.67%   -0.01%     
==========================================
  Files         272      272              
  Lines       39301    39300       -1     
  Branches     5877     5877              
==========================================
- Hits        32493    32492       -1     
  Misses       5473     5473              
  Partials     1335     1335
Impacted Files Coverage Δ
sphinx/ext/intersphinx.py 88.23% <100%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b58d0e8...c0ed808. Read the comment docs.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. But could you rebase this into 1.8 branch? Then I'll merge this into next release.
Thanks,

@tk0miya tk0miya added this to the 1.8.5 milestone Mar 6, 2019
@lsaffre
Copy link
Contributor Author

lsaffre commented Mar 8, 2019

Seems that my brain is too small for this task. But I tried and created a new pull request #6148
Does this help?

@tk0miya
Copy link
Member

tk0miya commented Mar 8, 2019

Merged #6148 instead.

@tk0miya tk0miya closed this Mar 8, 2019
tk0miya added a commit that referenced this pull request Mar 8, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants