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