Change in vdsm[master]: contrib: schema converter

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Wed Mar 23 08:57:59 UTC 2016


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 <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list