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

repo: catch error updating the package cache #2079

Merged
merged 4 commits into from Apr 25, 2018
Merged

repo: catch error updating the package cache #2079

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0937e00
repo: catch error updating the package cache
kalikiana Apr 16, 2018
37f3fae
Clear index if cache update fails and try again
Apr 24, 2018
2244528
Drop " copy" from _HASHSUM_MISMATCH_PATTERN
Apr 24, 2018
e7e267a
Merge branch 'master' into cache-update-error
Apr 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions snapcraft/internal/repo/_deb.py
View file
Open in desktop
Expand Up @@ -122,8 +122,11 @@ def _setup_apt(self, cache_dir):
"Cannot find 'dpkg' command needed to support multiarch")

apt_cache = apt.Cache(rootdir=cache_dir, memonly=True)
apt_cache.update(fetch_progress=self.progress,
sources_list=sources_list_file)
try:
apt_cache.update(fetch_progress=self.progress,
sources_list=sources_list_file)
except apt.cache.FetchFailedException:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the current exception message? We should consider printing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't assume that there is a message. If there is one, like in the linked bug, the message might look like so:

E:Failed to fetch copy:/home/alan/.cache/snapcraft/stage-packages/apt/a810ed86fa085532f20ab86eb7554a1ff527e7f8f46e49c343abc3d7c4d141ae0c39e374361c05f4561cf2aec6689aac/var/lib/apt/lists/partial/security.ubuntu.com_ubuntu_dists_xenial-security_multiverse_dep11_icons-64x64.tar.gz Hash Sum mismatch, E:Failed to fetch copy:/home/alan/.cache/snapcraft/stage-packages/apt/a810ed86fa085532f20ab86eb7554a1ff527e7f8f46e49c343abc3d7c4d141ae0c39e374361c05f4561cf2aec6689aac/var/lib/apt/lists/partial/gb.archive.ubuntu.com_ubuntu_dists_xenial-backports_main_dep11_icons-64x64.tar.gz Hash Sum mismatch, E:Some index files failed to download. They have been ignored, or old ones used instead.

To my mind this doesn't seem very user-friendly. In the same situation, the printed output before that will have Err http://security.ubuntu.com/ubuntu xenial-security/multiverse DEP-11 128x128 Icons Hash Sum mismatch at the end, so I'd rather be looking at that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the messages to the error (if they're present). And the index will be cleared if it's a hashsum mismatch (regex) so Snapcraft can continue as normal after that, meaning ideally the error should not be surfacing to the user if that was the issue.

raise errors.CacheUpdateFailedError()

return apt_cache

Expand Down
12 changes: 12 additions & 0 deletions snapcraft/internal/repo/errors.py
View file
Open in desktop
Expand Up @@ -38,6 +38,18 @@ def __init__(self):
super().__init__(distro=distro)


class CacheUpdateFailedError(RepoError):

fmt = (
"Failed to update the package cache: "
"Some files could not be downloaded. "
"Check that the sources on your host are configured correctly."
)

def __init__(self):
super().__init__()


class BuildPackageNotFoundError(RepoError):

fmt = "Could not find a required package in 'build-packages': {package}"
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/repo/test_deb.py
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import apt
import os
from subprocess import CalledProcessError
from unittest.mock import ANY, call, patch, MagicMock
Expand Down Expand Up @@ -52,6 +53,16 @@ def _fetch_binary(download_dir, **kwargs):
self.mock_cache.return_value.get_changes.return_value = [
self.mock_package]

def test_cache_update_failed(self):
self.mock_cache().update.side_effect = apt.cache.FetchFailedException()
project_options = snapcraft.ProjectOptions(
use_geoip=False)
ubuntu = repo.Ubuntu(self.tempdir, project_options=project_options)
self.assertRaises(
errors.CacheUpdateFailedError,
ubuntu.get,
['fake-package'])

def test_get_pkg_name_parts_name_only(self):
name, version = repo.get_pkg_name_parts('hello')
self.assertThat(name, Equals('hello'))
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/test_errors.py
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@

from snapcraft.internal import errors
from snapcraft.internal.meta import _errors as meta_errors
from snapcraft.internal.repo import errors as repo_errors
from tests import unit


Expand Down Expand Up @@ -429,6 +430,15 @@ class ErrorFormattingTestCase(unit.TestCase):
"again."
)
}),
('CacheUpdateFailedError', {
'exception': repo_errors.CacheUpdateFailedError,
'kwargs': {},
'expected_message': (
"Failed to update the package cache: "
"Some files could not be downloaded. "
"Check that the sources on your host are configured correctly."
)
}),
)

def test_error_formatting(self):
Expand Down