Juan Hernandez has posted comments on this change. Change subject: cli: Check that the --max option has a value ......................................................................
Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/24080/2/src/ovirtcli/command/command.py File src/ovirtcli/command/command.py: Line 261: Line 262: # Check that all the options that require a value actually have it: Line 263: for k, v in mopts.iteritems(): Line 264: if v is None and self._is_value_required(k): Line 265: self.error(Messages.Error.OPTION_REQUIRES_VALUE % k) > you have single _is_value_required() for all list() commands, what if in l This is not the case for the "max" option, it works the same for all "list" commands. If there were the need to differentiate list' and list'' then this logic should go in the _is_value_required method. It is impossible to do this in a generic way in the CLI, as it is impossible to extract this knowledge from the SDK. A parameter defined as "max=None" doesn't provide any information to infer if the parameter can or can't be sent without a value. In addition, what we currently do is that we just don't send to the server parameters without values. For example, the following command: list vms --max Results in the following request: GET /api/vms HTTP/1.1 So the validation can't be performed on the server either. Line 266: Line 267: return query, kw Line 268: Line 269: -- To view, visit http://gerrit.ovirt.org/24080 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0801c60d33b5e30f8844659df98e6d8405f34167 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine-cli Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Michael Pasternak <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
