On Wed, Mar 19, 2014 at 1:21 PM, Hrvoje Ribicic <[email protected]> wrote:
> The formatting functions in qa_utils.py cannot be used by modules
> imported there, such as qa_config. This patch factors the function
> calls into a separate file. Also reorders imports in touched files.
>
> Signed-off-by: Hrvoje Ribicic <[email protected]>
> ---
> Makefile.am | 1 +
> qa/qa_cluster.py | 5 ++--
> qa/qa_logging.py | 73
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qa/qa_rapi.py | 21 ++++++++--------
> qa/qa_utils.py | 47 +-----------------------------------
> 5 files changed, 89 insertions(+), 58 deletions(-)
> create mode 100644 qa/qa_logging.py
>
> diff --git a/Makefile.am b/Makefile.am
> index 2c87d77..d968987 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -809,6 +809,7 @@ qa_scripts = \
> qa/qa_group.py \
> qa/qa_instance.py \
> qa/qa_job.py \
> + qa/qa_logging.py \
Probably it's just a glitch in my mail client, but I see this line
strangely indented. Please, double check that the proper number of
spaces or tabs is used, before pushing the patch.
> qa/qa_node.py \
> qa/qa_os.py \
> qa/qa_rapi.py \
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index 42ffcb9..ecdf369 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -33,9 +33,10 @@ from ganeti import utils
> from ganeti import pathutils
>
> import qa_config
> -import qa_utils
> import qa_error
> import qa_instance
> +import qa_logging
> +import qa_utils
>
> from qa_utils import AssertEqual, AssertCommand, GetCommandOutput
>
> @@ -254,7 +255,7 @@ def TestClusterRename():
> original_name = qa_config.get("name")
> rename_target = qa_config.get("rename", None)
> if rename_target is None:
> - print qa_utils.FormatError('"rename" entry is missing')
> + print qa_logging.FormatError('"rename" entry is missing')
> return
>
> for data in [
> diff --git a/qa/qa_logging.py b/qa/qa_logging.py
> new file mode 100644
> index 0000000..c1a2461
> --- /dev/null
> +++ b/qa/qa_logging.py
> @@ -0,0 +1,73 @@
> +#
> +#
> +
> +# Copyright (C) 2014 Google Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA.
> +
> +""" Handles the logging of messages with appropriate coloring.
> +
> +"""
> +
> +import sys
> +
> +
> +_INFO_SEQ = None
> +_WARNING_SEQ = None
> +_ERROR_SEQ = None
> +_RESET_SEQ = None
> +
> +
> +def _SetupColours():
> + """Initializes the colour constants.
> +
> + """
> + # pylint: disable=W0603
> + # due to global usage
> + global _INFO_SEQ, _WARNING_SEQ, _ERROR_SEQ, _RESET_SEQ
> +
> + # Don't use colours if stdout isn't a terminal
> + if not sys.stdout.isatty():
> + return
> +
> + try:
> + import curses
> + except ImportError:
> + # Don't use colours if curses module can't be imported
> + return
> +
> + curses.setupterm()
> +
> + _RESET_SEQ = curses.tigetstr("op")
> +
> + setaf = curses.tigetstr("setaf")
> + _INFO_SEQ = curses.tparm(setaf, curses.COLOR_GREEN)
> + _WARNING_SEQ = curses.tparm(setaf, curses.COLOR_YELLOW)
> + _ERROR_SEQ = curses.tparm(setaf, curses.COLOR_RED)
> +
> +
> +_SetupColours()
> +
> +
> +def _FormatWithColor(text, seq):
> + if not seq:
> + return text
> + return "%s%s%s" % (seq, text, _RESET_SEQ)
> +
> +
> +FormatWarning = lambda text: _FormatWithColor(text, _WARNING_SEQ)
> +FormatError = lambda text: _FormatWithColor(text, _ERROR_SEQ)
> +FormatInfo = lambda text: _FormatWithColor(text, _INFO_SEQ)
> diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py
> index a68b819..35dc750 100644
> --- a/qa/qa_rapi.py
> +++ b/qa/qa_rapi.py
> @@ -43,8 +43,9 @@ import ganeti.rapi.client # pylint: disable=W0611
> import ganeti.rapi.client_utils
>
> import qa_config
> -import qa_utils
> import qa_error
> +import qa_logging
> +import qa_utils
>
> from qa_instance import IsFailoverSupported
> from qa_instance import IsMigrationSupported
> @@ -90,8 +91,8 @@ def Setup(username, password):
>
> if qa_config.UseVirtualCluster():
> # TODO: Implement full support for RAPI on virtual clusters
> - print qa_utils.FormatWarning("RAPI tests are not yet supported on"
> - " virtual clusters and will be disabled")
> + print qa_logging.FormatWarning("RAPI tests are not yet supported on"
> + " virtual clusters and will be disabled")
>
> assert _rapi_client is None
> else:
> @@ -630,8 +631,8 @@ def TestRapiInstanceRemove(instance, use_client):
> def TestRapiInstanceMigrate(instance):
> """Test migrating instance via RAPI"""
> if not IsMigrationSupported(instance):
> - print qa_utils.FormatInfo("Instance doesn't support migration, skipping"
> - " test")
> + print qa_logging.FormatInfo("Instance doesn't support migration,
> skipping"
> + " test")
> return
> # Move to secondary node
> _WaitForRapiJob(_rapi_client.MigrateInstance(instance.name))
> @@ -644,8 +645,8 @@ def TestRapiInstanceMigrate(instance):
> def TestRapiInstanceFailover(instance):
> """Test failing over instance via RAPI"""
> if not IsFailoverSupported(instance):
> - print qa_utils.FormatInfo("Instance doesn't support failover, skipping"
> - " test")
> + print qa_logging.FormatInfo("Instance doesn't support failover, skipping"
> + " test")
> return
> # Move to secondary node
> _WaitForRapiJob(_rapi_client.FailoverInstance(instance.name))
> @@ -685,7 +686,7 @@ def TestRapiInstanceRenameAndBack(rename_source,
> rename_target):
> def TestRapiInstanceReinstall(instance):
> """Test reinstalling an instance via RAPI"""
> if instance.disk_template == constants.DT_DISKLESS:
> - print qa_utils.FormatInfo("Test not supported for diskless instances")
> + print qa_logging.FormatInfo("Test not supported for diskless instances")
> return
>
> _WaitForRapiJob(_rapi_client.ReinstallInstance(instance.name))
> @@ -701,8 +702,8 @@ def TestRapiInstanceReinstall(instance):
> def TestRapiInstanceReplaceDisks(instance):
> """Test replacing instance disks via RAPI"""
> if not IsDiskReplacingSupported(instance):
> - print qa_utils.FormatInfo("Instance doesn't support disk replacing,"
> - " skipping test")
> + print qa_logging.FormatInfo("Instance doesn't support disk replacing,"
> + " skipping test")
> return
> fn = _rapi_client.ReplaceInstanceDisks
> _WaitForRapiJob(fn(instance.name,
> diff --git a/qa/qa_utils.py b/qa/qa_utils.py
> index 7ec0d49..2b3553b 100644
> --- a/qa/qa_utils.py
> +++ b/qa/qa_utils.py
> @@ -48,11 +48,8 @@ from ganeti import vcluster
> import qa_config
> import qa_error
>
> +from qa_logging import FormatInfo
>
> -_INFO_SEQ = None
> -_WARNING_SEQ = None
> -_ERROR_SEQ = None
> -_RESET_SEQ = None
>
> _MULTIPLEXERS = {}
>
> @@ -70,37 +67,6 @@ _QA_OUTPUT = pathutils.GetLogFilename("qa-output")
> RETURN_VALUE) = range(1000, 1002)
>
>
> -def _SetupColours():
> - """Initializes the colour constants.
> -
> - """
> - # pylint: disable=W0603
> - # due to global usage
> - global _INFO_SEQ, _WARNING_SEQ, _ERROR_SEQ, _RESET_SEQ
> -
> - # Don't use colours if stdout isn't a terminal
> - if not sys.stdout.isatty():
> - return
> -
> - try:
> - import curses
> - except ImportError:
> - # Don't use colours if curses module can't be imported
> - return
> -
> - curses.setupterm()
> -
> - _RESET_SEQ = curses.tigetstr("op")
> -
> - setaf = curses.tigetstr("setaf")
> - _INFO_SEQ = curses.tparm(setaf, curses.COLOR_GREEN)
> - _WARNING_SEQ = curses.tparm(setaf, curses.COLOR_YELLOW)
> - _ERROR_SEQ = curses.tparm(setaf, curses.COLOR_RED)
> -
> -
> -_SetupColours()
> -
> -
> def AssertIn(item, sequence):
> """Raises an error when item is not in sequence.
>
> @@ -575,17 +541,6 @@ def GenericQueryFieldsTest(cmd, fields):
> constants.EXIT_UNKNOWN_FIELD)
>
>
> -def _FormatWithColor(text, seq):
> - if not seq:
> - return text
> - return "%s%s%s" % (seq, text, _RESET_SEQ)
> -
> -
> -FormatWarning = lambda text: _FormatWithColor(text, _WARNING_SEQ)
> -FormatError = lambda text: _FormatWithColor(text, _ERROR_SEQ)
> -FormatInfo = lambda text: _FormatWithColor(text, _INFO_SEQ)
> -
> -
> def AddToEtcHosts(hostnames):
> """Adds hostnames to /etc/hosts.
>
> --
> 1.9.0.279.gdc9e3eb
>
LGTM, thanks.
Michele
--
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores