Michael Pasternak 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) > This is not the case for the "max" option, it works the same for all "list" > 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. you cannot address all the cases, - it will mean that once new list() added in REST, you'll have to add per-case treatment in CLI > 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. right, actually you can know if it mandatory, but information in sdk is about parameter existence in request, and not about it content, this is because we avoid having business logic in clients > 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. true, but this is due to a bug we had in REST (search for NPE when max==null), and is my bad for not cleaning it, Juan, i understand your concern about silent ignorance, but the thing is that i just don't want to split the logic between server and clients, so if one day we will have a feature where certain functionality can be triggered or disabled via empty param (url/header/etc.) we won't have to look for all places where it is blocked, otherwise CLI no longer will be deploy-sdk&run 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
