linkcheck: replace 'linkcheck_allowed_redirects' default sentinel with boolean false value

[jayaddison]

Purpose

Patches the linkcheck module so that linkcheck_allowed_redirects is never assigned a value of None -- this was failing to typecheck against the allowed values for the config value when HTML themes were enabled (ref: #13462).

Specifically: this pull request updates the default value of linkcheck_allowed_redirects from the previous sentinel value to a new value of False.

Linkchecking and allowed redirections behaviour

When the linkcheck builder follows a hyperlink that redirects, the link is considering working when either of the following occur:

• The server issues a redirect back to the same URL (with any trailing-slashes removed).
• A matching {origin, destination} pattern pair for the server-issued redirect is found in linkcheck_allowed_redirects.

All other redirects are reported with the redirected status code.

When linkcheck_allowed_redirects is configured -- e.g. set to any other value than the default of False -- hyperlinks that encounter redirects with no matching matching {origin, destination} pattern pair in the configuration value also emit a warning-level log message.

References

• Resolves #13462.
• Relates-to #13439.
• Tests should succeed both with and without #13466.

Edit: possible-values in this description requires redrafting; add a todo placeholder. Edit: update proposed config value behaviours. Edit: invert default value (was: true; now: false). Edit: attempt to clarify this description. Edit: describe the change-in-behaviour nearer the start of this description, reformat a few elements, and attempt to communicate the logic more clearly. Edit: a correction about the existing default config value (sentinel), and a clarification about the described conditions under which redirected hyperlinks are considered working.

[pllim]

FWIW this PR turns our downstream job from red to green: https://github.com/astropy/sphinx-astropy/pull/82

Thanks!

[jayaddison]

Thank you @pllim for testing it :)

[bsipocz]

Can we merge this please, so I don't keep getting the failing cron notifications 😄 ?

[jayaddison]

@AA-Turner ping for possible code review?

[jayaddison]

When linkcheck_allowed_redirects is configured -- e.g. set to any other value than the default of False -- hyperlinks that encounter redirects with no matching matching {origin, destination} pattern pair in the configuration value also emit a warning-level log message.

As far as I can tell, we currently lack unit test coverage for the scenario where an info-level log message is emitted for a redirect. This could be useful to add to catch possible future regressions - the logic related to allowed redirects feels nontrivial to me.

[jayaddison]

When linkcheck_allowed_redirects is configured -- e.g. set to any other value than the default of False -- hyperlinks that encounter redirects with no matching matching {origin, destination} pattern pair in the configuration value also emit a warning-level log message.

As far as I can tell, we currently lack unit test coverage for the scenario where an info-level log message is emitted for a redirect. This could be useful to add to catch possible future regressions - the logic related to allowed redirects feels nontrivial to me.

(done - test coverage added)

[bsipocz]

Can we get this merged please? the sphinx-astropy tests are failing with the dev version of sphinx for a while due to this for a while. However, in the meantime a new failure is popped up that we didn't notice at first as the job was failing for a while already.

[AA-Turner]

@jayaddison sorry for the delay. I kept the sentinel but fixed the underlying problem.

@bsipocz please could you see if this fixes the problems for astropy?

A

[bsipocz]

@AA-Turner - Actually, this fixed both of the issues we saw in the most recent runs. Thank you!

[jayaddison]

Thank you @AA-Turner @bsipocz!

From a brief glance earlier, I have a feeling that one of those failures may be an intermittent intersphinx problem unrelated to these changes - it's good if it appears to have gone away, but I'd (personally) stay on the lookout for it reoccurring. I hadn't gotten to the stage of trying to figure out how/why it occurs, though.