Version sync: skip syncing versions when lsremote fails #12540
Merged
+40
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR adds error handling for the git ls-remote command to prevent sync failures when the command fails. The main purpose is to gracefully handle repository access errors during version synchronization.
Key changes:
- Added exit code checking in
lsremote()to raiseRepositoryErrorwhen the git command fails - Added a new error code
FAILED_TO_GET_VERSIONSto theRepositoryErrorexception class - Wrapped
lsremote()call in a try-except block to catch errors and prevent downstream task execution
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| readthedocs/vcs_support/backends/git.py | Added exit code validation to raise RepositoryError when git ls-remote fails |
| readthedocs/projects/exceptions.py | Added new error message ID FAILED_TO_GET_VERSIONS for repository version retrieval failures |
| readthedocs/projects/tasks/mixins.py | Added exception handling to catch RepositoryError from lsremote() and log warning instead of propagating error |
| readthedocs/rtd_tests/tests/test_backend.py | Added test case to verify RepositoryError is raised when git ls-remote returns non-zero exit code |
| readthedocs/projects/tests/test_build_tasks.py | Added test case to verify sync_versions_task is not called when lsremote() raises RepositoryError |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
humitos
approved these changes
Oct 30, 2025
Co-authored-by: Manuel Kaufmann <humitos@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, if lsremote fails, we are returning an empty list of branches and tags, which is interpreted by our system as if the branches/tags were deleted, which is incorrect.
I was thinking on raising this exception to the user, but I decided against it, as the user won't be able to fix the problem (it could be a temporal issue), if the repo isn't correctly configured, they will get the error when trying to clone the repo.