Re: [nft PATCH RFC 0/2] JSON schema for nftables.py

[Phil Sutter <phil@nwl.cc>]

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