Clarify HAVE_PYTHON_APT/install_python_apt handling in apt_repository

1. Don't test check_mode in both the caller and in the callee.
2. Don't test HAVE_PYTHON_APT inside an if that tests HAVE_PYTHON_APT
3. Don't be irritatingly vague about why the module fails ("You may be
   seeing this because…").

Note that if «apt-get -y install python-apt» succeeds with rc==0, but
for some reason python_apt is not usable afterwards, this will break
because the imports in install_python_apt aren't wrapped inside a
try/except.

In other words, we assume that install_python_apt either succeeds or
fails with a traceback. This commit doesn't affect that behaviour.
pull/18777/head
Abhijit Menon-Sen 9 years ago committed by Matt Clay
parent 7643d3aecf
commit 46c015375d

@ -423,24 +423,24 @@ def main():
) )
params = module.params params = module.params
if params['install_python_apt'] and not HAVE_PYTHON_APT and not module.check_mode:
install_python_apt(module)
repo = module.params['repo'] repo = module.params['repo']
state = module.params['state'] state = module.params['state']
update_cache = module.params['update_cache'] update_cache = module.params['update_cache']
sourceslist = None sourceslist = None
if HAVE_PYTHON_APT: if not HAVE_PYTHON_APT:
if params['install_python_apt']:
install_python_apt(module)
else:
module.fail_json(msg='python-apt is not installed, and install_python_apt is False')
if isinstance(distro, aptsources_distro.UbuntuDistribution): if isinstance(distro, aptsources_distro.UbuntuDistribution):
sourceslist = UbuntuSourcesList(module, sourceslist = UbuntuSourcesList(module,
add_ppa_signing_keys_callback=get_add_ppa_signing_key_callback(module)) add_ppa_signing_keys_callback=get_add_ppa_signing_key_callback(module))
elif HAVE_PYTHON_APT and \ elif isinstance(distro, aptsources_distro.DebianDistribution) or isinstance(distro, aptsources_distro.Distribution):
isinstance(distro, aptsources_distro.DebianDistribution) or isinstance(distro, aptsources_distro.Distribution):
sourceslist = SourcesList() sourceslist = SourcesList()
else: else:
module.fail_json(msg='Module apt_repository supports only Debian and Ubuntu. ' + \ module.fail_json(msg='Module apt_repository supports only Debian and Ubuntu.')
'You may be seeing this because python-apt is not installed, but you requested that it not be auto-installed')
sources_before = sourceslist.dump() sources_before = sourceslist.dump()

Loading…
Cancel
Save