Adam Litke 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 change. 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. 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. 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. 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 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' 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 <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
