Hi Eric, On Thu, Apr 25, 2019 at 12:35:46PM -0400, Eric Garver wrote: > On Thu, Apr 25, 2019 at 04:05:00PM +0200, Phil Sutter wrote: [...] > > Keeping the schema in a "pure" JSON file makes things a bit complex: It > > has to be shipped as data file and loaded by the validator using > > json.load(). OTOH the content may be fed into json_verify and my editor > > provides nicer syntax highlighting. An alternative would be to name it > > schema.py, prefix the content with 'nftschema = ' and simply import it > > into nftables.py. I don't think inlining the content is a good option > > simply due to how large the file will get once definitions for all > > statements and expressions are in there. > > I very much prefer the external JSON file. Other projects can then use > it to validate the JSON they generate without going through libnftables. Valid point, although the file will be hidden in /lib64/python2.7/site-packages/nftables/schema.json after installation. > > Introducing that SchemaValidator class is not really required, either. > > Though squeezing everything into json_validate() method felt clumsy. > > Also I wanted to avoid the explicit schema loading mentioned above upon > > each call to json_validate(), so having an instance of a validator class > > seemed like how one is supposed to do things in an object-oriented > > language. > > > > Note that SchemaValidator imports jsonschema upon instantiation. This > > may be a bad idea to begin with, but the intention is to not introduce a > > hard dependency on jsonschema in nftables.py. Same argument holds for > > conditional import of traceback module in nft-test.py, although > > validator errors are practically useless without it. > > I agree with the optional jsonschema dependency. > > traceback is part of the python standard library. No reason to make it a > conditional import unless you're worried about the cost of importing it. Thanks for pointing this out! For the testsuite, unconditionally importing traceback should be perfectly fine. Thanks, Phil
