Re: [PATCH master 1/7] Factor out check for storage params

[Helga Velroyen]

On Tue, Jul 9, 2013 at 1:24 PM, Klaus Aehlig <aeh...@google.com> wrote:

> On Tue, Jul 09, 2013 at 12:14:38PM +0200, Helga Velroyen wrote:
> > This patch factors out the sanity checks for storage
> > parameters in its own function. Unit tests included.
> >
> > Signed-off-by: Helga Velroyen <hel...@google.com>
> > ---
> >  lib/backend.py                     | 32 ++++++++++++++++++++++----------
> >  test/py/ganeti.backend_unittest.py | 35
> ++++++++++++++++++++++++++++-------
> >  2 files changed, 50 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/backend.py b/lib/backend.py
> > index 09b9984..cb25b64 100644
> > --- a/lib/backend.py
> > +++ b/lib/backend.py
> > @@ -573,25 +573,37 @@ def LeaveCluster(modify_ssh_setup):
> >    raise errors.QuitGanetiException(True, "Shutdown scheduled")
> >
> >
> > -def _GetLvmVgSpaceInfo(name, params):
> > -  """Wrapper around C{_GetVgInfo} which checks the storage parameters.
> > +def _CheckStorageParams(params, num_params):
> > +  """Performs sanity checks for storage parameters.
> >
> > -  @type name: string
> > -  @param name: name of the volume group
> >    @type params: list
> > -  @param params: list of storage parameters, which in this case should
> be
> > -    containing only one for exclusive storage
> > +  @param params: list of storage parameters
> > +  @type num_params: int
> > +  @param num_params: expected number of parameters
> >
> >    """
> >    if params is None:
> > -    raise errors.ProgrammerError("No storage parameter for LVM vg
> storage"
> > +    raise errors.ProgrammerError("No storage parameters for storage"
> >                                   " reporting is provided.")
> >    if not isinstance(params, list):
> >      raise errors.ProgrammerError("The storage parameters are not of
> type"
> >                                   " list: '%s'" % params)
> > -  if not len(params) == 1:
> > +  if not len(params) == num_params:
> >      raise errors.ProgrammerError("Received more than one storage
> parameter:"
>
> If you change the test, please also change the error message to reflect
> the new
> number of parameters expected.
>

Oh, right. Thanks for noticing. FYI, interdiff:

diff --git a/lib/backend.py b/lib/backend.py
index 025c0a5..8dd739b 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -589,8 +589,9 @@ def _CheckStorageParams(params, num_params):
     raise errors.ProgrammerError("The storage parameters are not of type"
                                  " list: '%s'" % params)
   if not len(params) == num_params:
-    raise errors.ProgrammerError("Received more than one storage
parameter:"
-                                 " '%s'" % params)
+    raise errors.ProgrammerError("Did not receive the expected number of"
+                                 "storage parameters: expected %s,"
+                                 " received '%s'" % (num_params,
len(params)))



>
> >                                   " '%s'" % params)
> > +
> > +
>
> Rest LGTM.
>
>