On 11/10/2014 05:51 PM, Aaron Karper wrote:
> 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] <mailto:[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.
>
ACK
>>
>> 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.
>
ACK
>>
>> + """
>> + 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.
>
ACK
>
>> + 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
>
ACK
>>
>> +
>> + 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
--
Alex | [email protected]
