Piotr Kliczewski has posted comments on this change. Change subject: contrib: schema converter ......................................................................
Patch Set 25: (7 comments) https://gerrit.ovirt.org/#/c/52864/25/lib/api/schemaapi.py File lib/api/schemaapi.py: Line 48: raise SchemaNotFound("Unable to find API schema file in %s or %s", Line 49: localpath, installedpath) Line 50: Line 51: Line 52: class SchemaCache(object): > Can you rename this class in this patch? (should not break the next patches I missed it in this patch. All the other patches are correctly named. Will update. Line 53: Line 54: def __init__(self, paths): Line 55: self._schema = {} Line 56: try: Line 54: def __init__(self, paths): Line 55: self._schema = {} Line 56: try: Line 57: for path in paths: Line 58: with open(path) as f: > Why do we need to support multiple schema files? Vdsm and gluster schema files and in future patches event schema. Line 59: data = yaml.load(f) Line 60: self._schema.update({key: value for (key, value) Line 61: in data.items() if key != 'types'}) Line 62: Line 55: self._schema = {} Line 56: try: Line 57: for path in paths: Line 58: with open(path) as f: Line 59: data = yaml.load(f) > data is good name for blob of bytes. But this is a dict, or actually a sche Will fix. Line 60: self._schema.update({key: value for (key, value) Line 61: in data.items() if key != 'types'}) Line 62: Line 63: self._schema['types'] = dict( Line 61: in data.items() if key != 'types'}) Line 62: Line 63: self._schema['types'] = dict( Line 64: self._schema.get('types', {}).items() + Line 65: data.get('types').items()) > This code is not clear and very inefficient. Will fix. Line 66: except IOError: Line 67: raise SchemaNotFound("Unable to find API schema file") Line 68: Line 69: def get_params(self, class_name, method_name): https://gerrit.ovirt.org/#/c/52864/25/tests/schemaValidationTest.py File tests/schemaValidationTest.py: Line 77 Line 78 Line 79 Line 80 Line 81 > Why do we need to change this test? We do not need bridge in this test, except of one place. We validate yaml schema now. Line 36: from nose.plugins.skip import SkipTest Line 37: Line 38: Line 39: @contextmanager Line 40: def schema_not_found(): > Why do we need this? Looks like leftover. Will remove. Line 41: try: Line 42: yield Line 43: except schemaapi.SchemaNotFound: Line 44: raise SkipTest('yaml schema not available') Line 194: else: Line 195: return None Line 196: Line 197: Line 198: class JsonToYamlValidation(TestCaseBase): > To test the yaml schema, we don't need to find the schema. The test itself should not convert the schema. It uses already converted schema to compare with json. Line 199: Line 200: def swap_types(self, name): Line 201: if name == 'str': Line 202: return 'string' -- To view, visit https://gerrit.ovirt.org/52864 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3921cebb7f550f63849f3bc5c80636b6e9495c92 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Darshan N <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[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
