This change is mostly cosmetic. Previously, the literal "cluster" was used for the 'name' field of tag operations on the cluster (as opposed to a node or an instance). Since this field has a type of TMaybeString specifically for the case of the cluster, it seems more correct to use None, rather than an arbitrary string (that is not used by the callee).
Additionally: note in opcodes.py that groups also expect a name; the previous comment only referred to nodes and instances. Signed-off-by: Dato Simó <[email protected]> --- On Fri, Oct 26, 2012 at 13:28 +0200, Iustin Pop <[email protected]> wrote: > On Tue, Oct 23, 2012 at 01:35:30PM +0100, Dato Simó wrote: > > On Tue, Oct 23, 2012 at 10:30 +0200, Iustin Pop <[email protected]> wrote: > > > > The resulting combination, NoDefault + TMaybeString, is that the field > > > > can be None, but even in that case it must be explicitly specified > > > > (i.e., it must be present in the serialization even if as null). The > > > > CLI, for example, solves this by using "cluster" as the value for name > > > > in the case of `gnt-cluster add-tags`, though it could have as well used > > > > None (I may send a patch for that, actually). > > > TBH, it sends "cluster" in order to have a clear type. But maybe this is > > > the wrong approach. > > Given that the expected type is TMaybeString, and that the received > > string for kind == "custer" is not checked/used at all and could be any > > value, then my expectation was that a more correct value to send would > > be None, that's all. > Ack, that makes sense. Done. lib/cli.py | 2 +- lib/opcodes.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cli.py b/lib/cli.py index c1b3e24..dae3bf7 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -412,7 +412,7 @@ def _ExtractTagsObject(opts, args): raise errors.ProgrammerError("tag_type not passed to _ExtractTagsObject") kind = opts.tag_type if kind == constants.TAG_CLUSTER: - retval = kind, kind + retval = kind, None elif kind in (constants.TAG_NODEGROUP, constants.TAG_NODE, constants.TAG_INSTANCE): diff --git a/lib/opcodes.py b/lib/opcodes.py index 2da68ce..5bdf85e 100644 --- a/lib/opcodes.py +++ b/lib/opcodes.py @@ -1815,7 +1815,7 @@ class OpTagsSet(OpCode): OP_PARAMS = [ _PTagKind, _PTags, - # Name is only meaningful for nodes and instances + # Name is only meaningful for groups, nodes and instances ("name", ht.NoDefault, ht.TMaybeString, "Name of object where tag(s) should be added"), ] @@ -1827,7 +1827,7 @@ class OpTagsDel(OpCode): OP_PARAMS = [ _PTagKind, _PTags, - # Name is only meaningful for nodes and instances + # Name is only meaningful for groups, nodes and instances ("name", ht.NoDefault, ht.TMaybeString, "Name of object where tag(s) should be deleted"), ] -- 1.7.12.2-x20-1
