Piotr Kliczewski has posted comments on this change. Change subject: bridge: data verification ......................................................................
Patch Set 10: (6 comments) https://gerrit.ovirt.org/#/c/53919/10/lib/api/schemaapi.py File lib/api/schemaapi.py: Line 111: self._report_inconsistency('Parameter %s is not %s type' Line 112: % (name, t)) Line 113: Line 114: def _report_inconsistency(self, message): Line 115: if config.getboolean('devel', 'strict_mode'): > Get this value only once and cache it here in the module since it won't cha Done Line 116: raise yajsonrpc.JsonRpcInvalidParamsError(message) Line 117: else: Line 118: self.log.warning(message) Line 119: Line 178: for a in arg: Line 179: self._verify_type(t[0], a, class_name, method_name) Line 180: else: Line 181: self._verify_ctype(t.get('type'), t, arg, name, class_name, Line 182: method_name) > The above cases each need a comment to explain what they are for. Done Line 183: Line 184: def _verify_ctype(self, t_type, t, arg, name, class_name, method_name): Line 185: if t_type == 'alias': Line 186: self._check_primitive_type(t.get('sourcetype'), arg, name) Line 180: else: Line 181: self._verify_ctype(t.get('type'), t, arg, name, class_name, Line 182: method_name) Line 183: Line 184: def _verify_ctype(self, t_type, t, arg, name, class_name, method_name): > _verify_complex_type would be a better name. Done Line 185: if t_type == 'alias': Line 186: self._check_primitive_type(t.get('sourcetype'), arg, name) Line 187: elif t_type == 'map': Line 188: for key, value in arg.iteritems(): Line 193: elif t_type == 'union': Line 194: for value in t.get('values'): Line 195: props = value.get('properties') Line 196: prop_names = [prop.get('name') for prop in props] Line 197: if not [key for key in arg.keys() if key not in prop_names]: > This is all getting complex. Please add some comments here. Done Line 198: self._verify_ctype(value.get('type'), value, arg, name, Line 199: class_name, method_name) Line 200: return Line 201: self._report_inconsistency('Provided parameters %s do not match' Line 241: continue Line 242: Line 243: self._verify_type(prop, a, class_name, method_name) Line 244: Line 245: def verify_ret_params(self, class_name, method_name, ret): > rename to verify_retval Done Line 246: try: Line 247: ret_args = self.get_ret_param(class_name, method_name) Line 248: Line 249: if ret_args: https://gerrit.ovirt.org/#/c/53919/10/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 440: ('health_check_interval', '60', Line 441: 'Number of seconds to wait between health checks.'), Line 442: Line 443: ('strict_mode', 'false', Line 444: 'Enable exception throwing when rpc data is not correct.'), > value should be something like 'api-strict-mode' or 'rpc-strict-mode' Done Line 445: Line 446: ]), Line 447: Line 448: # Section: [gluster] -- To view, visit https://gerrit.ovirt.org/53919 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id24a5e078fa92e4129d37a47593c7a167e78712e Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
