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

Reply via email to