Skip to content

Commit 582aa15

Browse files
stsewdhumitos
andauthored
Version sync: skip syncing versions when lsremote fails (#12540)
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. --------- Co-authored-by: Manuel Kaufmann <[email protected]>
1 parent e3bc644 commit 582aa15

File tree

5 files changed

+40
-5
lines changed

5 files changed

+40
-5
lines changed

readthedocs/projects/exceptions.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class RepositoryError(BuildUserError):
2424
FAILED_TO_CHECKOUT = "project:repository:checkout-failed"
2525
GENERIC = "project:repository:generic-error"
2626
UNSUPPORTED_VCS = "project:repository:unsupported-vcs"
27+
FAILED_TO_GET_VERSIONS = "project:repository:failed-to-get-versions"
2728

2829

2930
class SyncRepositoryLocked(BuildAppError):

readthedocs/projects/tasks/mixins.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,17 @@ def sync_versions(self, vcs_repository):
4444
# check this here and do not depend on ``vcs_repository``.
4545
sync_tags = not self.data.project.has_feature(Feature.SKIP_SYNC_TAGS)
4646
sync_branches = not self.data.project.has_feature(Feature.SKIP_SYNC_BRANCHES)
47-
branches, tags = vcs_repository.lsremote(
48-
include_tags=sync_tags,
49-
include_branches=sync_branches,
50-
)
47+
try:
48+
branches, tags = vcs_repository.lsremote(
49+
include_tags=sync_tags,
50+
include_branches=sync_branches,
51+
)
52+
except RepositoryError:
53+
log.warning(
54+
"Error running lsremote to get versions from the repository.",
55+
exc_info=True,
56+
)
57+
return
5158

5259
tags_data = [
5360
{

readthedocs/projects/tests/test_build_tasks.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,3 +3040,10 @@ def test_check_duplicate_reserved_version_latest(self, on_failure, verbose_name)
30403040
exception = on_failure.call_args[0][0]
30413041
assert isinstance(exception, RepositoryError) == True
30423042
assert exception.message_id == RepositoryError.DUPLICATED_RESERVED_VERSIONS
3043+
3044+
@mock.patch("readthedocs.builds.tasks.sync_versions_task")
3045+
@mock.patch("readthedocs.vcs_support.backends.git.Backend.lsremote")
3046+
def test_skip_sync_version_task_if_lsremote_fails(self, lsremote, sync_versions_task):
3047+
lsremote.side_effect = RepositoryError(RepositoryError.FAILED_TO_GET_VERSIONS)
3048+
self._trigger_sync_repository_task()
3049+
sync_versions_task.assert_not_called()

readthedocs/rtd_tests/tests/test_backend.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,23 @@ def test_git_lsremote_branches_only(self):
146146
{branch.verbose_name: branch.identifier for branch in repo_branches},
147147
)
148148

149+
def test_git_lsremote_fails(self):
150+
version = self.project.versions.first()
151+
repo = self.project.vcs_repo(environment=self.build_environment, version=version)
152+
with mock.patch(
153+
"readthedocs.vcs_support.backends.git.Backend.run",
154+
return_value=(1, "Error", ""),
155+
):
156+
with self.assertRaises(RepositoryError) as e:
157+
repo.lsremote(
158+
include_tags=True,
159+
include_branches=True,
160+
)
161+
self.assertEqual(
162+
e.exception.message_id,
163+
RepositoryError.FAILED_TO_GET_VERSIONS,
164+
)
165+
149166
def test_git_update_and_checkout(self):
150167
version = self.project.versions.first()
151168
repo = self.project.vcs_repo(environment=self.build_environment, version=version)

readthedocs/vcs_support/backends/git.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,10 @@ def lsremote(self, include_tags=True, include_branches=True):
397397
cmd = ["git", "ls-remote", *extra_args, self.repo_url]
398398

399399
self.check_working_dir()
400-
_, stdout, _ = self.run(*cmd, demux=True, record=False)
400+
exit_code, stdout, _ = self.run(*cmd, demux=True, record=False)
401+
402+
if exit_code != 0:
403+
raise RepositoryError(message_id=RepositoryError.FAILED_TO_GET_VERSIONS)
401404

402405
branches = []
403406
# Git has two types of tags: lightweight and annotated.

0 commit comments

Comments
 (0)