On Wed, Mar 19, 2014 at 1:48 PM, Michele Tartara <[email protected]>wrote:

> 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.
>
>
You are completely right, I was editing this on the command line with an
improperly setup editor.
Thanks!


> >         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