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

[Rene Nussbaumer]

On Mon, Feb 14, 2011 at 5:37 PM, Michael Hanselmann <han...@google.com> wrote:
> ---
>  doc/rapi.rst                       |    6 +++-
>  lib/rapi/rlib2.py                  |   14 ++++++---
>  test/ganeti.rapi.rlib2_unittest.py |   49 
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+), 7 deletions(-)
>
> diff --git a/doc/rapi.rst b/doc/rapi.rst
> index 940f35a..8e1b2c7 100644
> --- a/doc/rapi.rst
> +++ b/doc/rapi.rst
> @@ -412,8 +412,10 @@ Returns: a job ID that can be used later for polling.
>
>  Body parameters:
>
> -``name`` (string, required)
> -  Node group name.
> +.. opcode_params:: OP_GROUP_ADD
> +
> +Earlier versions used a parameter named ``name`` which, while still
> +supported, has been renamed to ``group_name``.
>
>
>  ``/2/groups/[group_name]``
> diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
> index 371faf7..94511a2 100644
> --- a/lib/rapi/rlib2.py
> +++ b/lib/rapi/rlib2.py
> @@ -555,12 +555,16 @@ def _ParseCreateGroupRequest(data, dry_run):
>   @return: Group creation opcode
>
>   """
> -  group_name = baserlib.CheckParameter(data, "name")
> -  alloc_policy = baserlib.CheckParameter(data, "alloc_policy", default=None)
> +  override = {
> +    "dry_run": dry_run,
> +    }
>
> -  return opcodes.OpGroupAdd(group_name=group_name,
> -                            alloc_policy=alloc_policy,
> -                            dry_run=dry_run)
> +  rename = {
> +    "name": "group_name",
> +    }
> +
> +  return baserlib.FillOpcode(opcodes.OpGroupAdd, data, override,
> +                             rename=rename)
>
>
>  class R_2_groups(baserlib.R_Generic):
> diff --git a/test/ganeti.rapi.rlib2_unittest.py 
> b/test/ganeti.rapi.rlib2_unittest.py
> index 52ee7f3..98538eb 100755
> --- a/test/ganeti.rapi.rlib2_unittest.py
> +++ b/test/ganeti.rapi.rlib2_unittest.py
> @@ -551,5 +551,54 @@ class TestParseModifyGroupRequest(unittest.TestCase):
>     self.assertFalse(hasattr(op, "alloc_policy"))
>
>
> +class TestParseCreateGroupRequest(unittest.TestCase):
> +  def setUp(self):
> +    self.Parse = rlib2._ParseCreateGroupRequest
> +
> +  def test(self):
> +    name = "group3618"
> +
> +    for policy in constants.VALID_ALLOC_POLICIES:
> +      data = {
> +        "group_name": name,
> +        "alloc_policy": policy,
> +        }
> +
> +      op = self.Parse(data, False)
> +      self.assert_(isinstance(op, opcodes.OpGroupAdd))
> +      self.assertEqual(op.group_name, name)
> +      self.assertEqual(op.alloc_policy, policy)
> +      self.assertFalse(op.dry_run)
> +
> +  def testUnknownPolicy(self):
> +    data = {
> +      "alloc_policy": "_unknown_policy_",
> +      }
> +
> +    self.assertRaises(http.HttpBadRequest, self.Parse, "name", data)
> +
> +  def testDefaults(self):
> +    name = "group15395"
> +    data = {
> +      "group_name": name,
> +      }
> +
> +    op = self.Parse(data, True)
> +    self.assert_(isinstance(op, opcodes.OpGroupAdd))
> +    self.assertEqual(op.group_name, name)
> +    self.assertFalse(hasattr(op, "alloc_policy"))
> +    self.assertTrue(op.dry_run)
> +
> +  def testLegacyName(self):
> +    name = "group29852"
> +    data = {
> +      "name": name,
> +      }
> +
> +    op = self.Parse(data, True)
> +    self.assert_(isinstance(op, opcodes.OpGroupAdd))
> +    self.assertEqual(op.group_name, name)
> +
> +
>  if __name__ == '__main__':
>   testutils.GanetiTestProgram()
> --
> 1.7.3.5

LGTM

>
>