Even with the nested try-exception code, I'm OK with this patch :-D -- Reviewed-by: Paulo Vital <pvital@linux.vnet.ibm.com> On Tue, 2014-03-11 at 17:34 -0300, Aline Manera wrote:

From: Aline Manera <alinefm@br.ibm.com>

Instead of using platform.linux_distribution() and keep a list of possible values to match, this patch verifies the tools on the system to identify which update tool to use in Kimchi. So try to import yum module, apt module or run the zypper command and in last case raise an error.

Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/swupdate.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/kimchi/swupdate.py b/src/kimchi/swupdate.py index 2c2713a..ff5c9d1 100644 --- a/src/kimchi/swupdate.py +++ b/src/kimchi/swupdate.py @@ -17,7 +17,6 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

-import platform import subprocess import time

@@ -25,11 +24,6 @@ from kimchi.basemodel import Singleton from kimchi.exception import NotFoundError, OperationFailed from kimchi.utils import kimchi_log, run_command

-YUM_DISTROS = ['fedora', 'red hat enterprise linux', - 'red hat enterprise linux server'] -APT_DISTROS = ['debian', 'ubuntu'] -ZYPPER_DISTROS = ['opensuse ', 'suse linux enterprise server '] -

class SoftwareUpdate(object): __metaclass__ = Singleton @@ -51,19 +45,24 @@ class SoftwareUpdate(object):

# Get the distro of host machine and creates an object related to # correct package management system - self._distro = platform.linux_distribution()[0].lower() - if (self._distro in YUM_DISTROS): + try: + __import__('yum') kimchi_log.info("Loading YumUpdate features.") self._pkg_mnger = YumUpdate() - elif (self._distro in APT_DISTROS): - kimchi_log.info("Loading AptUpdate features.") - self._pkg_mnger = AptUpdate() - elif (self._distro in ZYPPER_DISTROS): - kimchi_log.info("Loading ZypperUpdate features.") - self._pkg_mnger = ZypperUpdate() - else: - raise Exception("There is no compatible package manager for " - "this system.") + except ImportError: + try: + __import__('apt') + kimchi_log.info("Loading AptUpdate features.") + self._pkg_mnger = AptUpdate() + except ImportError: + zypper_help = "zypper --help" + (stdout, stderr, returncode) = run_command(zypper_help) + if returncode == 0: + kimchi_log.info("Loading ZypperUpdate features.") + self._pkg_mnger = ZypperUpdate() + else: + raise Exception("There is no compatible package manager " + "for this system.")

def _scanUpdates(self): """

Reviewed-by: Adam King <rak@linux.vnet.ibm.com> On 03/11/2014 04:34 PM, Aline Manera wrote:

From: Aline Manera <alinefm@br.ibm.com>

Instead of using platform.linux_distribution() and keep a list of possible values to match, this patch verifies the tools in the system to identify which repository management tool to use in Kimchi. So try to import yum module or apt_pkg module and in last case raise an error.

Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/repositories.py | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index 2e70008..2727c84 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -18,15 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

import os -import platform

from kimchi.basemodel import Singleton from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import OperationFailed, NotFoundError, MissingParameter

-YUM_DISTROS = ['fedora', 'opensuse ', 'suse linux enterprise server '] -APT_DISTROS = ['debian', 'ubuntu'] -

class Repositories(object): __metaclass__ = Singleton @@ -43,20 +39,19 @@ class Repositories(object): # 'enabled': True/False, 'gpgcheck': True/False, # 'gpgkey': ([<string>], None), # 'is_mirror': True/False} - - self._repo_storage = {} - - self._distro = platform.linux_distribution()[0].lower() - if (self._distro in YUM_DISTROS or self._distro.startswith('red hat')): + try: + __import__('yum') self._pkg_mnger = YumRepo() - elif (self._distro in APT_DISTROS): - self._pkg_mnger = AptRepo() - else: - raise InvalidOperation("KCHREPOS0014E") + except ImportError: + try: + __import__('apt_pkg') + self._pkg_mnger = AptRepo() + except ImportError: + raise InvalidOperation('KCHREPOS0019E')

- if self._pkg_mnger: - # update the self._repo_storage with system's repositories - self._scanSystemRepositories() + self._repo_storage = {} + # update the self._repo_storage with system's repositories + self._scanSystemRepositories()

def _scanSystemRepositories(self): """

-- Adam King <rak@linux.vnet.ibm.com> IBM CSI

-- Reviewed-by: Paulo Vital <pvital@linux.vnet.ibm.com> On Tue, 2014-03-11 at 17:34 -0300, Aline Manera wrote:

From: Aline Manera <alinefm@br.ibm.com>

Probably, in some cases Kimchi can not be able to identify which repository management tool to use so expose this information to properly update the UI

Also update the API.md and test case to reflect this change

Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- docs/API.md | 2 ++ src/kimchi/mockmodel.py | 4 +++- src/kimchi/model/config.py | 11 ++++++++++- tests/test_rest.py | 2 ++ 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/docs/API.md b/docs/API.md index b3b6c49..698fc66 100644 --- a/docs/API.md +++ b/docs/API.md @@ -579,6 +579,8 @@ creation. the system; False, otherwise. * update_tool: True if there is a compatible package manager for the system; False, otherwise + * repo_mngt_tool: True if there is a compatible repository management tool + for the system; False, otherwise * **POST**: *See Configuration Actions*

**Actions (POST):** diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index b842b6c..72764b0 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -67,7 +67,9 @@ class MockModel(object): ['http', 'https', 'ftp', 'ftps', 'tftp'], 'qemu_stream': True, 'screenshot': True, - 'system_report_tool': True} + 'system_report_tool': True, + 'update_tool': True, + 'repo_mngt_tool': True}

def reset(self): self._mock_vms = {} diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index 3580ad3..5b27188 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -27,6 +27,7 @@ from kimchi.distroloader import DistroLoader from kimchi.exception import NotFoundError from kimchi.featuretests import FeatureTests from kimchi.model.debugreports import DebugReportsModel +from kimchi.repositories import Repositories from kimchi.screenshot import VMScreenshot from kimchi.swupdate import SoftwareUpdate from kimchi.utils import check_url_path, kimchi_log @@ -80,11 +81,19 @@ class CapabilitiesModel(object): else: update_tool = True

+ try: + Repositories() + except Exception: + repo_mngt_tool = False + else: + repo_mngt_tool = True + return {'libvirt_stream_protocols': self.libvirt_stream_protocols, 'qemu_stream': self.qemu_stream, 'screenshot': VMScreenshot.get_stream_test_result(), 'system_report_tool': bool(report_tool), - 'update_tool': update_tool} + 'update_tool': update_tool, + 'repo_mngt_tool': repo_mngt_tool}

class DistrosModel(object): diff --git a/tests/test_rest.py b/tests/test_rest.py index 65d50e0..92c1a83 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1340,6 +1340,8 @@ class RestTests(unittest.TestCase): self.assertIn('qemu_stream', conf) self.assertIn('screenshot', conf) self.assertIn('system_report_tool', conf) + self.assertIn('update_tool', conf) + self.assertIn('repo_mngt_tool', conf)

def test_auth_unprotected(self): hdrs = {'AUTHORIZATION': ''}

Reviewed-by: Adam King <rak@linux.vnet.ibm.com> On 03/11/2014 04:34 PM, Aline Manera wrote:

From: Aline Manera <alinefm@br.ibm.com>

Kimchi should not block user to use its functionalities if a repository management tool was not identified. So allow user starts up kimchi and raise an error only if he/she tries to use the repository management feature.

Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/i18n.py | 2 +- src/kimchi/model/host.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 1ae3889..ddb2fcb 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -238,7 +238,7 @@ messages = { "KCHREPOS0011E": _("There is no disabled repository called %(repo_id)s."), "KCHREPOS0012E": _("There is no enabled repository called %(repo_id)s."), "KCHREPOS0013E": _("There are no parameters to update repository."), - "KCHREPOS0014E": _("OS distro not supported."), + "KCHREPOS0014E": _("Repository management tool was not recognized for your system."), "KCHREPOS0015E": _("There is no YUM configuration directory."), "KCHREPOS0016E": _("There are no parameters to create a new repo file."), "KCHREPOS0017E": _("Could not write repo file %(repo_file)s"), diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index a7db7c9..b195a97 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -310,12 +310,21 @@ class PackageUpdateModel(object):

class RepositoriesModel(object): def __init__(self, **kargs): - self.host_repositories = Repositories() + try: + self.host_repositories = Repositories() + except: + self.host_repositories = None

def get_list(self): + if self.host_repositories is None: + raise InvalidOperation('KCHREPOS0014E') + return self.host_repositories.getRepositories().keys()

def create(self, params): + if self.host_repositories is None: + raise InvalidOperation('KCHREPOS0014E') + repo_id = params.get('repo_id', None)

# Create a repo_id if not given by user. The repo_id will follow @@ -335,20 +344,35 @@ class RepositoriesModel(object):

class RepositoryModel(object): def __init__(self, **kargs): - self._repositories = Repositories() + try: + self._repositories = Repositories() + except: + self._repositories = None

def lookup(self, repo_id): + if self._repositories is None: + raise InvalidOperation('KCHREPOS0014E') + return self._repositories.getRepository(repo_id)

def enable(self, repo_id): + if self._repositories is None: + raise InvalidOperation('KCHREPOS0014E') + if not self._repositories.enableRepository(repo_id): raise OperationFailed("KCHREPOS0007E", {'repo_id': repo_id})

def disable(self, repo_id): + if self._repositories is None: + raise InvalidOperation('KCHREPOS0014E') + if not self._repositories.disableRepository(repo_id): raise OperationFailed("KCHREPOS0008E", {'repo_id': repo_id})

def update(self, repo_id, params): + if self._repositories is None: + raise InvalidOperation('KCHREPOS0014E') + try: self._repositories.updateRepository(repo_id, params) except: @@ -356,4 +380,7 @@ class RepositoryModel(object): return repo_id

def delete(self, repo_id): + if self._repositories is None: + raise InvalidOperation('KCHREPOS0014E') + return self._repositories.removeRepository(repo_id)

-- Adam King <rak@linux.vnet.ibm.com> IBM CSI