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.

Sign up for GitHub

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

Jump to bottom

linkcheck: add a distinct 'timeout' reporting status #11876

Merged
merged 1 commit into from
Jan 13, 2024
Merged

linkcheck: add a distinct 'timeout' reporting status #11876

merged 1 commit into from
Jan 13, 2024

Conversation

jayaddison
Copy link
Contributor

Feature or Bugfix

  • Feature

Purpose

  • Provides a distinction between broken links (fairly high confidence that the URI/resource does not currently exist) and timeouts (no response was received for the URI within the alloted time).

Detail

  • Adds an additional case-handler for Timeout exceptions from the requests library when checking hyperlinks.
  • Maintains a non-zero exit code when timeouts are encountered - the rationale here is that there was an unexpected problem that could require investigation (in other words: not all links were filtered/ignored/found-successfully).

Relates

@jayaddison
Copy link
Contributor Author

Todo: consider the interaction between these changes and the low-level Python socket timeout, because we configure a value for that.

@jayaddison
Copy link
Contributor Author

Todo: consider the interaction between these changes and the low-level Python socket timeout, because we configure a value for that.

Generally I think that because we provide a timeout= argument when making HTTP requests via requests (that in turn configures timeouts via urllib3), the socket.setdefaulttimeout call should -- in theory -- be redundant.

However: I think we should wait until urllib3>=2.0.0 is our baseline before considering removing the configuration of a 5s socket timeout there. That's because there were some bugs relating to timeout handling that are addressed in v2.0.0 of urllib3.

As a result, I don't think we should remove the setdefaulttimeout call before we declare requests>=2.30.0 as a baseline, at the very earliest. In practice we should wait longer, because just because requests 2.30.0 added support for urllib3 v2.0.0 doesn't mean that everyone using it will upgrade immediately.

@AA-Turner AA-Turner merged commit 9e198c7 into sphinx-doc:master Jan 13, 2024
21 of 22 checks passed
@jayaddison jayaddison deleted the issue-11868/linkcheck-distinct-timeout-status branch January 13, 2024 09:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2024
@AA-Turner AA-Turner added this to the 7.3.0 milestone Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.3.0
Development

Successfully merging this pull request may close these issues.

linkcheck builder: add a distinct reporting status code for timeouts
2 participants
@jayaddison @AA-Turner