Re: [PATCH master 3/8] RAPI: Use FillOpCode for modifying node group

[Rene Nussbaumer]

On Mon, Feb 14, 2011 at 5:37 PM, Michael Hanselmann <han...@google.com> wrote:
> ---
>  doc/rapi.rst                       |    4 ++--
>  lib/rapi/rlib2.py                  |    6 ++++--
>  test/ganeti.rapi.rlib2_unittest.py |   34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/doc/rapi.rst b/doc/rapi.rst
> index 97a7b73..da70366 100644
> --- a/doc/rapi.rst
> +++ b/doc/rapi.rst
> @@ -451,8 +451,8 @@ Returns a job ID.
>
>  Body parameters:
>
> -``alloc_policy`` (string)
> -  If present, the new allocation policy for the node group.
> +.. opcode_params:: OP_GROUP_SET_PARAMS
> +   :exclude: group_name
>
>
>  ``/2/groups/[group_name]/rename``
> diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
> index 4f335c2..e70044b 100644
> --- a/lib/rapi/rlib2.py
> +++ b/lib/rapi/rlib2.py
> @@ -628,8 +628,10 @@ def _ParseModifyGroupRequest(name, data):
>   @return: Group modify opcode
>
>   """
> -  alloc_policy = baserlib.CheckParameter(data, "alloc_policy", default=None)
> -  return opcodes.OpGroupSetParams(group_name=name, alloc_policy=alloc_policy)
> +  return baserlib.FillOpcode(opcodes.OpGroupSetParams, data, {
> +    "group_name": name,
> +    })
> +
>
>
>  class R_2_groups_name_modify(baserlib.R_Generic):
> diff --git a/test/ganeti.rapi.rlib2_unittest.py 
> b/test/ganeti.rapi.rlib2_unittest.py
> index 395a919..6246f37 100755
> --- a/test/ganeti.rapi.rlib2_unittest.py
> +++ b/test/ganeti.rapi.rlib2_unittest.py
> @@ -476,5 +476,39 @@ class 
> TestParseInstanceReplaceDisksRequest(unittest.TestCase):
>                       })
>
>
> +class TestParseModifyGroupRequest(unittest.TestCase):
> +  def setUp(self):
> +    self.Parse = rlib2._ParseModifyGroupRequest
> +
> +  def test(self):
> +    name = "group6002"
> +
> +    for policy in constants.VALID_ALLOC_POLICIES:
> +      data = {
> +        "alloc_policy": policy,
> +        }
> +
> +      op = self.Parse(name, data)
> +      self.assert_(isinstance(op, opcodes.OpGroupSetParams))
> +      self.assertEqual(op.group_name, name)
> +      self.assertEqual(op.alloc_policy, policy)
> +
> +  def testUnknownPolicy(self):
> +    data = {
> +      "alloc_policy": "_unknown_policy_",
> +      }
> +
> +    self.assertRaises(http.HttpBadRequest, self.Parse, "name", data)
> +
> +  def testDefaults(self):
> +    name = "group6679"
> +    data = {}
> +
> +    op = self.Parse(name, data)
> +    self.assert_(isinstance(op, opcodes.OpGroupSetParams))
> +    self.assertEqual(op.group_name, name)
> +    self.assertFalse(hasattr(op, "alloc_policy"))
> +
> +
>  if __name__ == '__main__':
>   testutils.GanetiTestProgram()
> --
> 1.7.3.5

LGTM

>
>