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

Raise exception in failed checkout #5035

Merged
merged 4 commits into from
Jan 10, 2019
Merged

Raise exception in failed checkout #5035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbd56d2
Raise RepositoryError onf failed checkout
stsewd Dec 24, 2018
73ed82f
Raise RepositoryError for bzr and hg
stsewd Dec 24, 2018
af128a6
Test
stsewd Dec 24, 2018
1c19302
Rename test
stsewd Dec 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions readthedocs/projects/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class RepositoryError(BuildEnvironmentError):
'You can not have two versions with the name latest or stable.'
)

FAILED_TO_CHECKOUT = _(
'Failed to checkout revision: {}'
Copy link
Member

Choose a reason for hiding this comment

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

Will this exception log to Sentry? It should not.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. Actually I'm not sure, I haven't seen a RepositoryError in sentry

Copy link
Member

Choose a reason for hiding this comment

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

There are some,

https://sentry.io/read-the-docs/readthedocs-org/issues/713027767/?query=RepositoryError

There are some places where it's handled

https://github.com/rtfd/readthedocs.org/blob/d2ea32ae7c25aea609970e598bafa404c0316d38/readthedocs/projects/tasks.py#L252-L273

and there other where it's not:

https://github.com/rtfd/readthedocs.org/blob/d2ea32ae7c25aea609970e598bafa404c0316d38/readthedocs/projects/tasks.py#L211-L215

I suppose that this is the final solution, #3856

Copy link
Member

Choose a reason for hiding this comment

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

Going to merge this, and if it logs, we can handle it later.

)

def get_default_message(self):
if settings.ALLOW_PRIVATE_REPOS:
return self.PRIVATE_ALLOWED
Expand Down
22 changes: 22 additions & 0 deletions readthedocs/rtd_tests/tests/test_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ def test_git_update_and_checkout(self):
self.assertEqual(code, 0)
self.assertTrue(exists(repo.working_dir))

def test_git_checkout_invalid_revision(self):
repo = self.project.vcs_repo()
repo.update()
version = 'invalid-revision'
with self.assertRaises(RepositoryError) as e:
repo.checkout(version)
self.assertEqual(
str(e.exception),
RepositoryError.FAILED_TO_CHECKOUT.format(version)
)

def test_git_tags(self):
repo_path = self.project.repo
create_git_tag(repo_path, 'v01')
Expand Down Expand Up @@ -254,6 +265,17 @@ def test_update_and_checkout(self):
self.assertEqual(code, 0)
self.assertTrue(exists(repo.working_dir))

def test_checkout_invalid_revision(self):
repo = self.project.vcs_repo()
repo.update()
version = 'invalid-revision'
with self.assertRaises(RepositoryError) as e:
repo.checkout(version)
self.assertEqual(
str(e.exception),
RepositoryError.FAILED_TO_CHECKOUT.format(version)
)

def test_parse_tags(self):
data = """\
tip 13575:8e94a1b4e9a4
Expand Down
7 changes: 6 additions & 1 deletion readthedocs/vcs_support/backends/bzr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,9 @@ def checkout(self, identifier=None):
super(Backend, self).checkout()
if not identifier:
return self.up()
return self.run('bzr', 'switch', identifier)
exit_code, stdout, stderr = self.run('bzr', 'switch', identifier)
if exit_code != 0:
raise RepositoryError(
RepositoryError.FAILED_TO_CHECKOUT.format(identifier)
)
return exit_code, stdout, stderr
4 changes: 3 additions & 1 deletion readthedocs/vcs_support/backends/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ def checkout_revision(self, revision=None):

code, out, err = self.run('git', 'checkout', '--force', revision)
if code != 0:
log.warning("Failed to checkout revision '%s': %s", revision, code)
raise RepositoryError(
RepositoryError.FAILED_TO_CHECKOUT.format(revision)
)
return [code, out, err]

def clone(self):
Expand Down
9 changes: 8 additions & 1 deletion readthedocs/vcs_support/backends/hg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,11 @@ def checkout(self, identifier=None):
super(Backend, self).checkout()
if not identifier:
identifier = 'tip'
return self.run('hg', 'update', '--clean', identifier)
exit_code, stdout, stderr = self.run(
'hg', 'update', '--clean', identifier
)
if exit_code != 0:
raise RepositoryError(
RepositoryError.FAILED_TO_CHECKOUT.format(identifier)
)
return exit_code, stdout, stderr