Thanks for this cleanup.

On Thu, Nov 06, 2014 at 12:30:39AM +0200, Alex Pyrgiotis wrote:
> Convert the index operations that are used in "add" to functions, so
> that they can be used by "attach" later on.
>
> Signed-off-by: Alex Pyrgiotis <[email protected]>
>
> diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py
> index f59afc7..f3b6a40 100644
> --- a/lib/cmdlib/instance_utils.py
> +++ b/lib/cmdlib/instance_utils.py
> @@ -908,36 +908,13 @@ def ApplyContainerMods(kind, container, chgdesc,
mods,
>      changes = None
>
>      if op == constants.DDM_ADD:
> -      # Calculate where item will be added
> -      # When adding an item, identifier can only be an index
> -      try:
> -        idx = int(identifier)
> -      except ValueError:
> -        raise errors.OpPrereqError("Only possitive integer or -1 is
accepted as"
> -                                   " identifier for %s" %
constants.DDM_ADD,
> -                                   errors.ECODE_INVAL)
> -      if idx == -1:
> -        addidx = len(container)
> -      else:
> -        if idx < 0:
> -          raise IndexError("Not accepting negative indices other than
-1")
> -        elif idx > len(container):
> -          raise IndexError("Got %s index %s, but there are only %s" %
> -                           (kind, idx, len(container)))
> -        addidx = idx
> -
> +      addidx = GetIndexFromIdentifier(identifier, kind, container)
>        if create_fn is None:
>          item = params
>        else:
>          (item, changes) = create_fn(addidx, params, private)
>
> -      if idx == -1:
> -        container.append(item)
> -      else:
> -        assert idx >= 0
> -        assert idx <= len(container)
> -        # list.insert does so before the specified index
> -        container.insert(idx, item)
> +      InsertItemToIndex(int(identifier), item, container)

nitpick: make conversion in the function, so that the call is ~equal to
the one to GetIndexFromIdentifier.

>
>        if post_add_fn is not None:
>          post_add_fn(addidx, item)
> @@ -1006,6 +983,43 @@ def GetItemFromContainer(identifier, kind,
container):
>                               (kind, identifier), errors.ECODE_NOENT)
>
>
> +def GetIndexFromIdentifier(identifier, kind, container):
> +  """Check if the identifier represents a valid container index and
return it.
> +
> +  Used in "add" and "attach" actions.

Please add a description for the parameters.

> +  """
> +  try:
> +    idx = int(identifier)
> +  except ValueError:
> +    raise errors.OpPrereqError("Only positive integer or -1 is accepted
as"
> +                               " identifier for add/attach",
errors.ECODE_INVAL)
> +  if idx == -1:
> +    addidx = len(container)

nitpick: return here.

> +  else:
> +    if idx < 0:
> +      raise IndexError("Not accepting negative indices other than -1")
> +    elif idx > len(container):
> +      raise IndexError("Got %s index %s, but there are only %s" %
> +                        (kind, idx, len(container)))
> +    addidx = idx

same

> +
> +  return addidx
> +
> +
> +def InsertItemToIndex(idx, item, container):
> +  """Insert an item to the provided index of a container.
> +
> +  Used in "add" and "attach" actions.
> +  """
> +  if idx == -1:
> +    container.append(item)
> +  else:
> +    assert idx >= 0
> +    assert idx <= len(container)
> +    # list.insert does so before the specified index
> +    container.insert(idx, item)
> +
> +
>  def CheckNodesPhysicalCPUs(lu, node_uuids, requested, hypervisor_specs):
>    """Checks if nodes have enough physical CPUs
>
> --
> 1.7.10.4
>

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